From c746f93ba34fd6f15e665aff9e960b9a67127596 Mon Sep 17 00:00:00 2001 From: Swapnanil-Gupta Date: Mon, 6 Apr 2026 16:24:16 -0700 Subject: [PATCH 1/5] feat: make disk sizes (both boot & data) configurable on macos Signed-off-by: Swapnanil-Gupta --- cmd/finch/virtual_machine_darwin.go | 6 +-- cmd/finch/virtual_machine_disk_resize.go | 36 ++++++++++--- cmd/finch/virtual_machine_settings_darwin.go | 31 ++++++++++- pkg/config/config.go | 14 +++-- pkg/config/config_darwin.go | 13 ++++- pkg/config/defaults_darwin.go | 18 +++++++ pkg/config/lima_config_applier.go | 2 + pkg/config/lima_config_applier_darwin.go | 5 ++ pkg/config/validate_darwin.go | 22 ++++++++ pkg/disk/disk_darwin.go | 54 ++++++++++++++++++-- pkg/disk/disk_darwin_test.go | 2 +- 11 files changed, 179 insertions(+), 24 deletions(-) diff --git a/cmd/finch/virtual_machine_darwin.go b/cmd/finch/virtual_machine_darwin.go index 6c966d724..fe821771c 100644 --- a/cmd/finch/virtual_machine_darwin.go +++ b/cmd/finch/virtual_machine_darwin.go @@ -19,14 +19,14 @@ import ( "github.com/runfinch/finch/pkg/path" ) -func newDiskVMCommand(creator command.NerdctlCmdCreator, logger flog.Logger) *cobra.Command { +func newDiskVMCommand(creator command.NerdctlCmdCreator, logger flog.Logger, lca config.LimaConfigApplier, fs afero.Fs) *cobra.Command { diskCmd := &cobra.Command{ Use: "disk", Short: "Manage virtual machine disk operations", } diskCmd.AddCommand( - newVMDiskResizeCommand(creator, logger), + newVMDiskResizeCommand(creator, logger, lca, fs), newVMDiskInfoCommand(creator, logger), ) @@ -57,7 +57,7 @@ func newVirtualMachineCommand( newInitVMCommand(limaCmdCreator, logger, optionalDepGroups, lca, nca, fp.BaseYamlFilePath(), fs, fp.LimaSSHPrivateKeyPath(), diskManager, finchConfig), newSettingsVMCommand(logger, lca, fs, os.Stdout), - newDiskVMCommand(limaCmdCreator, logger), + newDiskVMCommand(limaCmdCreator, logger, lca, fs), ) return virtualMachineCommand diff --git a/cmd/finch/virtual_machine_disk_resize.go b/cmd/finch/virtual_machine_disk_resize.go index b35a5fb08..1b266e46b 100644 --- a/cmd/finch/virtual_machine_disk_resize.go +++ b/cmd/finch/virtual_machine_disk_resize.go @@ -8,18 +8,20 @@ package main import ( "fmt" + "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/runfinch/finch/pkg/command" + "github.com/runfinch/finch/pkg/config" "github.com/runfinch/finch/pkg/flog" ) -func newVMDiskResizeCommand(limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger) *cobra.Command { +func newVMDiskResizeCommand(limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger, lca config.LimaConfigApplier, fs afero.Fs) *cobra.Command { var size string cmd := &cobra.Command{ Use: "resize", - Short: "Resize the virtual machine disk", - RunE: newVMDiskResizeAction(limaCmdCreator, logger).runAdapter, + Short: "Resize the virtual machine's data disk", + RunE: newVMDiskResizeAction(limaCmdCreator, logger, lca, fs).runAdapter, } cmd.Flags().StringVar(&size, "size", "", "New size for the disk (required)") _ = cmd.MarkFlagRequired("size") @@ -27,14 +29,18 @@ func newVMDiskResizeCommand(limaCmdCreator command.NerdctlCmdCreator, logger flo } type diskResizeVMAction struct { - logger flog.Logger - creator command.NerdctlCmdCreator + logger flog.Logger + creator command.NerdctlCmdCreator + limaConfigApplier config.LimaConfigApplier + fs afero.Fs } -func newVMDiskResizeAction(limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger) *diskResizeVMAction { +func newVMDiskResizeAction(limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger, lca config.LimaConfigApplier, fs afero.Fs) *diskResizeVMAction { return &diskResizeVMAction{ - logger: logger, - creator: limaCmdCreator, + logger: logger, + creator: limaCmdCreator, + limaConfigApplier: lca, + fs: fs, } } @@ -56,5 +62,19 @@ func (dva *diskResizeVMAction) run(size string) error { } dva.logger.Info("Disk resized successfully.") + + isConfigUpdated, err := config.ModifyFinchConfig( + dva.fs, + dva.logger, + dva.limaConfigApplier.GetFinchConfigPath(), + config.VMConfigOpts{DataDisk: &size}, + ) + if err != nil { + return fmt.Errorf("disk resized but failed to update config: %w", err) + } + if isConfigUpdated { + dva.logger.Info("Configuration updated with new disk size.") + } + return nil } diff --git a/cmd/finch/virtual_machine_settings_darwin.go b/cmd/finch/virtual_machine_settings_darwin.go index e0999193b..8ebe9261e 100644 --- a/cmd/finch/virtual_machine_settings_darwin.go +++ b/cmd/finch/virtual_machine_settings_darwin.go @@ -38,6 +38,16 @@ func newSettingsVMCommand( config.DefaultMemory, "the amount of memory to dedicate to the virtual machine (restart the vm when applying this change.)", ) + settingsVMCommand.Flags().String( + "bootdisk", + config.DefaultBootDisk, + "the amount of boot disk space (sparse) to give to the virtual machine (restart the vm when applying this change.)", + ) + settingsVMCommand.Flags().String( + "datadisk", + config.DefaultDataDisk, + "the amount of data disk space (sparse) to give to the virtual machine (restart the vm when applying this change.)", + ) return settingsVMCommand } @@ -74,11 +84,23 @@ func (sva *settingsVMAction) runAdapter(cmd *cobra.Command, _ []string) error { return err } + bootDisk, err := cmd.Flags().GetString("bootdisk") + if err != nil { + return err + } + + dataDisk, err := cmd.Flags().GetString("datadisk") + if err != nil { + return err + } + cpusChanged := cmd.Flags().Changed("cpus") memoryChanged := cmd.Flags().Changed("memory") + bootDiskChanged := cmd.Flags().Changed("bootdisk") + dataDiskChanged := cmd.Flags().Changed("datadisk") // check if any flags were provided by the user - if !cpusChanged && !memoryChanged { + if !cpusChanged && !memoryChanged && !bootDiskChanged && !dataDiskChanged { return cmd.Help() } @@ -90,6 +112,13 @@ func (sva *settingsVMAction) runAdapter(cmd *cobra.Command, _ []string) error { opts.Memory = &memory } + if bootDiskChanged { + opts.BootDisk = &bootDisk + } + if dataDiskChanged { + opts.DataDisk = &dataDisk + } + return sva.run(opts) } diff --git a/pkg/config/config.go b/pkg/config/config.go index b21dfbe1b..0ce083a9e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -66,14 +66,18 @@ type Nerdctl struct { // VMConfigOpts represents the Options for finch vm settings command. type VMConfigOpts struct { - CPUs *int - Memory *string + CPUs *int + Memory *string + BootDisk *string + DataDisk *string } -// Default values for the command line arguments --cpus and --memory. +// Default values for the command line arguments --cpus, --memory, --bootdisk and --datadisk. const ( - DefaultCPUs = 0 - DefaultMemory = "" + DefaultCPUs = 0 + DefaultMemory = "" + DefaultBootDisk = "" + DefaultDataDisk = "" ) // LimaConfigApplier applies lima configuration changes. diff --git a/pkg/config/config_darwin.go b/pkg/config/config_darwin.go index d2dd3260e..52af7d970 100644 --- a/pkg/config/config_darwin.go +++ b/pkg/config/config_darwin.go @@ -25,6 +25,8 @@ type SystemSettings struct { Memory *string `yaml:"memory,omitempty"` AdditionalDirectories []AdditionalDirectory `yaml:"additional_directories,omitempty"` Rosetta *bool `yaml:"rosetta,omitempty"` + BootDisk *string `yaml:"bootdisk,omitempty"` + DataDisk *string `yaml:"datadisk,omitempty"` SharedSystemSettings `yaml:",inline"` } @@ -92,8 +94,9 @@ func ModifyFinchConfig(fs afero.Fs, logger flog.Logger, finchConfigPath string, } cpus, memory := opts.CPUs, opts.Memory + bootDisk, dataDisk := opts.BootDisk, opts.DataDisk // This should never happen when called from the CLI, but good to cover just in case - if cpus == nil && memory == nil { + if cpus == nil && memory == nil && bootDisk == nil && dataDisk == nil { return isConfigUpdated, fmt.Errorf("specify at least one flag") } if cpus != nil && *cpus != *finchCfg.CPUs { @@ -104,6 +107,14 @@ func ModifyFinchConfig(fs afero.Fs, logger flog.Logger, finchConfigPath string, *finchCfg.Memory = *memory isConfigUpdated = true } + if bootDisk != nil && *bootDisk != *finchCfg.BootDisk { + *finchCfg.BootDisk = *bootDisk + isConfigUpdated = true + } + if dataDisk != nil && *dataDisk != *finchCfg.DataDisk { + *finchCfg.DataDisk = *dataDisk + isConfigUpdated = true + } if !isConfigUpdated { return isConfigUpdated, nil diff --git a/pkg/config/defaults_darwin.go b/pkg/config/defaults_darwin.go index 3b388aab8..d678fa55e 100644 --- a/pkg/config/defaults_darwin.go +++ b/pkg/config/defaults_darwin.go @@ -19,6 +19,9 @@ const ( // 2,147,483,648 => 2GiB. fallbackMemory float64 = 2_147_483_648 fallbackCPUs int = 2 + // 53_687_091_200 => 50GiB + fallbackBootDisk float64 = 53_687_091_200 + fallbackDatadisk float64 = 53_687_091_200 ) func vmDefault(cfg *Finch, supportsVz bool) { @@ -65,6 +68,18 @@ func credHelperDefault(cfg *Finch) { } } +func bootDiskDefault(cfg *Finch) { + if cfg.BootDisk == nil { + cfg.BootDisk = pointer.String(units.BytesSize(fallbackBootDisk)) + } +} + +func dataDiskDefault(cfg *Finch) { + if cfg.DataDisk == nil { + cfg.DataDisk = pointer.String(units.BytesSize(fallbackDatadisk)) + } +} + // applyDefaults sets default configuration options if they are not already set. func applyDefaults( cfg *Finch, @@ -83,5 +98,8 @@ func applyDefaults( rosettaDefault(cfg) credHelperDefault(cfg) + bootDiskDefault(cfg) + dataDiskDefault(cfg) + return cfg } diff --git a/pkg/config/lima_config_applier.go b/pkg/config/lima_config_applier.go index 4b0608c8c..f4d793e97 100644 --- a/pkg/config/lima_config_applier.go +++ b/pkg/config/lima_config_applier.go @@ -182,6 +182,8 @@ func (lca *limaConfigApplier) ConfigureOverrideLimaYaml() error { lca.configureCPUs(&limaCfg) lca.configureMemory(&limaCfg) lca.configureMounts(&limaCfg) + lca.configureDisk(&limaCfg) + if *lca.cfg.VMType != "wsl2" && len(limaCfg.AdditionalDisks) == 0 { limaCfg.AdditionalDisks = append(limaCfg.AdditionalDisks, limatype.Disk{ Name: "finch", diff --git a/pkg/config/lima_config_applier_darwin.go b/pkg/config/lima_config_applier_darwin.go index b0af77782..cc4e1f552 100644 --- a/pkg/config/lima_config_applier_darwin.go +++ b/pkg/config/lima_config_applier_darwin.go @@ -110,3 +110,8 @@ func (lca *limaConfigApplier) configureMounts(limaCfg *limatype.LimaYAML) *limat } return limaCfg } + +func (lca *limaConfigApplier) configureDisk(limaCfg *limatype.LimaYAML) *limatype.LimaYAML { + limaCfg.Disk = lca.cfg.BootDisk + return limaCfg +} diff --git a/pkg/config/validate_darwin.go b/pkg/config/validate_darwin.go index 141fedffe..4cf35e4da 100644 --- a/pkg/config/validate_darwin.go +++ b/pkg/config/validate_darwin.go @@ -54,5 +54,27 @@ func validate(cfg *Finch, log flog.Logger, systemDeps LoadSystemDeps, mem fmemor ) } + bootDiskInt, err := units.FromHumanSize(*cfg.BootDisk) + if err != nil { + return fmt.Errorf("failed to parse bootdisk to uint: %w", err) + } + if bootDiskInt <= 10 { + return fmt.Errorf( + "specified size of boot disk (%s) must be greater than 10GiB", + *cfg.BootDisk, + ) + } + + dataDiskInt, err := units.FromHumanSize(*cfg.DataDisk) + if err != nil { + return fmt.Errorf("failed to parse datadisk to uint: %w", err) + } + if dataDiskInt <= 10 { + return fmt.Errorf( + "specified size of datadisk (%s) must be greater than 10GiB", + *cfg.DataDisk, + ) + } + return nil } diff --git a/pkg/disk/disk_darwin.go b/pkg/disk/disk_darwin.go index d4eb1cbc3..d53e5b8e7 100644 --- a/pkg/disk/disk_darwin.go +++ b/pkg/disk/disk_darwin.go @@ -18,8 +18,7 @@ import ( const ( // diskName must always be consistent with the value set for AdditionalDisks in lima_config_applier.go. - diskName = "finch" - diskSizeStr = "50GB" + diskName = "finch" ) type qemuDiskInfo struct { @@ -78,6 +77,10 @@ func (m *userDataDiskManager) EnsureUserDataDisk() error { } } + if err := m.resizeDiskIfNeeded(); err != nil { + return err + } + if m.limaDiskIsLocked() { err := m.unlockLimaDisk() if err != nil { @@ -153,7 +156,7 @@ func (m *userDataDiskManager) convertToRaw(diskPath string) error { } func (m *userDataDiskManager) createLimaDisk() error { - size, err := sizeString() + size, err := sizeString(*m.config.DataDisk) if err != nil { return fmt.Errorf("failed to get disk size: %w", err) } @@ -214,8 +217,49 @@ func (m *userDataDiskManager) unlockLimaDisk() error { return nil } -func sizeString() (string, error) { - sizeB, err := units.RAMInBytes(diskSizeStr) +func (m *userDataDiskManager) resizeDiskIfNeeded() error { + if m.config.DataDisk == nil { + return nil + } + + configuredDataDiskBytes, err := units.RAMInBytes(*m.config.DataDisk) + if err != nil { + return fmt.Errorf("failed to parse configured disk size: %w", err) + } + + diskPath := m.finch.UserDataDiskPath(m.rootDir) + info, err := m.getDiskInfo(diskPath) + if err != nil { + return err + } + + actualDataDiskBytes := int64(info.VirtualSize) + if configuredDataDiskBytes == actualDataDiskBytes { + return nil + } + + if configuredDataDiskBytes < actualDataDiskBytes { + m.logger.Warnf("Shrinking the data disk is not supported (configured: %s, current: %s), skipping resize", + units.BytesSize(float64(configuredDataDiskBytes)), units.BytesSize(float64(actualDataDiskBytes))) + return nil + } + + size, err := sizeString(*m.config.DataDisk) + if err != nil { + return fmt.Errorf("failed to get disk size: %w", err) + } + + m.logger.Infof("Resizing data disk from %s to %s", units.BytesSize(float64(actualDataDiskBytes)), size) + cmd := m.ncc.CreateWithoutStdio("disk", "resize", diskName, "--size", size) + if logs, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to resize disk to %s, debug logs:\n%s", size, logs) + } + + return nil +} + +func sizeString(size string) (string, error) { + sizeB, err := units.RAMInBytes(size) if err != nil { return "", err } diff --git a/pkg/disk/disk_darwin_test.go b/pkg/disk/disk_darwin_test.go index b41088ab7..7afd76708 100644 --- a/pkg/disk/disk_darwin_test.go +++ b/pkg/disk/disk_darwin_test.go @@ -39,7 +39,7 @@ func TestUserDataDiskManager_InitializeUserDataDisk(t *testing.T) { finch := fpath.Finch("mock_finch") homeDir := "mock_home" - size, err := sizeString() + size, err := sizeString("50GiB") assert.NoError(t, err) limaPath := path.Join(finch.LimaHomePath(), "_disks", diskName, "datadisk") From 3430859df22fb32ec9acee4300b3af34c322acc5 Mon Sep 17 00:00:00 2001 From: Swapnanil-Gupta Date: Tue, 7 Apr 2026 14:51:38 -0700 Subject: [PATCH 2/5] fix unit tests and add unit tests for disk config Signed-off-by: Swapnanil-Gupta --- .../virtual_machine_settings_darwin_test.go | 126 +++++++++++++++++- pkg/config/config_darwin_test.go | 4 + pkg/config/defaults_darwin.go | 5 +- pkg/config/lima_config_applier_windows.go | 4 + pkg/config/validate_darwin.go | 42 +++--- pkg/config/validate_darwin_test.go | 86 ++++++++++++ pkg/disk/disk_darwin.go | 3 - pkg/disk/disk_darwin_test.go | 16 ++- 8 files changed, 256 insertions(+), 30 deletions(-) diff --git a/cmd/finch/virtual_machine_settings_darwin_test.go b/cmd/finch/virtual_machine_settings_darwin_test.go index 05a058346..6e41bcea5 100644 --- a/cmd/finch/virtual_machine_settings_darwin_test.go +++ b/cmd/finch/virtual_machine_settings_darwin_test.go @@ -100,6 +100,46 @@ func TestSettingsVMAction_runAdapter(t *testing.T) { lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) }, }, + { + name: "should configure the instance for valid bootdisk value", + wantErr: nil, + command: &cobra.Command{ + Use: "settings", + }, + args: []string{ + "--bootdisk=60GiB", + }, + mockSvc: func( + lca *mocks.LimaConfigApplier, + fs afero.Fs, + ) { + finchConfigPath := "/config.yaml" + data := "cpus: 2\nmemory: 6GiB\nbootdisk: 100GiB\ndatadisk: 50GiB" + require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600)) + + lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) + }, + }, + { + name: "should configure the instance for valid datadisk value", + wantErr: nil, + command: &cobra.Command{ + Use: "settings", + }, + args: []string{ + "--datadisk=60GiB", + }, + mockSvc: func( + lca *mocks.LimaConfigApplier, + fs afero.Fs, + ) { + finchConfigPath := "/config.yaml" + data := "cpus: 2\nmemory: 6GiB\nbootdisk: 100GiB\ndatadisk: 50GiB" + require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600)) + + lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) + }, + }, { name: "should show settings --help when no flags are provided", wantErr: nil, @@ -155,8 +195,10 @@ func TestSettingsVMAction_run(t *testing.T) { *mocks.LimaConfigApplier, afero.Fs, ) - cpus *int - memory *string + cpus *int + memory *string + bootDisk *string + dataDisk *string }{ { name: "should update vm settings", @@ -230,6 +272,44 @@ func TestSettingsVMAction_run(t *testing.T) { cpus: intPtr(1), memory: stringPtr("2gi"), }, + { + name: "should return an error if the configuration of bootdisk is invalid", + wantErr: "failed to validate config file: failed to parse bootdisk to uint: invalid suffix: 'gi'", + wantStatusOutput: "", + wantWarnOutput: "", + mockSvc: func( + lca *mocks.LimaConfigApplier, + fs afero.Fs, + ) { + finchConfigPath := "/config.yaml" + data := "cpus: 2\nmemory: 6GiB\nbootdisk: 100GiB\ndatadisk: 50GiB" + require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600)) + + lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) + }, + cpus: intPtr(1), + memory: stringPtr("2GiB"), + bootDisk: stringPtr("50gi"), + }, + { + name: "should return an error if the configuration of datadisk is invalid", + wantErr: "failed to validate config file: failed to parse datadisk to uint: invalid suffix: 'gi'", + wantStatusOutput: "", + wantWarnOutput: "", + mockSvc: func( + lca *mocks.LimaConfigApplier, + fs afero.Fs, + ) { + finchConfigPath := "/config.yaml" + data := "cpus: 2\nmemory: 6GiB\nbootdisk: 100GiB\ndatadisk: 50GiB" + require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600)) + + lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) + }, + cpus: intPtr(1), + memory: stringPtr("2GiB"), + dataDisk: stringPtr("50gi"), + }, { name: "should not return an error if the configuration of CPU and memory matches existing config", wantErr: "", @@ -240,7 +320,7 @@ func TestSettingsVMAction_run(t *testing.T) { fs afero.Fs, ) { finchConfigPath := "/config.yaml" - data := "cpus: 2\nmemory: 6GiB" + data := "cpus: 2\nmemory: 6GiB\nbootdisk: 100GiB\ndatadisk: 50GiB" require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600)) lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) @@ -248,6 +328,40 @@ func TestSettingsVMAction_run(t *testing.T) { cpus: intPtr(2), memory: stringPtr("6GiB"), }, + { + name: "should not return an error if the configuration of bootdisk matches existing config", + wantErr: "", + wantStatusOutput: "", + wantWarnOutput: "Provided flags match existing settings, no changes made.", + mockSvc: func( + lca *mocks.LimaConfigApplier, + fs afero.Fs, + ) { + finchConfigPath := "/config.yaml" + data := "cpus: 2\nmemory: 6GiB\nbootdisk: 100GiB\ndatadisk: 50GiB" + require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600)) + + lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) + }, + bootDisk: stringPtr("100GiB"), + }, + { + name: "should not return an error if the configuration of datadisk matches existing config", + wantErr: "", + wantStatusOutput: "", + wantWarnOutput: "Provided flags match existing settings, no changes made.", + mockSvc: func( + lca *mocks.LimaConfigApplier, + fs afero.Fs, + ) { + finchConfigPath := "/config.yaml" + data := "cpus: 2\nmemory: 6GiB\nbootdisk: 100GiB\ndatadisk: 50GiB" + require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600)) + + lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath) + }, + dataDisk: stringPtr("50GiB"), + }, } for _, tc := range testCases { @@ -260,8 +374,10 @@ func TestSettingsVMAction_run(t *testing.T) { fs := afero.NewMemMapFs() stdout := bytes.Buffer{} opts := config.VMConfigOpts{ - CPUs: tc.cpus, - Memory: tc.memory, + CPUs: tc.cpus, + Memory: tc.memory, + BootDisk: tc.bootDisk, + DataDisk: tc.dataDisk, } tc.mockSvc(lca, fs) diff --git a/pkg/config/config_darwin_test.go b/pkg/config/config_darwin_test.go index 62d46b367..d4c671c73 100644 --- a/pkg/config/config_darwin_test.go +++ b/pkg/config/config_darwin_test.go @@ -23,6 +23,8 @@ func makeConfig(vmType limayaml.VMType, memory string, cpus int, rosetta bool) * fc.Memory = pointer.String(memory) fc.CPUs = pointer.Int(cpus) fc.Rosetta = pointer.Bool(rosetta) + fc.BootDisk = pointer.String("100GiB") + fc.DataDisk = pointer.String("50GiB") fc.CredsHelpers = []string{"osxkeychain"} // default helper return &fc } @@ -38,6 +40,8 @@ func makeExperimentalConfig( fc.Memory = pointer.String(memory) fc.CPUs = pointer.Int(cpus) fc.Rosetta = pointer.Bool(rosetta) + fc.BootDisk = pointer.String("100GiB") + fc.DataDisk = pointer.String("50GiB") fc.Experimental = experimentalSettings fc.CredsHelpers = []string{"osxkeychain"} // default helper return &fc diff --git a/pkg/config/defaults_darwin.go b/pkg/config/defaults_darwin.go index d678fa55e..585d8a31d 100644 --- a/pkg/config/defaults_darwin.go +++ b/pkg/config/defaults_darwin.go @@ -19,8 +19,11 @@ const ( // 2,147,483,648 => 2GiB. fallbackMemory float64 = 2_147_483_648 fallbackCPUs int = 2 + // Default disk size is 100GiB for Lima + // https://github.com/lima-vm/lima/blob/e78406086b65bea1e143e456c066d34366d6c6f7/templates/default.yaml#L45 + // 107_374_182_400 => 100GiB + fallbackBootDisk float64 = 107_374_182_400 // 53_687_091_200 => 50GiB - fallbackBootDisk float64 = 53_687_091_200 fallbackDatadisk float64 = 53_687_091_200 ) diff --git a/pkg/config/lima_config_applier_windows.go b/pkg/config/lima_config_applier_windows.go index 93551c359..23ab804cc 100644 --- a/pkg/config/lima_config_applier_windows.go +++ b/pkg/config/lima_config_applier_windows.go @@ -40,3 +40,7 @@ func (lca *limaConfigApplier) configureMemory(limaCfg *limatype.LimaYAML) *limat func (lca *limaConfigApplier) configureMounts(limaCfg *limatype.LimaYAML) *limatype.LimaYAML { return limaCfg } + +func (lca *limaConfigApplier) configureDisk(limaCfg *limatype.LimaYAML) *limatype.LimaYAML { + return limaCfg +} diff --git a/pkg/config/validate_darwin.go b/pkg/config/validate_darwin.go index 4cf35e4da..a778b0e8c 100644 --- a/pkg/config/validate_darwin.go +++ b/pkg/config/validate_darwin.go @@ -22,7 +22,7 @@ func validate(cfg *Finch, log flog.Logger, systemDeps LoadSystemDeps, mem fmemor ) } - memInt, err := units.FromHumanSize(*cfg.Memory) + memInt, err := units.RAMInBytes(*cfg.Memory) if err != nil { return fmt.Errorf("failed to parse memory to uint: %w", err) } @@ -54,26 +54,30 @@ func validate(cfg *Finch, log flog.Logger, systemDeps LoadSystemDeps, mem fmemor ) } - bootDiskInt, err := units.FromHumanSize(*cfg.BootDisk) - if err != nil { - return fmt.Errorf("failed to parse bootdisk to uint: %w", err) - } - if bootDiskInt <= 10 { - return fmt.Errorf( - "specified size of boot disk (%s) must be greater than 10GiB", - *cfg.BootDisk, - ) + if cfg.BootDisk != nil { + bootDiskInt, err := units.RAMInBytes(*cfg.BootDisk) + if err != nil { + return fmt.Errorf("failed to parse bootdisk to uint: %w", err) + } + if bootDiskInt <= 10*1024*1024*1024 { + return fmt.Errorf( + "specified size of boot disk (%s) must be greater than 10GiB", + *cfg.BootDisk, + ) + } } - dataDiskInt, err := units.FromHumanSize(*cfg.DataDisk) - if err != nil { - return fmt.Errorf("failed to parse datadisk to uint: %w", err) - } - if dataDiskInt <= 10 { - return fmt.Errorf( - "specified size of datadisk (%s) must be greater than 10GiB", - *cfg.DataDisk, - ) + if cfg.DataDisk != nil { + dataDiskInt, err := units.RAMInBytes(*cfg.DataDisk) + if err != nil { + return fmt.Errorf("failed to parse datadisk to uint: %w", err) + } + if dataDiskInt <= 10*1024*1024*1024 { + return fmt.Errorf( + "specified size of datadisk (%s) must be greater than 10GiB", + *cfg.DataDisk, + ) + } } return nil diff --git a/pkg/config/validate_darwin_test.go b/pkg/config/validate_darwin_test.go index b7dc278e8..99bf7c197 100644 --- a/pkg/config/validate_darwin_test.go +++ b/pkg/config/validate_darwin_test.go @@ -7,8 +7,10 @@ package config import ( "errors" + "fmt" "testing" + "github.com/docker/go-units" "github.com/stretchr/testify/require" "github.com/xorcare/pointer" "go.uber.org/mock/gomock" @@ -103,6 +105,90 @@ func TestValidate(t *testing.T) { }, err: nil, }, + { + name: "happy path with bootdisk and datadisk", + cfg: &Finch{ + SystemSettings: SystemSettings{ + CPUs: pointer.Int(4), + Memory: pointer.String("4GiB"), + BootDisk: pointer.String("50GiB"), + DataDisk: pointer.String("50GiB"), + }, + }, + mockSvc: func(_ *mocks.Logger, deps *mocks.LoadSystemDeps, mem *mocks.Memory) { + deps.EXPECT().NumCPU().Return(8) + mem.EXPECT().TotalMemory().Return(uint64(12_880_000_000)) + }, + err: nil, + }, + { + name: "config specifies bootdisk less than minimum", + cfg: &Finch{ + SystemSettings: SystemSettings{ + CPUs: pointer.Int(4), + Memory: pointer.String("4GiB"), + BootDisk: pointer.String("5GiB"), + }, + }, + mockSvc: func(_ *mocks.Logger, deps *mocks.LoadSystemDeps, mem *mocks.Memory) { + deps.EXPECT().NumCPU().Return(8) + mem.EXPECT().TotalMemory().Return(uint64(12_880_000_000)) + }, + err: errors.New("specified size of boot disk (5GiB) must be greater than 10GiB"), + }, + { + name: "config specifies datadisk less than minimum", + cfg: &Finch{ + SystemSettings: SystemSettings{ + CPUs: pointer.Int(4), + Memory: pointer.String("4GiB"), + BootDisk: pointer.String("50GiB"), + DataDisk: pointer.String("5GiB"), + }, + }, + mockSvc: func(_ *mocks.Logger, deps *mocks.LoadSystemDeps, mem *mocks.Memory) { + deps.EXPECT().NumCPU().Return(8) + mem.EXPECT().TotalMemory().Return(uint64(12_880_000_000)) + }, + err: errors.New("specified size of datadisk (5GiB) must be greater than 10GiB"), + }, + { + name: "config specifies invalid bootdisk format", + cfg: &Finch{ + SystemSettings: SystemSettings{ + CPUs: pointer.Int(4), + Memory: pointer.String("4GiB"), + BootDisk: pointer.String("50gi"), + }, + }, + mockSvc: func(_ *mocks.Logger, deps *mocks.LoadSystemDeps, mem *mocks.Memory) { + deps.EXPECT().NumCPU().Return(8) + mem.EXPECT().TotalMemory().Return(uint64(12_880_000_000)) + }, + err: func() error { + _, e := units.RAMInBytes("50gi") + return fmt.Errorf("failed to parse bootdisk to uint: %w", e) + }(), + }, + { + name: "config specifies invalid datadisk format", + cfg: &Finch{ + SystemSettings: SystemSettings{ + CPUs: pointer.Int(4), + Memory: pointer.String("4GiB"), + BootDisk: pointer.String("50GiB"), + DataDisk: pointer.String("50gi"), + }, + }, + mockSvc: func(_ *mocks.Logger, deps *mocks.LoadSystemDeps, mem *mocks.Memory) { + deps.EXPECT().NumCPU().Return(8) + mem.EXPECT().TotalMemory().Return(uint64(12_880_000_000)) + }, + err: func() error { + _, e := units.RAMInBytes("50gi") + return fmt.Errorf("failed to parse datadisk to uint: %w", e) + }(), + }, } for _, tc := range testCases { diff --git a/pkg/disk/disk_darwin.go b/pkg/disk/disk_darwin.go index d53e5b8e7..2c5be4a91 100644 --- a/pkg/disk/disk_darwin.go +++ b/pkg/disk/disk_darwin.go @@ -263,9 +263,6 @@ func sizeString(size string) (string, error) { if err != nil { return "", err } - if err != nil { - return "", err - } return units.BytesSize(float64(sizeB)), nil } diff --git a/pkg/disk/disk_darwin_test.go b/pkg/disk/disk_darwin_test.go index 7afd76708..41d5bd0b6 100644 --- a/pkg/disk/disk_darwin_test.go +++ b/pkg/disk/disk_darwin_test.go @@ -90,10 +90,11 @@ func TestUserDataDiskManager_InitializeUserDataDisk(t *testing.T) { SharedSystemSettings: config.SharedSystemSettings{ VMType: pointer.String("qemu"), }, + DataDisk: pointer.String("50GiB"), }, }, wantErr: nil, - mockSvc: func(ncc *mocks.NerdctlCmdCreator, dfs *mocks.MockdiskFS, cmd *mocks.Command, _ *mocks.CommandCreator) { + mockSvc: func(ncc *mocks.NerdctlCmdCreator, dfs *mocks.MockdiskFS, cmd *mocks.Command, ecc *mocks.CommandCreator) { ncc.EXPECT().CreateWithoutStdio(mockListArgs).Return(cmd) cmd.EXPECT().Output().Return([]byte(""), nil) @@ -107,6 +108,11 @@ func TestUserDataDiskManager_InitializeUserDataDisk(t *testing.T) { dfs.EXPECT().Stat(limaPath).Return(nil, fs.ErrNotExist) dfs.EXPECT().SymlinkIfPossible(finch.UserDataDiskPath(homeDir), limaPath).Return(nil) + // resizeDiskIfNeeded: getDiskInfo returns current size == configured size, no resize needed + diskInfoOutput := []byte(`{"virtual-size": 53687091200, "filename": "mock", "format": "raw", "actual-size": 0, "dirty-flag": false}`) + ecc.EXPECT().Create(mockQemuImgExePath, mockDiskInfoArgs).Return(cmd) + cmd.EXPECT().CombinedOutput().Return(diskInfoOutput, nil) + dfs.EXPECT().Stat(lockPath).Return(nil, fs.ErrNotExist) }, }, @@ -163,10 +169,11 @@ func TestUserDataDiskManager_InitializeUserDataDisk(t *testing.T) { SharedSystemSettings: config.SharedSystemSettings{ VMType: pointer.String("qemu"), }, + DataDisk: pointer.String("50GiB"), }, }, wantErr: nil, - mockSvc: func(ncc *mocks.NerdctlCmdCreator, dfs *mocks.MockdiskFS, cmd *mocks.Command, _ *mocks.CommandCreator) { + mockSvc: func(ncc *mocks.NerdctlCmdCreator, dfs *mocks.MockdiskFS, cmd *mocks.Command, ecc *mocks.CommandCreator) { ncc.EXPECT().CreateWithoutStdio(mockListArgs).Return(cmd) cmd.EXPECT().Output().Return([]byte(""), nil) @@ -180,6 +187,11 @@ func TestUserDataDiskManager_InitializeUserDataDisk(t *testing.T) { dfs.EXPECT().SymlinkIfPossible(finch.UserDataDiskPath(homeDir), limaPath).Return(nil) + // resizeDiskIfNeeded: getDiskInfo returns current size == configured size, no resize needed + diskInfoOutput := []byte(`{"virtual-size": 53687091200, "filename": "mock", "format": "raw", "actual-size": 0, "dirty-flag": false}`) + ecc.EXPECT().Create(mockQemuImgExePath, mockDiskInfoArgs).Return(cmd) + cmd.EXPECT().CombinedOutput().Return(diskInfoOutput, nil) + dfs.EXPECT().Stat(lockPath).Return(nil, fs.ErrNotExist) }, }, From 1a39d3c0f7a15e10d710d4a924ec2ba563148acb Mon Sep 17 00:00:00 2001 From: Swapnanil-Gupta Date: Tue, 7 Apr 2026 15:24:58 -0700 Subject: [PATCH 3/5] fix lint issues Signed-off-by: Swapnanil-Gupta --- cmd/finch/virtual_machine_disk_resize.go | 8 ++++++-- pkg/config/defaults_darwin.go | 6 +++--- pkg/disk/disk_darwin_test.go | 8 ++++++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/cmd/finch/virtual_machine_disk_resize.go b/cmd/finch/virtual_machine_disk_resize.go index 1b266e46b..17f86eb01 100644 --- a/cmd/finch/virtual_machine_disk_resize.go +++ b/cmd/finch/virtual_machine_disk_resize.go @@ -16,7 +16,9 @@ import ( "github.com/runfinch/finch/pkg/flog" ) -func newVMDiskResizeCommand(limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger, lca config.LimaConfigApplier, fs afero.Fs) *cobra.Command { +func newVMDiskResizeCommand( + limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger, lca config.LimaConfigApplier, fs afero.Fs, +) *cobra.Command { var size string cmd := &cobra.Command{ Use: "resize", @@ -35,7 +37,9 @@ type diskResizeVMAction struct { fs afero.Fs } -func newVMDiskResizeAction(limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger, lca config.LimaConfigApplier, fs afero.Fs) *diskResizeVMAction { +func newVMDiskResizeAction( + limaCmdCreator command.NerdctlCmdCreator, logger flog.Logger, lca config.LimaConfigApplier, fs afero.Fs, +) *diskResizeVMAction { return &diskResizeVMAction{ logger: logger, creator: limaCmdCreator, diff --git a/pkg/config/defaults_darwin.go b/pkg/config/defaults_darwin.go index 585d8a31d..5a34b121b 100644 --- a/pkg/config/defaults_darwin.go +++ b/pkg/config/defaults_darwin.go @@ -19,11 +19,11 @@ const ( // 2,147,483,648 => 2GiB. fallbackMemory float64 = 2_147_483_648 fallbackCPUs int = 2 - // Default disk size is 100GiB for Lima + // Default disk size is 100GiB for Lima. // https://github.com/lima-vm/lima/blob/e78406086b65bea1e143e456c066d34366d6c6f7/templates/default.yaml#L45 - // 107_374_182_400 => 100GiB + // 107_374_182_400 => 100GiB. fallbackBootDisk float64 = 107_374_182_400 - // 53_687_091_200 => 50GiB + // 53_687_091_200 => 50GiB. fallbackDatadisk float64 = 53_687_091_200 ) diff --git a/pkg/disk/disk_darwin_test.go b/pkg/disk/disk_darwin_test.go index 41d5bd0b6..31845de9e 100644 --- a/pkg/disk/disk_darwin_test.go +++ b/pkg/disk/disk_darwin_test.go @@ -109,7 +109,9 @@ func TestUserDataDiskManager_InitializeUserDataDisk(t *testing.T) { dfs.EXPECT().SymlinkIfPossible(finch.UserDataDiskPath(homeDir), limaPath).Return(nil) // resizeDiskIfNeeded: getDiskInfo returns current size == configured size, no resize needed - diskInfoOutput := []byte(`{"virtual-size": 53687091200, "filename": "mock", "format": "raw", "actual-size": 0, "dirty-flag": false}`) + diskInfoOutput := []byte( + `{"virtual-size": 53687091200, "filename": "mock", "format": "raw", "actual-size": 0, "dirty-flag": false}`, + ) ecc.EXPECT().Create(mockQemuImgExePath, mockDiskInfoArgs).Return(cmd) cmd.EXPECT().CombinedOutput().Return(diskInfoOutput, nil) @@ -188,7 +190,9 @@ func TestUserDataDiskManager_InitializeUserDataDisk(t *testing.T) { dfs.EXPECT().SymlinkIfPossible(finch.UserDataDiskPath(homeDir), limaPath).Return(nil) // resizeDiskIfNeeded: getDiskInfo returns current size == configured size, no resize needed - diskInfoOutput := []byte(`{"virtual-size": 53687091200, "filename": "mock", "format": "raw", "actual-size": 0, "dirty-flag": false}`) + diskInfoOutput := []byte( + `{"virtual-size": 53687091200, "filename": "mock", "format": "raw", "actual-size": 0, "dirty-flag": false}`, + ) ecc.EXPECT().Create(mockQemuImgExePath, mockDiskInfoArgs).Return(cmd) cmd.EXPECT().CombinedOutput().Return(diskInfoOutput, nil) From 1ad03182d087363a27afbe8a1814a754fd5432c7 Mon Sep 17 00:00:00 2001 From: Swapnanil-Gupta Date: Tue, 7 Apr 2026 16:09:29 -0700 Subject: [PATCH 4/5] add e2e tests Signed-off-by: Swapnanil-Gupta --- e2e/vm/disk_config_test.go | 148 +++++++++++++++++++++++++++++++++++++ e2e/vm/vm_darwin_test.go | 1 + 2 files changed, 149 insertions(+) create mode 100644 e2e/vm/disk_config_test.go diff --git a/e2e/vm/disk_config_test.go b/e2e/vm/disk_config_test.go new file mode 100644 index 000000000..6cee574de --- /dev/null +++ b/e2e/vm/disk_config_test.go @@ -0,0 +1,148 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build darwin + +package vm + +import ( + "os" + "path/filepath" + + "github.com/lima-vm/lima/v2/pkg/limatype" + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + "github.com/onsi/gomega/gbytes" + "github.com/onsi/gomega/gexec" + "github.com/runfinch/common-tests/command" + "github.com/runfinch/common-tests/option" + "gopkg.in/yaml.v3" +) + +var testDiskConfig = func(o *option.Option, installed bool) { + ginkgo.Describe("Disk config (bootdisk and datadisk)", ginkgo.Serial, func() { + ginkgo.It("should apply bootdisk and datadisk from config on vm init", func() { + resetVM(o) + resetDisks(o, installed) + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\nbootdisk: 120GiB\ndatadisk: 60GiB")) + command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // Verify bootdisk is set in override.yaml (lima Disk field) + 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 limatype.LimaYAML + err = yaml.Unmarshal(cfgBuf, &limaCfg) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(limaCfg.Disk).ShouldNot(gomega.BeNil()) + gomega.Expect(*limaCfg.Disk).Should(gomega.Equal("120GiB")) + + // Verify datadisk size via disk info + result := command.New(o, virtualMachineRootCmd, "disk", "info").Run() + gomega.Expect(result.Out.Contents()).To(gomega.ContainSubstring("60GiB")) + }) + + ginkgo.It("should update bootdisk and datadisk via vm settings command", func() { + resetVM(o) + resetDisks(o, installed) + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\nbootdisk: 100GiB\ndatadisk: 50GiB")) + command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // Update bootdisk and datadisk via settings + command.New(o, virtualMachineRootCmd, "settings", "--bootdisk=120GiB", "--datadisk=70GiB").Run() + + // Verify config file was updated + cfgBuf, err := os.ReadFile(filepath.Clean(finchConfigFilePath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(string(cfgBuf)).Should(gomega.ContainSubstring("bootdisk: 120GiB")) + gomega.Expect(string(cfgBuf)).Should(gomega.ContainSubstring("datadisk: 70GiB")) + }) + + ginkgo.It("should warn when reducing data disk size on vm start", func() { + resetVM(o) + resetDisks(o, installed) + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\ndatadisk: 60GiB")) + command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // Reduce datadisk in config and restart + command.New(o, virtualMachineRootCmd, "stop").WithTimeoutInSeconds(90).Run() + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\ndatadisk: 50GiB")) + session := command.New(o, virtualMachineRootCmd, "start"). + WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // VM should still start successfully but warn about shrink + gomega.Expect(session).Should(gexec.Exit(0)) + gomega.Expect(session.Err).Should(gbytes.Say("Shrinking the data disk is not supported")) + }) + + ginkgo.It("should warn when reducing data disk size on vm init", func() { + resetVM(o) + resetDisks(o, installed) + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\ndatadisk: 60GiB")) + command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // Remove VM but keep disks, then re-init with smaller datadisk + command.New(o, virtualMachineRootCmd, "stop", "-f").WithoutCheckingExitCode().WithTimeoutInSeconds(30).Run() + command.New(o, virtualMachineRootCmd, "remove", "-f").WithoutCheckingExitCode().WithTimeoutInSeconds(20).Run() + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\ndatadisk: 50GiB")) + session := command.New(o, virtualMachineRootCmd, "init"). + WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + gomega.Expect(session).Should(gexec.Exit(0)) + gomega.Expect(session.Err).Should(gbytes.Say("Shrinking the data disk is not supported")) + }) + + ginkgo.It("should successfully resize data disk on vm start after config increase", func() { + resetVM(o) + resetDisks(o, installed) + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\ndatadisk: 50GiB")) + command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // Increase datadisk in config and restart + command.New(o, virtualMachineRootCmd, "stop").WithTimeoutInSeconds(90).Run() + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\ndatadisk: 60GiB")) + command.New(o, virtualMachineRootCmd, "start").WithTimeoutInSeconds(240).Run() + + // Verify new disk size + result := command.New(o, virtualMachineRootCmd, "disk", "info").Run() + gomega.Expect(result.Out.Contents()).To(gomega.ContainSubstring("60GiB")) + }) + + ginkgo.It("should validate boot and data disk sizes after vm init", func() { + resetVM(o) + resetDisks(o, installed) + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\nbootdisk: 110GiB\ndatadisk: 55GiB")) + command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // Validate bootdisk in override.yaml + overrideConfigFilePath := filepath.Join(limaDataDirPath(installed), "_config", "override.yaml") + cfgBuf, err := os.ReadFile(filepath.Clean(overrideConfigFilePath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + + var limaCfg limatype.LimaYAML + err = yaml.Unmarshal(cfgBuf, &limaCfg) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(limaCfg.Disk).ShouldNot(gomega.BeNil()) + gomega.Expect(*limaCfg.Disk).Should(gomega.Equal("110GiB")) + + // Validate datadisk via disk info + result := command.New(o, virtualMachineRootCmd, "disk", "info").Run() + gomega.Expect(result.Out.Contents()).To(gomega.ContainSubstring("55GiB")) + + // Validate after stop/start cycle + command.New(o, virtualMachineRootCmd, "stop").WithTimeoutInSeconds(90).Run() + command.New(o, virtualMachineRootCmd, "start").WithTimeoutInSeconds(240).Run() + + overrideBuf, err := os.ReadFile(filepath.Clean(overrideConfigFilePath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + err = yaml.Unmarshal(overrideBuf, &limaCfg) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(*limaCfg.Disk).Should(gomega.Equal("110GiB")) + + result = command.New(o, virtualMachineRootCmd, "disk", "info").Run() + gomega.Expect(result.Out.Contents()).To(gomega.ContainSubstring("55GiB")) + }) + }) +} diff --git a/e2e/vm/vm_darwin_test.go b/e2e/vm/vm_darwin_test.go index f9eb4ff95..bee75ec1f 100644 --- a/e2e/vm/vm_darwin_test.go +++ b/e2e/vm/vm_darwin_test.go @@ -80,6 +80,7 @@ func TestVM(t *testing.T) { testVMNetwork(o, *e2e.Installed) testDaemon(o, *e2e.Installed) testVMDisk(o) + testDiskConfig(o, *e2e.Installed) }) gomega.RegisterFailHandler(ginkgo.Fail) From 4cf43d2fb013f18ef3701ebd333e9503ed2e4454 Mon Sep 17 00:00:00 2001 From: Swapnanil-Gupta Date: Wed, 8 Apr 2026 14:29:02 -0700 Subject: [PATCH 5/5] Changes: - Add warnings that increasing datadisk size is irreversible without losing data - Add ability to rollback config changes if disk resize operation fails - Add a test to set size of disk to a very large amount Signed-off-by: Swapnanil-Gupta --- cmd/finch/virtual_machine_disk_resize.go | 27 ++++++++++++++++++------ e2e/vm/disk_config_test.go | 22 +++++++++++++++++++ e2e/vm/vm_disk_test.go | 23 ++++++++++++++++++++ pkg/disk/disk_darwin.go | 1 + 4 files changed, 66 insertions(+), 7 deletions(-) diff --git a/cmd/finch/virtual_machine_disk_resize.go b/cmd/finch/virtual_machine_disk_resize.go index 17f86eb01..5365611c1 100644 --- a/cmd/finch/virtual_machine_disk_resize.go +++ b/cmd/finch/virtual_machine_disk_resize.go @@ -57,25 +57,38 @@ func (dva *diskResizeVMAction) runAdapter(cmd *cobra.Command, _ []string) error } func (dva *diskResizeVMAction) run(size string) error { + dva.logger.Warnf("Increasing disk size is irreversible without losing data.") dva.logger.Infof("Resizing disk to %s...", size) - limaCmd := dva.creator.CreateWithoutStdio("disk", "resize", limaInstanceName, "--size", size) - output, err := limaCmd.CombinedOutput() + configPath := dva.limaConfigApplier.GetFinchConfigPath() + oldConfig, err := afero.ReadFile(dva.fs, configPath) if err != nil { - return fmt.Errorf("failed to resize disk: %w\n%s", err, output) + return fmt.Errorf("failed to read finch config: %w", err) } - dva.logger.Info("Disk resized successfully.") - isConfigUpdated, err := config.ModifyFinchConfig( dva.fs, dva.logger, - dva.limaConfigApplier.GetFinchConfigPath(), + configPath, config.VMConfigOpts{DataDisk: &size}, ) if err != nil { - return fmt.Errorf("disk resized but failed to update config: %w", err) + return fmt.Errorf("failed to update finch config: %w", err) } + + limaCmd := dva.creator.CreateWithoutStdio("disk", "resize", limaInstanceName, "--size", size) + output, err := limaCmd.CombinedOutput() + if err != nil { + if isConfigUpdated { + if writeErr := afero.WriteFile(dva.fs, configPath, oldConfig, 0o644); writeErr != nil { + return fmt.Errorf("failed to rollback config (disk resize failed): %w", writeErr) + } + dva.logger.Info("Configuration changes rolled back.") + } + return fmt.Errorf("failed to resize disk: %w\n%s", err, output) + } + + dva.logger.Info("Disk resized successfully.") if isConfigUpdated { dva.logger.Info("Configuration updated with new disk size.") } diff --git a/e2e/vm/disk_config_test.go b/e2e/vm/disk_config_test.go index 6cee574de..697e4983c 100644 --- a/e2e/vm/disk_config_test.go +++ b/e2e/vm/disk_config_test.go @@ -110,6 +110,28 @@ var testDiskConfig = func(o *option.Option, installed bool) { gomega.Expect(result.Out.Contents()).To(gomega.ContainSubstring("60GiB")) }) + ginkgo.It("should handle extra large sparse disk sizes", func() { + resetVM(o) + resetDisks(o, installed) + writeFile(finchConfigFilePath, []byte("memory: 4GiB\ncpus: 2\nbootdisk: 1000GiB\ndatadisk: 1000GiB")) + command.New(o, virtualMachineRootCmd, "init").WithoutCheckingExitCode().WithTimeoutInSeconds(240).Run() + + // Verify bootdisk is set in override.yaml + overrideConfigFilePath := filepath.Join(limaDataDirPath(installed), "_config", "override.yaml") + cfgBuf, err := os.ReadFile(filepath.Clean(overrideConfigFilePath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + + var limaCfg limatype.LimaYAML + err = yaml.Unmarshal(cfgBuf, &limaCfg) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(limaCfg.Disk).ShouldNot(gomega.BeNil()) + gomega.Expect(*limaCfg.Disk).Should(gomega.Equal("1000GiB")) + + // Verify datadisk size - disks are sparse so this should succeed + result := command.New(o, virtualMachineRootCmd, "disk", "info").Run() + gomega.Expect(result.Out.Contents()).To(gomega.ContainSubstring("1000GiB")) + }) + ginkgo.It("should validate boot and data disk sizes after vm init", func() { resetVM(o) resetDisks(o, installed) diff --git a/e2e/vm/vm_disk_test.go b/e2e/vm/vm_disk_test.go index 6b228ea1a..db5af149b 100644 --- a/e2e/vm/vm_disk_test.go +++ b/e2e/vm/vm_disk_test.go @@ -6,6 +6,8 @@ package vm import ( + "os" + "path/filepath" "time" "github.com/onsi/ginkgo/v2" @@ -64,6 +66,27 @@ func testVMDisk(o *option.Option) { ginkgo.It("should fail with invalid size format", func() { command.New(o, virtualMachineRootCmd, "disk", "resize", "--size", "60aib").WithoutSuccessfulExit().Run() }) + + ginkgo.It("should update config after successful resize", func() { + command.New(o, virtualMachineRootCmd, "disk", "resize", "--size", "60GiB").Run() + + cfgBuf, err := os.ReadFile(filepath.Clean(finchConfigFilePath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(string(cfgBuf)).Should(gomega.ContainSubstring("datadisk: 60GiB")) + }) + + ginkgo.It("should rollback config on resize failure", func() { + oldConfig, err := os.ReadFile(filepath.Clean(finchConfigFilePath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Start the vm so resize will fail + command.New(o, virtualMachineRootCmd, "start").WithTimeoutInSeconds(240).Run() + command.New(o, virtualMachineRootCmd, "disk", "resize", "--size", "70GiB").WithoutSuccessfulExit().Run() + + newConfig, err := os.ReadFile(filepath.Clean(finchConfigFilePath)) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(string(newConfig)).Should(gomega.Equal(string(oldConfig))) + }) }) }) } diff --git a/pkg/disk/disk_darwin.go b/pkg/disk/disk_darwin.go index 2c5be4a91..ba1df2ae0 100644 --- a/pkg/disk/disk_darwin.go +++ b/pkg/disk/disk_darwin.go @@ -250,6 +250,7 @@ func (m *userDataDiskManager) resizeDiskIfNeeded() error { } m.logger.Infof("Resizing data disk from %s to %s", units.BytesSize(float64(actualDataDiskBytes)), size) + m.logger.Warnf("Increasing disk size is irreversible without losing data.") cmd := m.ncc.CreateWithoutStdio("disk", "resize", diskName, "--size", size) if logs, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to resize disk to %s, debug logs:\n%s", size, logs)