Skip to content

Commit

Permalink
fix: add finch vm settings subcommand (#887)
Browse files Browse the repository at this point in the history
The current implementation requires updating ~/.finch/finch.yaml to
change the CPU and Memory settings in the VM.

However, the following issue provides a feature request to set the CPU
and memory resources to be allocated to the VM from CLI options.

- #683

Therefore, this fix adds the finch vm settings command to add the
ability to set the CPU and memory resources allocated to VMs from the
CLI.

Issue #, if available: #683

*Description of changes:* Details are described in this commit message.

*Testing done:* Yes



- [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: Hayato Kiwata <[email protected]>
  • Loading branch information
haytok authored Apr 19, 2024
1 parent f46afb9 commit 8e809cc
Show file tree
Hide file tree
Showing 9 changed files with 726 additions and 5 deletions.
11 changes: 10 additions & 1 deletion cmd/finch/virtual_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func newVirtualMachineCommand(
newStatusVMCommand(limaCmdCreator, logger, os.Stdout),
newInitVMCommand(limaCmdCreator, logger, optionalDepGroups, lca, nca, fp.BaseYamlFilePath(), fs,
fp.LimaSSHPrivateKeyPath(), diskManager),
newSettingsVMCommand(logger, lca, fs, os.Stdout),
)

return virtualMachineCommand
Expand Down Expand Up @@ -109,7 +110,15 @@ func virtualMachineCommands(
lcc,
logger,
dependencies(ecc, fc, fp, fs, lcc, logger, fp.FinchDir(finchRootPath)),
config.NewLimaApplier(fc, ecc, fs, fp.LimaDefaultConfigPath(), fp.LimaOverrideConfigPath(), system.NewStdLib()),
config.NewLimaApplier(
fc,
ecc,
fs,
fp.LimaDefaultConfigPath(),
fp.LimaOverrideConfigPath(),
system.NewStdLib(),
fp.ConfigFilePath(finchRootPath),
),
config.NewNerdctlApplier(
fssh.NewDialer(),
fs,
Expand Down
99 changes: 99 additions & 0 deletions cmd/finch/virtual_machine_settings.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package main

import (
"fmt"
"io"

"github.com/runfinch/finch/pkg/config"
"github.com/runfinch/finch/pkg/flog"

"github.com/spf13/afero"
"github.com/spf13/cobra"
)

func newSettingsVMCommand(
logger flog.Logger,
lca config.LimaConfigApplier,
fs afero.Fs,
stdout io.Writer,
) *cobra.Command {
settingsVMCommand := &cobra.Command{
Use: "settings",
Short: "Configure the virtual machine instance",
RunE: newSettingsVMAction(logger, lca, fs, stdout).runAdapter,
}

settingsVMCommand.Flags().Int(
"cpus",
config.DefaultCPUs,
"the amount of vCPU to dedicate to the virtual machine (restart the vm when applying this change.)",
)
settingsVMCommand.Flags().String(
"memory",
config.DefaultMemory,
"the amount of memory to dedicate to the virtual machine (restart the vm when applying this change.)",
)

return settingsVMCommand
}

type settingsVMAction struct {
logger flog.Logger
limaConfigApplier config.LimaConfigApplier
fs afero.Fs
stdout io.Writer
}

func newSettingsVMAction(
logger flog.Logger,
lca config.LimaConfigApplier,
fs afero.Fs,
stdout io.Writer,
) *settingsVMAction {
return &settingsVMAction{
logger: logger,
limaConfigApplier: lca,
fs: fs,
stdout: stdout,
}
}

func (sva *settingsVMAction) runAdapter(cmd *cobra.Command, _ []string) error {
cpus, err := cmd.Flags().GetInt("cpus")
if err != nil {
return err
}

memory, err := cmd.Flags().GetString("memory")
if err != nil {
return err
}

opts := config.VMConfigOpts{
CPUs: cpus,
Memory: memory,
}

return sva.run(opts)
}

func (sva *settingsVMAction) run(opts config.VMConfigOpts) error {
isConfigUpdated, err := config.ModifyFinchConfig(
sva.fs,
sva.logger,
sva.limaConfigApplier.GetFinchConfigPath(),
opts,
)
if err != nil {
return err
}

if isConfigUpdated {
fmt.Fprintln(sva.stdout, "Configurations have been successfully updated.")
}

return nil
}
214 changes: 214 additions & 0 deletions cmd/finch/virtual_machine_settings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package main

import (
"bytes"
"errors"
"testing"

"github.com/golang/mock/gomock"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/runfinch/finch/pkg/config"
"github.com/runfinch/finch/pkg/mocks"
)

func TestNewSettingsMCommand(t *testing.T) {
t.Parallel()

cmd := newSettingsVMCommand(nil, nil, nil, nil)
assert.Equal(t, cmd.Name(), "settings")
}

func TestSettingsVMAction_runAdapter(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
wantErr error
command *cobra.Command
args []string
mockSvc func(
*mocks.LimaConfigApplier,
afero.Fs,
)
}{
{
name: "should configure the instance for valid CPU and memory values",
wantErr: nil,
command: &cobra.Command{
Use: "settings",
},
args: []string{
"--cpus=1",
"--memory=2GiB",
},
mockSvc: func(
lca *mocks.LimaConfigApplier,
fs afero.Fs,
) {
finchConfigPath := "/config.yaml"
data := "cpus: 2\nmemory: 6GiB"
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))

lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
},
},
{
name: "should configure the instance for valid CPU value",
wantErr: nil,
command: &cobra.Command{
Use: "settings",
},
args: []string{
"--cpus=1",
},
mockSvc: func(
lca *mocks.LimaConfigApplier,
fs afero.Fs,
) {
finchConfigPath := "/config.yaml"
data := "cpus: 2\nmemory: 6GiB"
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))

lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
},
},
{
name: "should configure the instance for valid memory value",
wantErr: nil,
command: &cobra.Command{
Use: "settings",
},
args: []string{
"--memory=2GiB",
},
mockSvc: func(
lca *mocks.LimaConfigApplier,
fs afero.Fs,
) {
finchConfigPath := "/config.yaml"
data := "cpus: 2\nmemory: 6GiB"
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))

lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
},
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
logger := mocks.NewLogger(ctrl)
lca := mocks.NewLimaConfigApplier(ctrl)
fs := afero.NewMemMapFs()
stdout := bytes.Buffer{}

tc.mockSvc(lca, fs)

cmd := newSettingsVMCommand(logger, lca, fs, &stdout)
cmd.SetArgs(tc.args)
err := cmd.Execute()
assert.Equal(t, err, tc.wantErr)
})
}
}

func TestSettingsVMAction_run(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
wantErr error
wantStatusOutput string
mockSvc func(
*mocks.LimaConfigApplier,
afero.Fs,
)
cpus int
memory string
}{
{
name: "should update vm settings",
wantErr: nil,
wantStatusOutput: "Configurations have been successfully updated.\n",
mockSvc: func(
lca *mocks.LimaConfigApplier,
fs afero.Fs,
) {
finchConfigPath := "/config.yaml"
data := "cpus: 2\nmemory: 6GiB"
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))

lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
},
cpus: 1,
memory: "2GiB",
},
{
name: "should return an error if the configuration of CPU or memory is invalid",
wantErr: errors.New("the number of CPUs or the amount of memory should be at least one valid value"),
wantStatusOutput: "",
mockSvc: func(
lca *mocks.LimaConfigApplier,
fs afero.Fs,
) {
finchConfigPath := "/config.yaml"
data := "cpus: 2\nmemory: 6GiB"
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))

lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
},
cpus: 0,
memory: "",
},
{
name: "should return an error if the configuration of CPU or memory is invalid",
wantErr: errors.New("the number of CPUs or the amount of memory should be at least one valid value"),
wantStatusOutput: "",
mockSvc: func(
lca *mocks.LimaConfigApplier,
fs afero.Fs,
) {
finchConfigPath := "/config.yaml"
data := "cpus: 2\nmemory: 6GiB"
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))

lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
},
cpus: 2,
memory: "6GiB",
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
logger := mocks.NewLogger(ctrl)
lca := mocks.NewLimaConfigApplier(ctrl)
fs := afero.NewMemMapFs()
stdout := bytes.Buffer{}
opts := config.VMConfigOpts{
CPUs: tc.cpus,
Memory: tc.memory,
}

tc.mockSvc(lca, fs)

err := newSettingsVMAction(logger, lca, fs, &stdout).run(opts)
assert.Equal(t, err, tc.wantErr)
assert.Equal(t, tc.wantStatusOutput, stdout.String())
})
}
}
2 changes: 1 addition & 1 deletion cmd/finch/virtual_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestVirtualMachineCommand(t *testing.T) {
assert.Equal(t, cmd.Use, virtualMachineRootCmd)

// check the number of subcommand for vm
assert.Equal(t, len(cmd.Commands()), 5)
assert.Equal(t, len(cmd.Commands()), 6)
}

func TestPostVMStartInitAction_runAdapter(t *testing.T) {
Expand Down
Loading

0 comments on commit 8e809cc

Please sign in to comment.