fix: Env variable CONTAINERD_SNAPSHOTTER cleared on overlayfs and ref…

…actoring of override.yaml operations

Signed-off-by: Shubharanshu Mahapatra <[email protected]>

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,

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()
 

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)
 

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
 	}

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)
 

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)

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)

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))