fix: Env variable CONTAINERD_SNAPSHOTTER cleared on overlayfs and ref… (

#816) Env variable CONTAINERD_SNAPSHOTTER cleared on overlayfs and refactoring lima config files. Issue #, if available: Testing done: unit test Add soci and check pull and push operation work with soci. Add soci and check build works for soci Add overlayfs and check it clears soci configs and sets everything to default. [x ] I've reviewed the guidance in CONTRIBUTING.md - [x] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Shubharanshu Mahapatra <[email protected]>
runfinch · Feb 16, 2024 · d222a34 · d222a34
1 parent 13de2b9
commit d222a34
Show file tree

Hide file tree

Showing 29 changed files with 648 additions and 633 deletions.
diff --git a/cmd/finch/virtual_machine.go b/cmd/finch/virtual_machine.go
@@ -109,7 +109,7 @@ func virtualMachineCommands(
 		lcc,
 		logger,
 		dependencies(ecc, fc, fp, fs, lcc, logger, fp.FinchDir(finchRootPath)),
-		config.NewLimaApplier(fc, ecc, fs, fp.LimaOverrideConfigPath(), system.NewStdLib()),
+		config.NewLimaApplier(fc, ecc, fs, fp.LimaDefaultConfigPath(), fp.LimaOverrideConfigPath(), system.NewStdLib()),
 		config.NewNerdctlApplier(
 			fssh.NewDialer(),
 			fs,

diff --git a/cmd/finch/virtual_machine_init.go b/cmd/finch/virtual_machine_init.go
@@ -76,16 +76,21 @@ func (iva *initVMAction) run() error {
 		return err
 	}
 
-	err = dependency.InstallOptionalDeps(iva.optionalDepGroups, iva.logger)
+	err = iva.limaConfigApplier.ConfigureDefaultLimaYaml()
 	if err != nil {
-		iva.logger.Errorf("Dependency error: %v", err)
+		return err
 	}
 
-	err = iva.limaConfigApplier.Apply(true)
+	err = iva.limaConfigApplier.ConfigureOverrideLimaYaml()
 	if err != nil {
 		return err
 	}
 
+	err = dependency.InstallOptionalDeps(iva.optionalDepGroups, iva.logger)
+	if err != nil {
+		iva.logger.Errorf("Dependency error: %v", err)
+	}
+
 	// ignore error, this is to ensure that the disk is only mounted once
 	_ = iva.diskManager.DetachUserDataDisk()
 

diff --git a/cmd/finch/virtual_machine_init_test.go b/cmd/finch/virtual_machine_init_test.go
@@ -72,7 +72,8 @@ func TestInitVMAction_runAdapter(t *testing.T) {
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "")
 
 				command := mocks.NewCommand(ctrl)
-				lca.EXPECT().Apply(true).Return(nil)
+				lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
 				dm.EXPECT().DetachUserDataDisk().Return(nil)
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 				lcc.EXPECT().CreateWithoutStdio("start", fmt.Sprintf("--name=%s", limaInstanceName),
@@ -137,7 +138,8 @@ func TestInitVMAction_run(t *testing.T) {
 				getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "")
 
-				lca.EXPECT().Apply(true).Return(nil)
+				lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
 				dm.EXPECT().DetachUserDataDisk().Return(nil)
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 
@@ -228,11 +230,10 @@ func TestInitVMAction_run(t *testing.T) {
 			},
 		},
 		{
-			// TODO: split this test case up:
 			// should succeed even if some optional dependencies fail to be installed
 			// return an error if Lima config fails to be applied
-			name:    "should print out error if InstallOptionalDeps fails and return error if LoadAndApplyLimaConfig fails",
-			wantErr: errors.New("load config fails"),
+			name:    "should print out error if InstallOptionalDeps fails",
+			wantErr: nil,
 			groups: func(ctrl *gomock.Controller) []*dependency.Group {
 				dep := mocks.NewDependency(ctrl)
 				deps := dependency.NewGroup([]dependency.Dependency{dep}, "", "mock_error_msg")
@@ -248,22 +249,58 @@ func TestInitVMAction_run(t *testing.T) {
 				lcc *mocks.LimaCmdCreator,
 				logger *mocks.Logger,
 				lca *mocks.LimaConfigApplier,
-				_ *mocks.UserDataDiskManager,
+				dm *mocks.UserDataDiskManager,
 				ctrl *gomock.Controller,
 			) {
 				getVMStatusC := mocks.NewCommand(ctrl)
 				lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
 				getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "")
 
-				lca.EXPECT().Apply(true).Return(errors.New("load config fails"))
+				lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
+
 				logger.EXPECT().Errorf("Dependency error: %v",
 					fmt.Errorf("failed to install dependencies: %w",
 						errors.Join(fmt.Errorf("%s: %w", "mock_error_msg", errors.Join(errors.New("dependency error occurs")))),
 					),
 				)
+				dm.EXPECT().DetachUserDataDisk().Return(nil)
+				dm.EXPECT().EnsureUserDataDisk().Return(nil)
+
+				command := mocks.NewCommand(ctrl)
+				lcc.EXPECT().CreateWithoutStdio("start", fmt.Sprintf("--name=%s", limaInstanceName),
+					mockBaseYamlFilePath, "--tty=false").Return(command)
+				command.EXPECT().CombinedOutput()
+
+				logger.EXPECT().Info("Initializing and starting Finch virtual machine...")
+				logger.EXPECT().Info("Finch virtual machine started successfully")
 			},
 		},
+		{
+			// should succeed even if some optional dependencies fail to be installed
+			// return an error if Lima config fails to be applied
+			name:    "return error if LoadAndApplyLimaConfig fails",
+			wantErr: errors.New("load config fails"),
+			groups: func(_ *gomock.Controller) []*dependency.Group {
+				return nil
+			},
+			mockSvc: func(
+				lcc *mocks.LimaCmdCreator,
+				logger *mocks.Logger,
+				lca *mocks.LimaConfigApplier,
+				_ *mocks.UserDataDiskManager,
+				ctrl *gomock.Controller,
+			) {
+				getVMStatusC := mocks.NewCommand(ctrl)
+				lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
+				getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
+				logger.EXPECT().Debugf("Status of virtual machine: %s", "")
+
+				lca.EXPECT().ConfigureDefaultLimaYaml().Return(errors.New("load config fails"))
+			},
+		},
+
 		{
 			name:    "should print error if instance fails to initialize",
 			wantErr: errors.New("failed to init instance"),
@@ -282,7 +319,8 @@ func TestInitVMAction_run(t *testing.T) {
 				getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "")
 
-				lca.EXPECT().Apply(true).Return(nil)
+				lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
 				dm.EXPECT().DetachUserDataDisk().Return(nil)
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 

diff --git a/cmd/finch/virtual_machine_start.go b/cmd/finch/virtual_machine_start.go
@@ -74,7 +74,7 @@ func (sva *startVMAction) run() error {
 		sva.logger.Errorf("Dependency error: %v", err)
 	}
 
-	err = sva.limaConfigApplier.Apply(false)
+	err = sva.limaConfigApplier.ConfigureOverrideLimaYaml()
 	if err != nil {
 		return err
 	}

diff --git a/cmd/finch/virtual_machine_start_test.go b/cmd/finch/virtual_machine_start_test.go
@@ -71,7 +71,7 @@ func TestStartVMAction_runAdapter(t *testing.T) {
 				getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
 
-				lca.EXPECT().Apply(false).Return(nil)
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
 
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 
@@ -146,7 +146,7 @@ func TestStartVMAction_run(t *testing.T) {
 				getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
 
-				lca.EXPECT().Apply(false).Return(nil)
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
 
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 
@@ -238,8 +238,31 @@ func TestStartVMAction_run(t *testing.T) {
 			// TODO: split this test case up:
 			// should succeed even if some optional dependencies fail to be installed
 			// return an error if Lima config fails to be applied
-			name:    "should print out error if InstallOptionalDeps fails and return error if LoadAndApplyLimaConfig fails",
+			name:    "should return error if LoadAndApplyLimaConfig fails",
 			wantErr: errors.New("load config fails"),
+			groups: func(_ *gomock.Controller) []*dependency.Group {
+				return nil
+			},
+			mockSvc: func(
+				lcc *mocks.LimaCmdCreator,
+				logger *mocks.Logger,
+				lca *mocks.LimaConfigApplier,
+				_ *mocks.UserDataDiskManager,
+				ctrl *gomock.Controller,
+			) {
+				getVMStatusC := mocks.NewCommand(ctrl)
+				lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
+				getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
+				logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
+
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(errors.New("load config fails"))
+			},
+		},
+		{
+			// should succeed even if some optional dependencies fail to be installed
+			// return an error if Lima config fails to be applied
+			name:    "should print out error if InstallOptionalDeps fails",
+			wantErr: nil,
 			groups: func(ctrl *gomock.Controller) []*dependency.Group {
 				dep := mocks.NewDependency(ctrl)
 				deps := dependency.NewGroup([]dependency.Dependency{dep}, "", "mock_error_msg")
@@ -255,15 +278,24 @@ func TestStartVMAction_run(t *testing.T) {
 				lcc *mocks.LimaCmdCreator,
 				logger *mocks.Logger,
 				lca *mocks.LimaConfigApplier,
-				_ *mocks.UserDataDiskManager,
+				dm *mocks.UserDataDiskManager,
 				ctrl *gomock.Controller,
 			) {
 				getVMStatusC := mocks.NewCommand(ctrl)
 				lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
 				getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
 
-				lca.EXPECT().Apply(false).Return(errors.New("load config fails"))
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
+
+				dm.EXPECT().EnsureUserDataDisk().Return(nil)
+
+				command := mocks.NewCommand(ctrl)
+				command.EXPECT().CombinedOutput()
+				lcc.EXPECT().CreateWithoutStdio("start", limaInstanceName).Return(command)
+
+				logger.EXPECT().Info("Starting existing Finch virtual machine...")
+				logger.EXPECT().Info("Finch virtual machine started successfully")
 
 				logger.EXPECT().Errorf("Dependency error: %v",
 					fmt.Errorf("failed to install dependencies: %w",
@@ -296,7 +328,7 @@ func TestStartVMAction_run(t *testing.T) {
 				getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
 				logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
 
-				lca.EXPECT().Apply(false).Return(nil)
+				lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
 
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 

diff --git a/docs/design/config_to_support_additional_directories.md b/docs/design/config_to_support_additional_directories.md
@@ -6,7 +6,7 @@
 
 ## Approach
 
-The [finch.yaml](https://github.com/runfinch/finch/blob/d8174ff773f0f92ec94d6d97c753a872a98f74a0/finch.yaml#L35) file which is used to boot in Lima has Mounts field to handle the mount points. However, changing it in finch.yaml would make the configs only applied in `vm init`, and finch.yaml is expected to be the place to keep Finch's default config without being messed up with user's customized configs. So instead of adding to finch.yaml, I recommended adding additional_directories to Lima’s override.yaml file. Both `vm init` and `vm start` can apply the configs in override.yaml. This is same to how Finch applies cpu and memory configs today.
+The [finch.yaml](https://github.com/runfinch/finch/blob/d8174ff773f0f92ec94d6d97c753a872a98f74a0/finch.yaml#L35) file which is used to boot in Lima has Mounts field to handle the mount points. Mount Points added in finch.yaml will configure override.yaml on `vm init` and `vm start`.
 
 For example, for Finch config:
 
@@ -36,8 +36,6 @@ sshfs:
     protocolVersion: 9p2000.L
     msize: 128KiB
     cache: mmap
-networks:
-- lima: finch-shared
 ```
 
 Different to cpu and memory, the “mounts” field in override.yaml will be appended to the default mounts instead of replacing it. So we don’t have to add the default home directory to the override.yaml file. [Reference](https://github.com/lima-vm/lima/blob/585d6e25af62d0337cec83ffca226a2c8146a428/pkg/limayaml/defaults.go#L410)

diff --git a/e2e/vm/additional_disk_test.go b/e2e/vm/additional_disk_test.go
@@ -23,7 +23,7 @@ const (
 var testAdditionalDisk = func(o *option.Option, installed bool) {
 	ginkgo.Describe("Additional disk", ginkgo.Serial, func() {
 		ginkgo.It("Retains container user data after the VM is deleted", func() {
-			resetVM(o, installed)
+			resetVM(o)
 			resetDisks(o, installed)
 			command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(160).Run()
 			command.Run(o, "volume", "create", volumeName)

diff --git a/e2e/vm/config_darwin_test.go b/e2e/vm/config_darwin_test.go
@@ -6,6 +6,7 @@ package vm
 
 import (
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 
@@ -16,12 +17,25 @@ import (
 	"github.com/runfinch/common-tests/option"
 	"gopkg.in/yaml.v3"
 
+	"github.com/runfinch/finch/e2e"
 	finch_cmd "github.com/runfinch/finch/pkg/command"
 	"github.com/runfinch/finch/pkg/config"
 )
 
 var finchConfigFilePath = os.Getenv("HOME") + "/.finch/finch.yaml"
 
+func limaDataDirPath(installed bool) string {
+	limaConfigFilePath := defaultLimaDataDirPath
+	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")
+	}
+	return limaConfigFilePath
+}
+
 var testConfig = func(o *option.Option, installed bool) {
 	ginkgo.Describe("Config (after init)", ginkgo.Serial, func() {
 		ginkgo.It("updates init-only config values when values are changed after init", func() {
@@ -31,21 +45,29 @@ var testConfig = func(o *option.Option, installed bool) {
 				ginkgo.Skip("Skipping because existing init only configuration options require Virtualization.framework support to test")
 			}
 
-			limaConfigFilePath := resetVM(o, installed)
+			resetVM(o)
 			resetDisks(o, installed)
 			writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 6\nvmType: vz\nrosetta: false"))
 			// vm init with VZ set sometimes takes 2 minutes just to convert the disk to raw
 			command.New(o, "vm", "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run()
 
-			gomega.Expect(limaConfigFilePath).Should(gomega.BeARegularFile())
-			cfgBuf, err := os.ReadFile(filepath.Clean(limaConfigFilePath))
+			overrideConfigFilePath := filepath.Join(limaDataDirPath(installed), "_config", "override.yaml")
+			gomega.Expect(overrideConfigFilePath).Should(gomega.BeARegularFile())
+			cfgBuf, err := os.ReadFile(filepath.Clean(overrideConfigFilePath))
 			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
 
 			var limaCfg limayaml.LimaYAML
 			err = yaml.Unmarshal(cfgBuf, &limaCfg)
 			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
 			gomega.Expect(*limaCfg.CPUs).Should(gomega.Equal(6))
 			gomega.Expect(*limaCfg.Memory).Should(gomega.Equal("4GiB"))
+
+			defaultConfigFilePath := filepath.Join(limaDataDirPath(installed), "_config", "default.yaml")
+			gomega.Expect(defaultConfigFilePath).Should(gomega.BeARegularFile())
+			cfgBuf, err = os.ReadFile(filepath.Clean(defaultConfigFilePath))
+			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+			err = yaml.Unmarshal(cfgBuf, &limaCfg)
+			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
 			gomega.Expect(*limaCfg.VMType).Should(gomega.Equal("vz"))
 			gomega.Expect(*limaCfg.Rosetta.Enabled).Should(gomega.Equal(false))
 			gomega.Expect(*limaCfg.Rosetta.BinFmt).Should(gomega.Equal(false))