From 476b269013b814dd76013dc6bad358970f56c254 Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Thu, 23 Mar 2023 15:29:36 -0700 Subject: [PATCH 01/12] Correctly set and pick up environment variables Signed-off-by: Ang Zhou --- cmd/finch/main.go | 2 +- cmd/finch/nerdctl.go | 2 +- pkg/config/nerdctl_config_applier.go | 8 +++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cmd/finch/main.go b/cmd/finch/main.go index a93cf811b..5c68b5517 100644 --- a/cmd/finch/main.go +++ b/cmd/finch/main.go @@ -117,7 +117,7 @@ func virtualMachineCommands( logger, optionalDepGroups, config.NewLimaApplier(fc, fs, fp.LimaOverrideConfigPath()), - config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath()), + config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), system.NewStdLib().Env("USER")), fp, fs, disk.NewUserDataDiskManager(lcc, &afero.OsFs{}, fp, system.NewStdLib().Env("HOME")), diff --git a/cmd/finch/nerdctl.go b/cmd/finch/nerdctl.go index b9dfe73a9..d7a08fbb4 100644 --- a/cmd/finch/nerdctl.go +++ b/cmd/finch/nerdctl.go @@ -146,7 +146,7 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { } finalArgs = append(finalArgs, nerdctlArgs...) - limaArgs := append([]string{"shell", limaInstanceName, "sudo", nerdctlCmdName, cmdName}, finalArgs...) + limaArgs := append([]string{"shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, cmdName}, finalArgs...) if nc.shouldReplaceForHelp(cmdName, args) { return nc.creator.RunWithReplacingStdout([]command.Replacement{{Source: "nerdctl", Target: "finch"}}, limaArgs...) diff --git a/pkg/config/nerdctl_config_applier.go b/pkg/config/nerdctl_config_applier.go index 6dbee669a..3dcaa8cea 100644 --- a/pkg/config/nerdctl_config_applier.go +++ b/pkg/config/nerdctl_config_applier.go @@ -26,17 +26,19 @@ type nerdctlConfigApplier struct { dialer fssh.Dialer fs afero.Fs privateKeyPath string + hostUser string } 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 string) NerdctlConfigApplier { +func NewNerdctlApplier(dialer fssh.Dialer, fs afero.Fs, privateKeyPath, hostUser string) NerdctlConfigApplier { return &nerdctlConfigApplier{ dialer: dialer, fs: fs, privateKeyPath: privateKeyPath, + hostUser: hostUser, } } @@ -52,7 +54,7 @@ func NewNerdctlApplier(dialer fssh.Dialer, fs afero.Fs, privateKeyPath string) N // // [registry nerdctl docs]: https://github.com/containerd/nerdctl/blob/master/docs/registry.md func updateEnvironment(fs afero.Fs, user string) error { - profileFilePath := "/root/.bashrc" + profileFilePath := fmt.Sprintf("/home/%s.linux/.bashrc", user) profBuf, err := afero.ReadFile(fs, profileFilePath) if err != nil { return fmt.Errorf("failed to read config file: %w", err) @@ -139,7 +141,7 @@ func (nca *nerdctlConfigApplier) Apply(remoteAddr string) error { return fmt.Errorf("failed to update the nerdctl config file: %w", err) } - if err := updateEnvironment(sftpFs, user); err != nil { + if err := updateEnvironment(sftpFs, nca.hostUser); err != nil { return fmt.Errorf("failed to update the user's .profile file: %w", err) } return nil From 76c99d368ec5025db093b8314ecb43df9bb1ae0e Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Thu, 23 Mar 2023 16:25:08 -0700 Subject: [PATCH 02/12] fix unit tests Signed-off-by: Ang Zhou --- cmd/finch/main.go | 6 ++--- cmd/finch/nerdctl_test.go | 28 +++++++++++------------ pkg/config/nerdctl_config_applier_test.go | 14 +++++++----- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/cmd/finch/main.go b/cmd/finch/main.go index 5c68b5517..46b8a1db5 100644 --- a/cmd/finch/main.go +++ b/cmd/finch/main.go @@ -111,16 +111,16 @@ func virtualMachineCommands( fc *config.Finch, ) *cobra.Command { optionalDepGroups := []*dependency.Group{vmnet.NewDependencyGroup(ecc, lcc, fs, fp, logger)} - + stdLib := system.NewStdLib() return newVirtualMachineCommand( lcc, logger, optionalDepGroups, config.NewLimaApplier(fc, fs, fp.LimaOverrideConfigPath()), - config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), system.NewStdLib().Env("USER")), + config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), stdLib.Env("USER")), fp, fs, - disk.NewUserDataDiskManager(lcc, &afero.OsFs{}, fp, system.NewStdLib().Env("HOME")), + disk.NewUserDataDiskManager(lcc, &afero.OsFs{}, fp, stdLib.Env("HOME")), ) } diff --git a/cmd/finch/nerdctl_test.go b/cmd/finch/nerdctl_test.go index 702efd16c..2828a5c9a 100644 --- a/cmd/finch/nerdctl_test.go +++ b/cmd/finch/nerdctl_test.go @@ -54,7 +54,7 @@ func TestNerdctlCommand_runAdaptor(t *testing.T) { getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "info").Return(c) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "info").Return(c) c.EXPECT().Run() }, }, @@ -104,7 +104,7 @@ func TestNerdctlCommand_run(t *testing.T) { getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "build", "-t", "demo", ".").Return(c) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "build", "-t", "demo", ".").Return(c) c.EXPECT().Run() }, }, @@ -205,7 +205,7 @@ func TestNerdctlCommand_run(t *testing.T) { logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") logger.EXPECT().SetLevel(flog.Debug) c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "pull", "test:tag").Return(c) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag").Return(c) c.EXPECT().Run() }, }, @@ -229,7 +229,7 @@ func TestNerdctlCommand_run(t *testing.T) { c := mocks.NewCommand(ctrl) ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("ARG3") - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "-e", "ARG1=val1", "--rm", "alpine:latest", "env").Return(c) c.EXPECT().Run() }, @@ -254,7 +254,7 @@ func TestNerdctlCommand_run(t *testing.T) { c := mocks.NewCommand(ctrl) ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "-e", "ARG3=val3", "--rm", "alpine:latest", "env").Return(c) c.EXPECT().Run() }, @@ -283,7 +283,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("NOTSETARG") lcc.EXPECT(). - Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", "-e", "ARG1=val1", "--rm", "alpine:latest", "env"). + Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "-e", "ARG1=val1", "--rm", "alpine:latest", "env"). Return(c) c.EXPECT().Run() }, @@ -312,7 +312,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("ARG2").Return("val2", true) ncsd.EXPECT().LookupEnv("NOTSETARG") lcc.EXPECT(). - Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", "-e", "ARG2=val2", "--rm", "alpine:latest", "env"). + Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "-e", "ARG2=val2", "--rm", "alpine:latest", "env"). Return(c) c.EXPECT().Run() }, @@ -355,7 +355,7 @@ func TestNerdctlCommand_run(t *testing.T) { logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name") c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "--add-host", "name:192.168.5.2", "alpine:latest").Return(c) c.EXPECT().Run() }, @@ -378,7 +378,7 @@ func TestNerdctlCommand_run(t *testing.T) { getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "--add-host", "name:0.0.0.0", "alpine:latest").Return(c) c.EXPECT().Run() }, @@ -401,7 +401,7 @@ func TestNerdctlCommand_run(t *testing.T) { getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", + 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")) }, @@ -425,7 +425,7 @@ func TestNerdctlCommand_run(t *testing.T) { logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name") c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "--add-host=name:192.168.5.2", "alpine:latest").Return(c) c.EXPECT().Run() }, @@ -448,7 +448,7 @@ func TestNerdctlCommand_run(t *testing.T) { getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", nerdctlCmdName, "run", + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "--add-host=name:0.0.0.0", "alpine:latest").Return(c) c.EXPECT().Run() }, @@ -471,7 +471,7 @@ func TestNerdctlCommand_run(t *testing.T) { getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") lcc.EXPECT().RunWithReplacingStdout( - testStdoutRs, "shell", limaInstanceName, "sudo", nerdctlCmdName, "pull", "test:tag", "--help").Return(nil) + testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help").Return(nil) }, }, { @@ -492,7 +492,7 @@ func TestNerdctlCommand_run(t *testing.T) { getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") lcc.EXPECT().RunWithReplacingStdout( - testStdoutRs, "shell", limaInstanceName, "sudo", nerdctlCmdName, "pull", "test:tag", "--help"). + testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help"). Return(fmt.Errorf("failed to replace")) }, }, diff --git a/pkg/config/nerdctl_config_applier_test.go b/pkg/config/nerdctl_config_applier_test.go index 18d0b841e..512a9395c 100644 --- a/pkg/config/nerdctl_config_applier_test.go +++ b/pkg/config/nerdctl_config_applier_test.go @@ -42,10 +42,10 @@ func Test_updateEnvironment(t *testing.T) { name: "happy path", user: "mock_user", mockSvc: func(t *testing.T, fs afero.Fs) { - require.NoError(t, afero.WriteFile(fs, "/root/.bashrc", []byte(""), 0o644)) + require.NoError(t, afero.WriteFile(fs, "/home/mock_user.linux/.bashrc", []byte(""), 0o644)) }, postRunCheck: func(t *testing.T, fs afero.Fs) { - fileBytes, err := afero.ReadFile(fs, "/root/.bashrc") + fileBytes, err := afero.ReadFile(fs, "/home/mock_user.linux/.bashrc") require.NoError(t, err) assert.Equal(t, []byte("\n"+`export DOCKER_CONFIG="/Users/mock_user/.finch"`+"\n"), fileBytes) }, @@ -59,14 +59,14 @@ func Test_updateEnvironment(t *testing.T) { t, afero.WriteFile( fs, - "/root/.bashrc", + "/home/mock_user.linux/.bashrc", []byte(`export DOCKER_CONFIG="/Users/mock_user/.finch"`), 0o644, ), ) }, postRunCheck: func(t *testing.T, fs afero.Fs) { - fileBytes, err := afero.ReadFile(fs, "/root/.bashrc") + fileBytes, err := afero.ReadFile(fs, "/home/mock_user.linux/.bashrc") require.NoError(t, err) assert.Equal(t, []byte(`export DOCKER_CONFIG="/Users/mock_user/.finch"`), fileBytes) }, @@ -79,7 +79,7 @@ func Test_updateEnvironment(t *testing.T) { postRunCheck: func(t *testing.T, fs afero.Fs) {}, want: fmt.Errorf( "failed to read config file: %w", - &fs.PathError{Op: "open", Path: "/root/.bashrc", Err: errors.New("file does not exist")}, + &fs.PathError{Op: "open", Path: "/home/mock_user.linux/.bashrc", Err: errors.New("file does not exist")}, ), }, } @@ -190,6 +190,7 @@ func TestNerdctlConfigApplier_Apply(t *testing.T) { testCases := []struct { name string path string + hostUser string remoteAddr string mockSvc func(t *testing.T, fs afero.Fs, d *mocks.Dialer) want error @@ -198,6 +199,7 @@ func TestNerdctlConfigApplier_Apply(t *testing.T) { name: "private key path doesn't exist", path: "/private-key", remoteAddr: "", + hostUser: "mock-host-user", mockSvc: func(t *testing.T, fs afero.Fs, d *mocks.Dialer) { }, want: fmt.Errorf( @@ -232,7 +234,7 @@ func TestNerdctlConfigApplier_Apply(t *testing.T) { d := mocks.NewDialer(ctrl) tc.mockSvc(t, fs, d) - got := NewNerdctlApplier(d, fs, tc.path).Apply(tc.remoteAddr) + got := NewNerdctlApplier(d, fs, tc.path, tc.hostUser).Apply(tc.remoteAddr) assert.Equal(t, tc.want, got) }) From 25847db5a9d41c4ec9b5ecd6f0d0ea98959b3722 Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Thu, 23 Mar 2023 16:31:35 -0700 Subject: [PATCH 03/12] add comment to -E flag Signed-off-by: Ang Zhou --- cmd/finch/nerdctl.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/finch/nerdctl.go b/cmd/finch/nerdctl.go index d7a08fbb4..a1abd8dfb 100644 --- a/cmd/finch/nerdctl.go +++ b/cmd/finch/nerdctl.go @@ -145,7 +145,8 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { finalArgs = append(finalArgs, "-e", fmt.Sprintf("%s=%s", key, val)) } finalArgs = append(finalArgs, nerdctlArgs...) - + // Add -E to sudo command in order to preserve existing environment variables, more info: + // https://stackoverflow.com/questions/8633461/how-to-keep-environment-variables-when-using-sudo/8633575#8633575 limaArgs := append([]string{"shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, cmdName}, finalArgs...) if nc.shouldReplaceForHelp(cmdName, args) { From 6d6360a7e3b5e8d7d76d5c026d12e0ff5964a9fa Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Fri, 24 Mar 2023 00:09:45 -0700 Subject: [PATCH 04/12] add e2e tests Signed-off-by: Ang Zhou --- e2e/vm/finch_config_file_test.go | 62 ++++++++++++++++++++++++++++++++ e2e/vm/vm_test.go | 1 + 2 files changed, 63 insertions(+) create mode 100644 e2e/vm/finch_config_file_test.go diff --git a/e2e/vm/finch_config_file_test.go b/e2e/vm/finch_config_file_test.go new file mode 100644 index 000000000..18a495b22 --- /dev/null +++ b/e2e/vm/finch_config_file_test.go @@ -0,0 +1,62 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package vm + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + "github.com/runfinch/common-tests/command" + "github.com/runfinch/common-tests/ffs" + "github.com/runfinch/common-tests/fnet" + "github.com/runfinch/common-tests/option" +) + +// testFinchConfigFile makes sure that DOCKER_CONFIG is properly set to ~/.finch so that related information +// is written to ~/.finch/config.json file. +var testFinchConfigFile = func(o *option.Option) { + ginkgo.Describe("finch config file", func() { + ginkgo.It("should store login credentials", func() { + filename := "htpasswd" + registryImage := "public.ecr.aws/docker/library/registry:latest" + // The htpasswd is generated by + // `finch run --entrypoint htpasswd public.ecr.aws/docker/library/httpd:2 -Bbn testUser testPassword`. + // We don't want to generate it on the fly because: + // 1. Pulling the httpd image can take a long time, sometimes even more 10 seconds. + // 2. It's unlikely that we will have to update this in the future. + // 3. It's not the thing we want to validate by the functional tests. We only want the output produced by it. + //nolint:gosec // This password is only used for testing purpose. + htpasswd := "testUser:$2y$05$wE0sj3r9O9K9q7R0MXcfPuIerl/06L1IsxXkCuUr3QZ8lHWwicIdS" + htpasswdDir := filepath.Dir(ffs.CreateTempFile(filename, htpasswd)) + ginkgo.DeferCleanup(os.RemoveAll, htpasswdDir) + port := fnet.GetFreePort() + command.Run(o, "run", + "-dp", fmt.Sprintf("%d:5000", port), + "--name", "registry", + "-v", fmt.Sprintf("%s:/auth", htpasswdDir), + "-e", "REGISTRY_AUTH=htpasswd", + "-e", "REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm", + "-e", fmt.Sprintf("REGISTRY_AUTH_HTPASSWD_PATH=/auth/%s", filename), + registryImage) + registry := fmt.Sprintf(`localhost:%d`, port) + command.Run(o, "login", registry, "-u", "testUser", "-p", "testPassword") + + homeDir, err := os.UserHomeDir() + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + configPath := fmt.Sprintf("%s/.finch/config.json", homeDir) + configContent, err := os.ReadFile(filepath.Clean(configPath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + + gomega.Expect(string(configContent)).Should(gomega.ContainSubstring(registry)) + command.Run(o, "logout", registry) + + configContent, err = os.ReadFile(filepath.Clean(configPath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(string(configContent)).ShouldNot(gomega.ContainSubstring(registry)) + }) + }) +} diff --git a/e2e/vm/vm_test.go b/e2e/vm/vm_test.go index b938df6e3..890ce2afb 100644 --- a/e2e/vm/vm_test.go +++ b/e2e/vm/vm_test.go @@ -41,6 +41,7 @@ func TestVM(t *testing.T) { testVMLifecycle(o) testAdditionalDisk(o) testConfig(o, *e2e.Installed) + testFinchConfigFile(o) testVersion(o) }) From 38fee416931ea5e43364429e316362ef9772ebec Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Fri, 24 Mar 2023 10:23:56 -0700 Subject: [PATCH 05/12] add comments to updateEnvironment Signed-off-by: Ang Zhou --- pkg/config/nerdctl_config_applier.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/config/nerdctl_config_applier.go b/pkg/config/nerdctl_config_applier.go index 3dcaa8cea..1dc033fe0 100644 --- a/pkg/config/nerdctl_config_applier.go +++ b/pkg/config/nerdctl_config_applier.go @@ -48,6 +48,9 @@ func NewNerdctlApplier(dialer fssh.Dialer, fs afero.Fs, privateKeyPath, hostUser // The [GNU docs for Bash] explain how these files work together in more details. // The default location of DOCKER_CONFIG is ~/.docker/config.json. This config change sets the location to // ~/.finch/config.json, but from the perspective of macOS (/Users//.finch/config.json). +// The reason that we don't set environment variables inside /root/.bashrc is that the vars inside it are +// not able to be picked up even if we specify `sudo -E`. We have to switch to root user in order to access them, while +// normally we would access the VM as the regular user. // For more information on the variable, see the registry nerdctl docs. // // [GNU docs for Bash]: https://www.gnu.org/software/bash/manual/html_node/Bash-Startup-Files.html From 49c4f85fd78f57101a916b2de42f59576e9c0297 Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Fri, 24 Mar 2023 12:48:41 -0700 Subject: [PATCH 06/12] clean up image and container Signed-off-by: Ang Zhou --- e2e/vm/finch_config_file_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/e2e/vm/finch_config_file_test.go b/e2e/vm/finch_config_file_test.go index 18a495b22..2fc11ec4a 100644 --- a/e2e/vm/finch_config_file_test.go +++ b/e2e/vm/finch_config_file_test.go @@ -20,9 +20,10 @@ import ( // is written to ~/.finch/config.json file. var testFinchConfigFile = func(o *option.Option) { ginkgo.Describe("finch config file", func() { - ginkgo.It("should store login credentials", func() { + ginkgo.FIt("should store login credentials", func() { filename := "htpasswd" registryImage := "public.ecr.aws/docker/library/registry:latest" + registryContainer := "registry" // The htpasswd is generated by // `finch run --entrypoint htpasswd public.ecr.aws/docker/library/httpd:2 -Bbn testUser testPassword`. // We don't want to generate it on the fly because: @@ -36,12 +37,14 @@ var testFinchConfigFile = func(o *option.Option) { port := fnet.GetFreePort() command.Run(o, "run", "-dp", fmt.Sprintf("%d:5000", port), - "--name", "registry", + "--name", registryContainer, "-v", fmt.Sprintf("%s:/auth", htpasswdDir), "-e", "REGISTRY_AUTH=htpasswd", "-e", "REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm", "-e", fmt.Sprintf("REGISTRY_AUTH_HTPASSWD_PATH=/auth/%s", filename), registryImage) + ginkgo.DeferCleanup(command.Run, o, "rmi", registryImage) + ginkgo.DeferCleanup(command.Run, o, "rm", "-f", registryContainer) registry := fmt.Sprintf(`localhost:%d`, port) command.Run(o, "login", registry, "-u", "testUser", "-p", "testPassword") From f37ec7141b270be8853888d9e14b9a3f41dff487 Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Fri, 24 Mar 2023 12:51:05 -0700 Subject: [PATCH 07/12] remove focus Signed-off-by: Ang Zhou --- e2e/vm/finch_config_file_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/vm/finch_config_file_test.go b/e2e/vm/finch_config_file_test.go index 2fc11ec4a..a5480bc38 100644 --- a/e2e/vm/finch_config_file_test.go +++ b/e2e/vm/finch_config_file_test.go @@ -20,7 +20,7 @@ import ( // is written to ~/.finch/config.json file. var testFinchConfigFile = func(o *option.Option) { ginkgo.Describe("finch config file", func() { - ginkgo.FIt("should store login credentials", func() { + ginkgo.It("should store login credentials", func() { filename := "htpasswd" registryImage := "public.ecr.aws/docker/library/registry:latest" registryContainer := "registry" From 95b2fa4a074ae03b1707e58d0d6c4643f739fbd9 Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Fri, 24 Mar 2023 12:54:23 -0700 Subject: [PATCH 08/12] add -E to version command Signed-off-by: Ang Zhou --- cmd/finch/version.go | 5 +++-- cmd/finch/version_test.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/finch/version.go b/cmd/finch/version.go index 79f362342..20c18d076 100644 --- a/cmd/finch/version.go +++ b/cmd/finch/version.go @@ -88,8 +88,9 @@ func (va *versionAction) printVersion() error { if status != lima.Running { return errors.New("detailed version info is unavailable because VM is not running") } - - limaArgs := []string{"shell", limaInstanceName, "sudo", "nerdctl", "version", "--format", "json"} + // Add -E to sudo command in order to preserve existing environment variables, more info: + // https://stackoverflow.com/questions/8633461/how-to-keep-environment-variables-when-using-sudo/8633575#8633575 + limaArgs := []string{"shell", limaInstanceName, "sudo", "-E", "nerdctl", "version", "--format", "json"} out, err := va.creator.CreateWithoutStdio(limaArgs...).Output() if err != nil { return fmt.Errorf("failed to create the nerdctl version command: %w", err) diff --git a/cmd/finch/version_test.go b/cmd/finch/version_test.go index 56b6b608f..f5f4c31f5 100644 --- a/cmd/finch/version_test.go +++ b/cmd/finch/version_test.go @@ -52,7 +52,7 @@ func TestVersionAction_runAdaptor(t *testing.T) { logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") command := mocks.NewCommand(ctrl) - lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "nerdctl", "version", "--format", "json").Return(command) + lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "-E", "nerdctl", "version", "--format", "json").Return(command) //nolint: lll // Version output format is larger than 500 command.EXPECT().Output().Return([]byte(`{"Client":{"Version":"v1.0.0","GitCommit":"c00780a1f5b905b09812722459c54936c9e070e6","GoVersion":"go1.19.2","Os":"linux","Arch":"arm64","Components":[{"Name":"buildctl","Version":"v0.10.5","Details":{"GitCommit":"bc26045116045516ff2427201abd299043eaf8f7"}}]},"Server":{"Components":[{"Name":"containerd","Version":"v1.6.8","Details":{"GitCommit":"9cd3357b7fd7218e4aec3eae239db1f68a5a6ec6"}},{"Name":"runc","Version":"1.1.4","Details":{"GitCommit":"v1.1.4-0-g5fd4c4d1"}}]}}`), nil) }, @@ -98,7 +98,7 @@ func TestVersionAction_run(t *testing.T) { logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") command := mocks.NewCommand(ctrl) - lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "nerdctl", "version", "--format", "json").Return(command) + lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "-E", "nerdctl", "version", "--format", "json").Return(command) command.EXPECT().Output().Return([]byte(`{ "Client":{ "Version":"v1.0.0", From 15f8630bd8df99f045f2310cc75e21943d94ae60 Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Fri, 24 Mar 2023 15:33:07 -0700 Subject: [PATCH 09/12] use different registry image Signed-off-by: Ang Zhou --- cmd/finch/version_test.go | 6 ++++-- e2e/vm/finch_config_file_test.go | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cmd/finch/version_test.go b/cmd/finch/version_test.go index f5f4c31f5..9ae52d9ee 100644 --- a/cmd/finch/version_test.go +++ b/cmd/finch/version_test.go @@ -52,7 +52,8 @@ func TestVersionAction_runAdaptor(t *testing.T) { logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") command := mocks.NewCommand(ctrl) - lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "-E", "nerdctl", "version", "--format", "json").Return(command) + lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "-E", "nerdctl", "version", + "--format", "json").Return(command) //nolint: lll // Version output format is larger than 500 command.EXPECT().Output().Return([]byte(`{"Client":{"Version":"v1.0.0","GitCommit":"c00780a1f5b905b09812722459c54936c9e070e6","GoVersion":"go1.19.2","Os":"linux","Arch":"arm64","Components":[{"Name":"buildctl","Version":"v0.10.5","Details":{"GitCommit":"bc26045116045516ff2427201abd299043eaf8f7"}}]},"Server":{"Components":[{"Name":"containerd","Version":"v1.6.8","Details":{"GitCommit":"9cd3357b7fd7218e4aec3eae239db1f68a5a6ec6"}},{"Name":"runc","Version":"1.1.4","Details":{"GitCommit":"v1.1.4-0-g5fd4c4d1"}}]}}`), nil) }, @@ -98,7 +99,8 @@ func TestVersionAction_run(t *testing.T) { logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") command := mocks.NewCommand(ctrl) - lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "-E", "nerdctl", "version", "--format", "json").Return(command) + lcc.EXPECT().CreateWithoutStdio("shell", limaInstanceName, "sudo", "-E", "nerdctl", "version", + "--format", "json").Return(command) command.EXPECT().Output().Return([]byte(`{ "Client":{ "Version":"v1.0.0", diff --git a/e2e/vm/finch_config_file_test.go b/e2e/vm/finch_config_file_test.go index a5480bc38..151cce44d 100644 --- a/e2e/vm/finch_config_file_test.go +++ b/e2e/vm/finch_config_file_test.go @@ -22,8 +22,8 @@ var testFinchConfigFile = func(o *option.Option) { ginkgo.Describe("finch config file", func() { ginkgo.It("should store login credentials", func() { filename := "htpasswd" - registryImage := "public.ecr.aws/docker/library/registry:latest" - registryContainer := "registry" + registryImage := "public.ecr.aws/docker/library/registry:2" + registryContainer := "auth-registry" // The htpasswd is generated by // `finch run --entrypoint htpasswd public.ecr.aws/docker/library/httpd:2 -Bbn testUser testPassword`. // We don't want to generate it on the fly because: @@ -45,6 +45,8 @@ var testFinchConfigFile = func(o *option.Option) { registryImage) ginkgo.DeferCleanup(command.Run, o, "rmi", registryImage) ginkgo.DeferCleanup(command.Run, o, "rm", "-f", registryContainer) + command.Run(o, "images") + command.Run(o, "ps", "-a") registry := fmt.Sprintf(`localhost:%d`, port) command.Run(o, "login", registry, "-u", "testUser", "-p", "testPassword") From 1d18c464964cd3048ce77ad1653ce356f7cfbe7b Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Tue, 28 Mar 2023 09:39:17 -0700 Subject: [PATCH 10/12] add sleep time Signed-off-by: Ang Zhou --- e2e/vm/finch_config_file_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/vm/finch_config_file_test.go b/e2e/vm/finch_config_file_test.go index 151cce44d..fa9740e52 100644 --- a/e2e/vm/finch_config_file_test.go +++ b/e2e/vm/finch_config_file_test.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "time" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -45,8 +46,7 @@ var testFinchConfigFile = func(o *option.Option) { registryImage) ginkgo.DeferCleanup(command.Run, o, "rmi", registryImage) ginkgo.DeferCleanup(command.Run, o, "rm", "-f", registryContainer) - command.Run(o, "images") - command.Run(o, "ps", "-a") + time.Sleep(2 * time.Second) registry := fmt.Sprintf(`localhost:%d`, port) command.Run(o, "login", registry, "-u", "testUser", "-p", "testPassword") From 037636dd285dae17c8a64c78b7413e8e3bc30a8a Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Tue, 28 Mar 2023 10:19:07 -0700 Subject: [PATCH 11/12] resolve conflict Signed-off-by: Ang Zhou --- cmd/finch/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/finch/main.go b/cmd/finch/main.go index 2072bd0d5..13ed92f23 100644 --- a/cmd/finch/main.go +++ b/cmd/finch/main.go @@ -111,13 +111,12 @@ func virtualMachineCommands( fc *config.Finch, ) *cobra.Command { optionalDepGroups := []*dependency.Group{vmnet.NewDependencyGroup(ecc, lcc, fs, fp, logger)} - stdLib := system.NewStdLib() return newVirtualMachineCommand( lcc, logger, optionalDepGroups, config.NewLimaApplier(fc, ecc, fs, fp.LimaOverrideConfigPath(), system.NewStdLib()), - config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), stdLib.Env("USER")), + config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), system.NewStdLib().Env("USER")), fp, fs, disk.NewUserDataDiskManager(lcc, ecc, &afero.OsFs{}, fp, system.NewStdLib().Env("HOME"), fc), From 99b1918083f53654180f5a2a64e6c366db2faef0 Mon Sep 17 00:00:00 2001 From: Ang Zhou Date: Tue, 28 Mar 2023 14:12:26 -0700 Subject: [PATCH 12/12] address comments Signed-off-by: Ang Zhou --- e2e/vm/finch_config_file_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e/vm/finch_config_file_test.go b/e2e/vm/finch_config_file_test.go index fa9740e52..f1bd02c68 100644 --- a/e2e/vm/finch_config_file_test.go +++ b/e2e/vm/finch_config_file_test.go @@ -36,7 +36,7 @@ var testFinchConfigFile = func(o *option.Option) { htpasswdDir := filepath.Dir(ffs.CreateTempFile(filename, htpasswd)) ginkgo.DeferCleanup(os.RemoveAll, htpasswdDir) port := fnet.GetFreePort() - command.Run(o, "run", + containerID := command.StdoutStr(o, "run", "-dp", fmt.Sprintf("%d:5000", port), "--name", registryContainer, "-v", fmt.Sprintf("%s:/auth", htpasswdDir), @@ -46,7 +46,9 @@ var testFinchConfigFile = func(o *option.Option) { registryImage) ginkgo.DeferCleanup(command.Run, o, "rmi", registryImage) ginkgo.DeferCleanup(command.Run, o, "rm", "-f", registryContainer) - time.Sleep(2 * time.Second) + for command.StdoutStr(o, "inspect", "-f", "{{.State.Running}}", containerID) != "true" { + time.Sleep(1 * time.Second) + } registry := fmt.Sprintf(`localhost:%d`, port) command.Run(o, "login", registry, "-u", "testUser", "-p", "testPassword")