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.

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

fix: Env variable CONTAINERD_SNAPSHOTTER cleared on overlayfs and ref… #816

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/virtual_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func virtualMachineCommands(
lcc,
Shubhranshu153 marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand Down
11 changes: 8 additions & 3 deletions cmd/finch/virtual_machine_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
54 changes: 46 additions & 8 deletions cmd/finch/virtual_machine_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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")
Expand All @@ -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"),
Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion cmd/finch/virtual_machine_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
44 changes: 38 additions & 6 deletions cmd/finch/virtual_machine_start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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")
Expand All @@ -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",
Expand Down Expand Up @@ -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)

Expand Down
4 changes: 1 addition & 3 deletions docs/design/config_to_support_additional_directories.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion e2e/vm/additional_disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 25 additions & 3 deletions e2e/vm/config_darwin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package vm

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

Expand All @@ -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() {
Expand All @@ -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))
Expand Down
Loading
Loading