Skip to content

Commit d222a34

Browse files
Shubhranshu153Shubhranshu153
authored
fix: Env variable CONTAINERD_SNAPSHOTTER cleared on overlayfs and ref… (runfinch#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]>
1 parent 13de2b9 commit d222a34

29 files changed

+648
-633
lines changed

Diff for: ‎cmd/finch/virtual_machine.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func virtualMachineCommands(
109109
lcc,
110110
logger,
111111
dependencies(ecc, fc, fp, fs, lcc, logger, fp.FinchDir(finchRootPath)),
112-
config.NewLimaApplier(fc, ecc, fs, fp.LimaOverrideConfigPath(), system.NewStdLib()),
112+
config.NewLimaApplier(fc, ecc, fs, fp.LimaDefaultConfigPath(), fp.LimaOverrideConfigPath(), system.NewStdLib()),
113113
config.NewNerdctlApplier(
114114
fssh.NewDialer(),
115115
fs,

Diff for: ‎cmd/finch/virtual_machine_init.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,21 @@ func (iva *initVMAction) run() error {
7676
return err
7777
}
7878

79-
err = dependency.InstallOptionalDeps(iva.optionalDepGroups, iva.logger)
79+
err = iva.limaConfigApplier.ConfigureDefaultLimaYaml()
8080
if err != nil {
81-
iva.logger.Errorf("Dependency error: %v", err)
81+
return err
8282
}
8383

84-
err = iva.limaConfigApplier.Apply(true)
84+
err = iva.limaConfigApplier.ConfigureOverrideLimaYaml()
8585
if err != nil {
8686
return err
8787
}
8888

89+
err = dependency.InstallOptionalDeps(iva.optionalDepGroups, iva.logger)
90+
if err != nil {
91+
iva.logger.Errorf("Dependency error: %v", err)
92+
}
93+
8994
// ignore error, this is to ensure that the disk is only mounted once
9095
_ = iva.diskManager.DetachUserDataDisk()
9196

Diff for: ‎cmd/finch/virtual_machine_init_test.go

+46-8
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ func TestInitVMAction_runAdapter(t *testing.T) {
7272
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
7373

7474
command := mocks.NewCommand(ctrl)
75-
lca.EXPECT().Apply(true).Return(nil)
75+
lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
76+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
7677
dm.EXPECT().DetachUserDataDisk().Return(nil)
7778
dm.EXPECT().EnsureUserDataDisk().Return(nil)
7879
lcc.EXPECT().CreateWithoutStdio("start", fmt.Sprintf("--name=%s", limaInstanceName),
@@ -137,7 +138,8 @@ func TestInitVMAction_run(t *testing.T) {
137138
getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
138139
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
139140

140-
lca.EXPECT().Apply(true).Return(nil)
141+
lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
142+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
141143
dm.EXPECT().DetachUserDataDisk().Return(nil)
142144
dm.EXPECT().EnsureUserDataDisk().Return(nil)
143145

@@ -228,11 +230,10 @@ func TestInitVMAction_run(t *testing.T) {
228230
},
229231
},
230232
{
231-
// TODO: split this test case up:
232233
// should succeed even if some optional dependencies fail to be installed
233234
// return an error if Lima config fails to be applied
234-
name: "should print out error if InstallOptionalDeps fails and return error if LoadAndApplyLimaConfig fails",
235-
wantErr: errors.New("load config fails"),
235+
name: "should print out error if InstallOptionalDeps fails",
236+
wantErr: nil,
236237
groups: func(ctrl *gomock.Controller) []*dependency.Group {
237238
dep := mocks.NewDependency(ctrl)
238239
deps := dependency.NewGroup([]dependency.Dependency{dep}, "", "mock_error_msg")
@@ -248,22 +249,58 @@ func TestInitVMAction_run(t *testing.T) {
248249
lcc *mocks.LimaCmdCreator,
249250
logger *mocks.Logger,
250251
lca *mocks.LimaConfigApplier,
251-
_ *mocks.UserDataDiskManager,
252+
dm *mocks.UserDataDiskManager,
252253
ctrl *gomock.Controller,
253254
) {
254255
getVMStatusC := mocks.NewCommand(ctrl)
255256
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
256257
getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
257258
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
258259

259-
lca.EXPECT().Apply(true).Return(errors.New("load config fails"))
260+
lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
261+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
262+
260263
logger.EXPECT().Errorf("Dependency error: %v",
261264
fmt.Errorf("failed to install dependencies: %w",
262265
errors.Join(fmt.Errorf("%s: %w", "mock_error_msg", errors.Join(errors.New("dependency error occurs")))),
263266
),
264267
)
268+
dm.EXPECT().DetachUserDataDisk().Return(nil)
269+
dm.EXPECT().EnsureUserDataDisk().Return(nil)
270+
271+
command := mocks.NewCommand(ctrl)
272+
lcc.EXPECT().CreateWithoutStdio("start", fmt.Sprintf("--name=%s", limaInstanceName),
273+
mockBaseYamlFilePath, "--tty=false").Return(command)
274+
command.EXPECT().CombinedOutput()
275+
276+
logger.EXPECT().Info("Initializing and starting Finch virtual machine...")
277+
logger.EXPECT().Info("Finch virtual machine started successfully")
265278
},
266279
},
280+
{
281+
// should succeed even if some optional dependencies fail to be installed
282+
// return an error if Lima config fails to be applied
283+
name: "return error if LoadAndApplyLimaConfig fails",
284+
wantErr: errors.New("load config fails"),
285+
groups: func(_ *gomock.Controller) []*dependency.Group {
286+
return nil
287+
},
288+
mockSvc: func(
289+
lcc *mocks.LimaCmdCreator,
290+
logger *mocks.Logger,
291+
lca *mocks.LimaConfigApplier,
292+
_ *mocks.UserDataDiskManager,
293+
ctrl *gomock.Controller,
294+
) {
295+
getVMStatusC := mocks.NewCommand(ctrl)
296+
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
297+
getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
298+
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
299+
300+
lca.EXPECT().ConfigureDefaultLimaYaml().Return(errors.New("load config fails"))
301+
},
302+
},
303+
267304
{
268305
name: "should print error if instance fails to initialize",
269306
wantErr: errors.New("failed to init instance"),
@@ -282,7 +319,8 @@ func TestInitVMAction_run(t *testing.T) {
282319
getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
283320
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
284321

285-
lca.EXPECT().Apply(true).Return(nil)
322+
lca.EXPECT().ConfigureDefaultLimaYaml().Return(nil)
323+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
286324
dm.EXPECT().DetachUserDataDisk().Return(nil)
287325
dm.EXPECT().EnsureUserDataDisk().Return(nil)
288326

Diff for: ‎cmd/finch/virtual_machine_start.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (sva *startVMAction) run() error {
7474
sva.logger.Errorf("Dependency error: %v", err)
7575
}
7676

77-
err = sva.limaConfigApplier.Apply(false)
77+
err = sva.limaConfigApplier.ConfigureOverrideLimaYaml()
7878
if err != nil {
7979
return err
8080
}

Diff for: ‎cmd/finch/virtual_machine_start_test.go

+38-6
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestStartVMAction_runAdapter(t *testing.T) {
7171
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
7272
logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
7373

74-
lca.EXPECT().Apply(false).Return(nil)
74+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
7575

7676
dm.EXPECT().EnsureUserDataDisk().Return(nil)
7777

@@ -146,7 +146,7 @@ func TestStartVMAction_run(t *testing.T) {
146146
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
147147
logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
148148

149-
lca.EXPECT().Apply(false).Return(nil)
149+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
150150

151151
dm.EXPECT().EnsureUserDataDisk().Return(nil)
152152

@@ -238,8 +238,31 @@ func TestStartVMAction_run(t *testing.T) {
238238
// TODO: split this test case up:
239239
// should succeed even if some optional dependencies fail to be installed
240240
// return an error if Lima config fails to be applied
241-
name: "should print out error if InstallOptionalDeps fails and return error if LoadAndApplyLimaConfig fails",
241+
name: "should return error if LoadAndApplyLimaConfig fails",
242242
wantErr: errors.New("load config fails"),
243+
groups: func(_ *gomock.Controller) []*dependency.Group {
244+
return nil
245+
},
246+
mockSvc: func(
247+
lcc *mocks.LimaCmdCreator,
248+
logger *mocks.Logger,
249+
lca *mocks.LimaConfigApplier,
250+
_ *mocks.UserDataDiskManager,
251+
ctrl *gomock.Controller,
252+
) {
253+
getVMStatusC := mocks.NewCommand(ctrl)
254+
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
255+
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
256+
logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
257+
258+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(errors.New("load config fails"))
259+
},
260+
},
261+
{
262+
// should succeed even if some optional dependencies fail to be installed
263+
// return an error if Lima config fails to be applied
264+
name: "should print out error if InstallOptionalDeps fails",
265+
wantErr: nil,
243266
groups: func(ctrl *gomock.Controller) []*dependency.Group {
244267
dep := mocks.NewDependency(ctrl)
245268
deps := dependency.NewGroup([]dependency.Dependency{dep}, "", "mock_error_msg")
@@ -255,15 +278,24 @@ func TestStartVMAction_run(t *testing.T) {
255278
lcc *mocks.LimaCmdCreator,
256279
logger *mocks.Logger,
257280
lca *mocks.LimaConfigApplier,
258-
_ *mocks.UserDataDiskManager,
281+
dm *mocks.UserDataDiskManager,
259282
ctrl *gomock.Controller,
260283
) {
261284
getVMStatusC := mocks.NewCommand(ctrl)
262285
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
263286
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
264287
logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
265288

266-
lca.EXPECT().Apply(false).Return(errors.New("load config fails"))
289+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
290+
291+
dm.EXPECT().EnsureUserDataDisk().Return(nil)
292+
293+
command := mocks.NewCommand(ctrl)
294+
command.EXPECT().CombinedOutput()
295+
lcc.EXPECT().CreateWithoutStdio("start", limaInstanceName).Return(command)
296+
297+
logger.EXPECT().Info("Starting existing Finch virtual machine...")
298+
logger.EXPECT().Info("Finch virtual machine started successfully")
267299

268300
logger.EXPECT().Errorf("Dependency error: %v",
269301
fmt.Errorf("failed to install dependencies: %w",
@@ -296,7 +328,7 @@ func TestStartVMAction_run(t *testing.T) {
296328
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
297329
logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")
298330

299-
lca.EXPECT().Apply(false).Return(nil)
331+
lca.EXPECT().ConfigureOverrideLimaYaml().Return(nil)
300332

301333
dm.EXPECT().EnsureUserDataDisk().Return(nil)
302334

Diff for: ‎docs/design/config_to_support_additional_directories.md

+1-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
## Approach
88

9-
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.
9+
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`.
1010

1111
For example, for Finch config:
1212

@@ -36,8 +36,6 @@ sshfs:
3636
protocolVersion: 9p2000.L
3737
msize: 128KiB
3838
cache: mmap
39-
networks:
40-
- lima: finch-shared
4139
```
4240
4341
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 for: ‎e2e/vm/additional_disk_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const (
2323
var testAdditionalDisk = func(o *option.Option, installed bool) {
2424
ginkgo.Describe("Additional disk", ginkgo.Serial, func() {
2525
ginkgo.It("Retains container user data after the VM is deleted", func() {
26-
resetVM(o, installed)
26+
resetVM(o)
2727
resetDisks(o, installed)
2828
command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(160).Run()
2929
command.Run(o, "volume", "create", volumeName)

Diff for: ‎e2e/vm/config_darwin_test.go

+25-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package vm
66

77
import (
88
"os"
9+
"os/exec"
910
"path/filepath"
1011
"runtime"
1112

@@ -16,12 +17,25 @@ import (
1617
"github.com/runfinch/common-tests/option"
1718
"gopkg.in/yaml.v3"
1819

20+
"github.com/runfinch/finch/e2e"
1921
finch_cmd "github.com/runfinch/finch/pkg/command"
2022
"github.com/runfinch/finch/pkg/config"
2123
)
2224

2325
var finchConfigFilePath = os.Getenv("HOME") + "/.finch/finch.yaml"
2426

27+
func limaDataDirPath(installed bool) string {
28+
limaConfigFilePath := defaultLimaDataDirPath
29+
if installed {
30+
path, err := exec.LookPath(e2e.InstalledTestSubject)
31+
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
32+
realFinchPath, err := filepath.EvalSymlinks(path)
33+
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
34+
limaConfigFilePath = filepath.Join(realFinchPath, "..", "..", "lima", "data")
35+
}
36+
return limaConfigFilePath
37+
}
38+
2539
var testConfig = func(o *option.Option, installed bool) {
2640
ginkgo.Describe("Config (after init)", ginkgo.Serial, func() {
2741
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) {
3145
ginkgo.Skip("Skipping because existing init only configuration options require Virtualization.framework support to test")
3246
}
3347

34-
limaConfigFilePath := resetVM(o, installed)
48+
resetVM(o)
3549
resetDisks(o, installed)
3650
writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 6\nvmType: vz\nrosetta: false"))
3751
// vm init with VZ set sometimes takes 2 minutes just to convert the disk to raw
3852
command.New(o, "vm", "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run()
3953

40-
gomega.Expect(limaConfigFilePath).Should(gomega.BeARegularFile())
41-
cfgBuf, err := os.ReadFile(filepath.Clean(limaConfigFilePath))
54+
overrideConfigFilePath := filepath.Join(limaDataDirPath(installed), "_config", "override.yaml")
55+
gomega.Expect(overrideConfigFilePath).Should(gomega.BeARegularFile())
56+
cfgBuf, err := os.ReadFile(filepath.Clean(overrideConfigFilePath))
4257
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
4358

4459
var limaCfg limayaml.LimaYAML
4560
err = yaml.Unmarshal(cfgBuf, &limaCfg)
4661
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
4762
gomega.Expect(*limaCfg.CPUs).Should(gomega.Equal(6))
4863
gomega.Expect(*limaCfg.Memory).Should(gomega.Equal("4GiB"))
64+
65+
defaultConfigFilePath := filepath.Join(limaDataDirPath(installed), "_config", "default.yaml")
66+
gomega.Expect(defaultConfigFilePath).Should(gomega.BeARegularFile())
67+
cfgBuf, err = os.ReadFile(filepath.Clean(defaultConfigFilePath))
68+
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
69+
err = yaml.Unmarshal(cfgBuf, &limaCfg)
70+
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
4971
gomega.Expect(*limaCfg.VMType).Should(gomega.Equal("vz"))
5072
gomega.Expect(*limaCfg.Rosetta.Enabled).Should(gomega.Equal(false))
5173
gomega.Expect(*limaCfg.Rosetta.BinFmt).Should(gomega.Equal(false))

0 commit comments

Comments
 (0)