fix: Env variable CONTAINERD_SNAPSHOTTER cleared on overlayfs and ref…

…actoring of override.yaml operations Signed-off-by: Shubharanshu Mahapatra <[email protected]>
runfinch · Feb 16, 2024 · 0226148 · 0226148
1 parent 6188a2c
commit 0226148
Show file tree

Hide file tree

Showing 27 changed files with 628 additions and 616 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,14 +76,14 @@ func (iva *initVMAction) run() error {
 		return err
 	}
 
-	err = dependency.InstallOptionalDeps(iva.optionalDepGroups, iva.logger)
+	err = iva.limaConfigApplier.ApplyInit()
 	if err != nil {
-		iva.logger.Errorf("Dependency error: %v", err)
+		return err
 	}
 
-	err = iva.limaConfigApplier.Apply(true)
+	err = dependency.InstallOptionalDeps(iva.optionalDepGroups, iva.logger)
 	if err != nil {
-		return err
+		iva.logger.Errorf("Dependency error: %v", err)
 	}
 
 	// ignore error, this is to ensure that the disk is only mounted once

diff --git a/cmd/finch/virtual_machine_init_test.go b/cmd/finch/virtual_machine_init_test.go
@@ -72,7 +72,7 @@ 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().ApplyInit().Return(nil)
 				dm.EXPECT().DetachUserDataDisk().Return(nil)
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 				lcc.EXPECT().CreateWithoutStdio("start", fmt.Sprintf("--name=%s", limaInstanceName),
@@ -137,7 +137,7 @@ 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().ApplyInit().Return(nil)
 				dm.EXPECT().DetachUserDataDisk().Return(nil)
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 
@@ -228,11 +228,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 +247,57 @@ 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().ApplyInit().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().ApplyInit().Return(errors.New("load config fails"))
+			},
+		},
+
 		{
 			name:    "should print error if instance fails to initialize",
 			wantErr: errors.New("failed to init instance"),
@@ -282,7 +316,7 @@ 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().ApplyInit().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.Apply()
 	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().Apply().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().Apply().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().Apply().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().Apply().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().Apply().Return(nil)
 
 				dm.EXPECT().EnsureUserDataDisk().Return(nil)
 

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