Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: resolve special IP host-gateway #453

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: resolve special IP host-gateway #453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
faa3b20
Merge branch 'upstream/main' and adding test
TrungBui59 Sep 1, 2023
410c2b7
Fixing nit issue
TrungBui59 Sep 3, 2023
65921b9
Adding test for finch config file for test case
TrungBui59 Sep 5, 2023
c540203
Fixing host_gateway_ip to follow nerdctl standard
TrungBui59 Sep 6, 2023
8f618b9
Merge branch 'runfinch:main' into main
TrungBui59 Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/finch/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func virtualMachineCommands(
logger,
optionalDepGroups,
config.NewLimaApplier(fc, ecc, fs, fp.LimaOverrideConfigPath(), system.NewStdLib()),
config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), system.NewStdLib().Env("USER")),
config.NewNerdctlApplier(fc, fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), system.NewStdLib().Env("USER")),
fp,
fs,
disk.NewUserDataDiskManager(lcc, ecc, &afero.OsFs{}, fp, system.NewStdLib().Env("HOME"), fc),
Expand Down
25 changes: 0 additions & 25 deletions cmd/finch/nerdctl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ import (
"path/filepath"
"strings"

dockerops "github.com/docker/docker/opts"
"github.com/lima-vm/lima/pkg/networks"

"github.com/spf13/afero"
"github.com/spf13/cobra"

Expand Down Expand Up @@ -112,15 +109,6 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
}
skip = shouldSkip
fileEnvs = append(fileEnvs, addEnvs...)
case strings.HasPrefix(arg, "--add-host"):
switch arg {
case "--add-host":
args[i+1] = resolveIP(args[i+1], nc.logger)
default:
resolvedIP := resolveIP(arg[11:], nc.logger)
arg = fmt.Sprintf("%s%s", arg[0:11], resolvedIP)
}
nerdctlArgs = append(nerdctlArgs, arg)
default:
nerdctlArgs = append(nerdctlArgs, arg)
}
Expand Down Expand Up @@ -305,19 +293,6 @@ func handleEnvFile(fs afero.Fs, systemDeps NerdctlCommandSystemDeps, arg, arg2 s
return skip, envs, nil
}

func resolveIP(host string, logger flog.Logger) string {
parts := strings.SplitN(host, ":", 2)
// If the IP Address is a string called "host-gateway", replace this value with the IP address that can be used to
// access host from the containers.
// TODO: make the host gateway ip configurable.
if parts[1] == dockerops.HostGatewayName {
resolvedIP := networks.SlirpGateway
logger.Debugf(`Resolving special IP "host-gateway" to %q for host %q`, resolvedIP, parts[0])
return fmt.Sprintf("%s:%s", parts[0], resolvedIP)
}
return host
}

var nerdctlCmds = map[string]string{
"build": "Build an image from Dockerfile",
"builder": "Manage builds",
Expand Down
138 changes: 0 additions & 138 deletions cmd/finch/nerdctl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,144 +365,6 @@ func TestNerdctlCommand_run(t *testing.T) {
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
},
},
{
ginglis13 marked this conversation as resolved.
Show resolved Hide resolved
name: "with --add-host flag and special IP by space",
cmdName: "run",
args: []string{"--rm", "--add-host", "name:host-gateway", "alpine:latest"},
wantErr: nil,
mockSvc: func(
t *testing.T,
lcc *mocks.LimaCmdCreator,
ncsd *mocks.NerdctlCommandSystemDeps,
logger *mocks.Logger,
ctrl *gomock.Controller,
fs afero.Fs,
) {
getVMStatusC := mocks.NewCommand(ctrl)
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name")
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
"--rm", "--add-host", "name:192.168.5.2", "alpine:latest").Return(c)
c.EXPECT().Run()
},
},
{
name: "with --add-host flag but without using special IP by space",
cmdName: "run",
args: []string{"--rm", "--add-host", "name:0.0.0.0", "alpine:latest"},
wantErr: nil,
mockSvc: func(
t *testing.T,
lcc *mocks.LimaCmdCreator,
ncsd *mocks.NerdctlCommandSystemDeps,
logger *mocks.Logger,
ctrl *gomock.Controller,
fs afero.Fs,
) {
getVMStatusC := mocks.NewCommand(ctrl)
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
"--rm", "--add-host", "name:0.0.0.0", "alpine:latest").Return(c)
c.EXPECT().Run()
},
},
{
name: "with --add-host flag but without subsequent arg",
cmdName: "run",
args: []string{"--rm", "--add-host", "alpine:latest"},
wantErr: errors.New("run cmd error"),
mockSvc: func(
t *testing.T,
lcc *mocks.LimaCmdCreator,
ncsd *mocks.NerdctlCommandSystemDeps,
logger *mocks.Logger,
ctrl *gomock.Controller,
fs afero.Fs,
) {
getVMStatusC := mocks.NewCommand(ctrl)
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
"--rm", "--add-host", "alpine:latest").Return(c)
c.EXPECT().Run().Return(errors.New("run cmd error"))
},
},
{
name: "with --add-host flag and special IP by equal",
cmdName: "run",
args: []string{"--rm", "--add-host=name:host-gateway", "alpine:latest"},
wantErr: nil,
mockSvc: func(
t *testing.T,
lcc *mocks.LimaCmdCreator,
ncsd *mocks.NerdctlCommandSystemDeps,
logger *mocks.Logger,
ctrl *gomock.Controller,
fs afero.Fs,
) {
getVMStatusC := mocks.NewCommand(ctrl)
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name")
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
"--rm", "--add-host=name:192.168.5.2", "alpine:latest").Return(c)
c.EXPECT().Run()
},
},
{
name: "with --add-host flag but without using special IP by equal",
cmdName: "run",
args: []string{"--rm", "--add-host=name:0.0.0.0", "alpine:latest"},
wantErr: nil,
mockSvc: func(
t *testing.T,
lcc *mocks.LimaCmdCreator,
ncsd *mocks.NerdctlCommandSystemDeps,
logger *mocks.Logger,
ctrl *gomock.Controller,
fs afero.Fs,
) {
getVMStatusC := mocks.NewCommand(ctrl)
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)

c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
"--rm", "--add-host=name:0.0.0.0", "alpine:latest").Return(c)
c.EXPECT().Run()
},
},
{
name: "with multiple nested volumes",
cmdName: "run",
Expand Down
80 changes: 80 additions & 0 deletions e2e/vm/host_gateway_ip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package vm

import (
"os"
"os/exec"
"path/filepath"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
"github.com/onsi/gomega/gexec"
"github.com/runfinch/common-tests/command"
"github.com/runfinch/common-tests/option"
"gopkg.in/yaml.v3"

"github.com/runfinch/finch/e2e"
"github.com/runfinch/finch/pkg/config"
)

const defaultImage = "public.ecr.aws/docker/library/alpine:latest"

var testHostGatewayIp = func(o *option.Option, installed bool) {
ningziwen marked this conversation as resolved.
Show resolved Hide resolved
// it requires disk I/O so this will be serial test
ginkgo.Describe("--add-host tag with host_gateway", ginkgo.Serial, func() {
var limaConfigFilePath string
ginkgo.BeforeEach(func() {
origFinchCfg := readFile(finchConfigFilePath)
limaConfigFilePath = defaultLimaConfigFilePath
if installed {
path, err := exec.LookPath(e2e.InstalledTestSubject)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
realFinchPath, err := filepath.EvalSymlinks(path)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
limaConfigFilePath = filepath.Join(realFinchPath, "../../lima/data/_config/override.yaml")
}
origLimaCfg := readFile(limaConfigFilePath)

ginkgo.DeferCleanup(func() {
writeFile(finchConfigFilePath, origFinchCfg)
writeFile(limaConfigFilePath, origLimaCfg)

command.New(o, virtualMachineRootCmd, "stop").WithoutCheckingExitCode().WithTimeoutInSeconds(90).Run()
command.New(o, virtualMachineRootCmd, "start").WithTimeoutInSeconds(240).Run()
})
})
ginkgo.It("should add a custom host-to-IP mapping with --add-host flag: default value", func() {
startCmdSession := updateAndApplyConfig(o, []byte("memory: 4GiB\ncpus: 6"))
gomega.Expect(startCmdSession).Should(gexec.Exit(0))

gomega.Expect(finchConfigFilePath).Should(gomega.BeARegularFile())
cfgBuf, err := os.ReadFile(filepath.Clean(finchConfigFilePath))
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())

var finchCfg config.Finch
err = yaml.Unmarshal(cfgBuf, &finchCfg)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
gomega.Expect(finchCfg.HostGatewayIp).Should(gomega.BeNil())

mapping := command.StdoutStr(o, "run", "--add-host", "test-host:host-gateway", defaultImage, "cat", "/etc/hosts")
gomega.Expect(mapping).Should(gomega.ContainSubstring("192.168.5.2"))
gomega.Expect(mapping).Should(gomega.ContainSubstring("test-host"))
})
ginkgo.It("should add a custom host-to-IP mapping with --add-host flag: custom value", func() {
startCmdSession := updateAndApplyConfig(o, []byte("memory: 4GiB\ncpus: 6\nhost_gateway_ip: 192.168.31.1"))
gomega.Expect(startCmdSession).Should(gexec.Exit(0))

gomega.Expect(finchConfigFilePath).Should(gomega.BeARegularFile())
cfgBuf, err := os.ReadFile(filepath.Clean(finchConfigFilePath))
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())

var finchCfg config.Finch
err = yaml.Unmarshal(cfgBuf, &finchCfg)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
gomega.Expect(*finchCfg.HostGatewayIp).Should(gomega.Equal("192.168.31.1"))

mapping := command.StdoutStr(o, "run", "--add-host", "test-host:host-gateway", defaultImage, "cat", "/etc/hosts")
gomega.Expect(mapping).Should(gomega.ContainSubstring("192.168.31.1"))
gomega.Expect(mapping).Should(gomega.ContainSubstring("test-host"))
})
})
}
1 change: 1 addition & 0 deletions e2e/vm/vm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func TestVM(t *testing.T) {
testSupportBundle(o)
testCredHelper(o, *e2e.Installed, *e2e.Registry)
testSoci(o, *e2e.Installed)
testHostGatewayIp(o, *e2e.Installed)
})

gomega.RegisterFailHandler(ginkgo.Fail)
Expand Down
4 changes: 3 additions & 1 deletion pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ type Finch struct {
// Requires macOS 13.0 or later and an Apple Silicon (ARM64) mac.
// Has no effect on systems where Rosetta 2 is not available (Intel/AMD64 macs, or macOS < 13.0).
// This setting will only be applied on vm init.
Rosetta *bool `yaml:"rosetta,omitempty"`
Rosetta *bool `yaml:"rosetta,omitempty"`
HostGatewayIP *string `yaml:"host_gateway_ip,omitempty"`
pendo324 marked this conversation as resolved.
Show resolved Hide resolved
}

// Nerdctl is a copy from github.com/containerd/nerdctl/cmd/nerdctl/main.go
Expand All @@ -72,6 +73,7 @@ type Nerdctl struct {
CgroupManager string `toml:"cgroup_manager,omitempty"`
InsecureRegistry bool `toml:"insecure_registry,omitempty"`
HostsDir []string `toml:"hosts_dir,omitempty"`
HostGatewayIP string `toml:"host_gateway_ip,omitempty"`
}

// LimaConfigApplier applies lima configuration changes.
Expand Down
12 changes: 9 additions & 3 deletions pkg/config/nerdctl_config_applier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const (
)

type nerdctlConfigApplier struct {
fc *Finch
dialer fssh.Dialer
fs afero.Fs
privateKeyPath string
Expand All @@ -33,8 +34,9 @@ var _ NerdctlConfigApplier = (*nerdctlConfigApplier)(nil)

// NewNerdctlApplier creates a new NerdctlConfigApplier that
// applies nerdctl configuration changes by SSHing to the lima VM to update the nerdctl configuration file in it.
func NewNerdctlApplier(dialer fssh.Dialer, fs afero.Fs, privateKeyPath, hostUser string) NerdctlConfigApplier {
func NewNerdctlApplier(fc *Finch, dialer fssh.Dialer, fs afero.Fs, privateKeyPath, hostUser string) NerdctlConfigApplier {
return &nerdctlConfigApplier{
fc: fc,
dialer: dialer,
fs: fs,
privateKeyPath: privateKeyPath,
Expand Down Expand Up @@ -93,7 +95,7 @@ func updateEnvironment(fs afero.Fs, user string) error {
}

// updateNerdctlConfig reads from the nerdctl config and updates values.
func updateNerdctlConfig(fs afero.Fs, user string, rootless bool) error {
func updateNerdctlConfig(fc *Finch, fs afero.Fs, user string, rootless bool) error {
nerdctlRootlessCfgPath := fmt.Sprintf("/home/%s.linux/.config/nerdctl/nerdctl.toml", user)

var cfgPath string
Expand Down Expand Up @@ -124,6 +126,10 @@ func updateNerdctlConfig(fs afero.Fs, user string, rootless bool) error {
}

cfg.Namespace = nerdctlNamespace
cfg.HostGatewayIP = "192.168.5.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of having this magic string down here, could we define as a constant defaultNerdctlHostGatewayIP here https://github.com/runfinch/finch/pull/453/files#diff-531afb9f97a7384e0de895046f9e14befbebc49c896b080fce4c41c75f14f1b4L20-L21

then you can also reference it in unit tests where you expect the HostGatewayIP to be the default value.

if fc.HostGatewayIP != nil {
cfg.HostGatewayIP = *fc.HostGatewayIP
}

updatedCfg, err := toml.Marshal(cfg)
if err != nil {
Expand Down Expand Up @@ -158,7 +164,7 @@ func (nca *nerdctlConfigApplier) Apply(remoteAddr string) error {
sftpFs := sftpfs.New(sftpClient)

// rootless hardcoded to false for now to match our finch.yaml file
if err := updateNerdctlConfig(sftpFs, user, false); err != nil {
if err := updateNerdctlConfig(nca.fc, sftpFs, user, false); err != nil {
return fmt.Errorf("failed to update the nerdctl config file: %w", err)
}

Expand Down
Loading
Loading