Skip to content

Commit 8e809cc

Browse files
authored
fix: add finch vm settings subcommand (runfinch#887)
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. - runfinch#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: runfinch#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]>
1 parent f46afb9 commit 8e809cc

9 files changed

+726
-5
lines changed

Diff for: cmd/finch/virtual_machine.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func newVirtualMachineCommand(
4949
newStatusVMCommand(limaCmdCreator, logger, os.Stdout),
5050
newInitVMCommand(limaCmdCreator, logger, optionalDepGroups, lca, nca, fp.BaseYamlFilePath(), fs,
5151
fp.LimaSSHPrivateKeyPath(), diskManager),
52+
newSettingsVMCommand(logger, lca, fs, os.Stdout),
5253
)
5354

5455
return virtualMachineCommand
@@ -109,7 +110,15 @@ func virtualMachineCommands(
109110
lcc,
110111
logger,
111112
dependencies(ecc, fc, fp, fs, lcc, logger, fp.FinchDir(finchRootPath)),
112-
config.NewLimaApplier(fc, ecc, fs, fp.LimaDefaultConfigPath(), fp.LimaOverrideConfigPath(), system.NewStdLib()),
113+
config.NewLimaApplier(
114+
fc,
115+
ecc,
116+
fs,
117+
fp.LimaDefaultConfigPath(),
118+
fp.LimaOverrideConfigPath(),
119+
system.NewStdLib(),
120+
fp.ConfigFilePath(finchRootPath),
121+
),
113122
config.NewNerdctlApplier(
114123
fssh.NewDialer(),
115124
fs,

Diff for: cmd/finch/virtual_machine_settings.go

+99
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package main
5+
6+
import (
7+
"fmt"
8+
"io"
9+
10+
"github.com/runfinch/finch/pkg/config"
11+
"github.com/runfinch/finch/pkg/flog"
12+
13+
"github.com/spf13/afero"
14+
"github.com/spf13/cobra"
15+
)
16+
17+
func newSettingsVMCommand(
18+
logger flog.Logger,
19+
lca config.LimaConfigApplier,
20+
fs afero.Fs,
21+
stdout io.Writer,
22+
) *cobra.Command {
23+
settingsVMCommand := &cobra.Command{
24+
Use: "settings",
25+
Short: "Configure the virtual machine instance",
26+
RunE: newSettingsVMAction(logger, lca, fs, stdout).runAdapter,
27+
}
28+
29+
settingsVMCommand.Flags().Int(
30+
"cpus",
31+
config.DefaultCPUs,
32+
"the amount of vCPU to dedicate to the virtual machine (restart the vm when applying this change.)",
33+
)
34+
settingsVMCommand.Flags().String(
35+
"memory",
36+
config.DefaultMemory,
37+
"the amount of memory to dedicate to the virtual machine (restart the vm when applying this change.)",
38+
)
39+
40+
return settingsVMCommand
41+
}
42+
43+
type settingsVMAction struct {
44+
logger flog.Logger
45+
limaConfigApplier config.LimaConfigApplier
46+
fs afero.Fs
47+
stdout io.Writer
48+
}
49+
50+
func newSettingsVMAction(
51+
logger flog.Logger,
52+
lca config.LimaConfigApplier,
53+
fs afero.Fs,
54+
stdout io.Writer,
55+
) *settingsVMAction {
56+
return &settingsVMAction{
57+
logger: logger,
58+
limaConfigApplier: lca,
59+
fs: fs,
60+
stdout: stdout,
61+
}
62+
}
63+
64+
func (sva *settingsVMAction) runAdapter(cmd *cobra.Command, _ []string) error {
65+
cpus, err := cmd.Flags().GetInt("cpus")
66+
if err != nil {
67+
return err
68+
}
69+
70+
memory, err := cmd.Flags().GetString("memory")
71+
if err != nil {
72+
return err
73+
}
74+
75+
opts := config.VMConfigOpts{
76+
CPUs: cpus,
77+
Memory: memory,
78+
}
79+
80+
return sva.run(opts)
81+
}
82+
83+
func (sva *settingsVMAction) run(opts config.VMConfigOpts) error {
84+
isConfigUpdated, err := config.ModifyFinchConfig(
85+
sva.fs,
86+
sva.logger,
87+
sva.limaConfigApplier.GetFinchConfigPath(),
88+
opts,
89+
)
90+
if err != nil {
91+
return err
92+
}
93+
94+
if isConfigUpdated {
95+
fmt.Fprintln(sva.stdout, "Configurations have been successfully updated.")
96+
}
97+
98+
return nil
99+
}

Diff for: cmd/finch/virtual_machine_settings_test.go

+214
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package main
5+
6+
import (
7+
"bytes"
8+
"errors"
9+
"testing"
10+
11+
"github.com/golang/mock/gomock"
12+
"github.com/spf13/afero"
13+
"github.com/spf13/cobra"
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/require"
16+
17+
"github.com/runfinch/finch/pkg/config"
18+
"github.com/runfinch/finch/pkg/mocks"
19+
)
20+
21+
func TestNewSettingsMCommand(t *testing.T) {
22+
t.Parallel()
23+
24+
cmd := newSettingsVMCommand(nil, nil, nil, nil)
25+
assert.Equal(t, cmd.Name(), "settings")
26+
}
27+
28+
func TestSettingsVMAction_runAdapter(t *testing.T) {
29+
t.Parallel()
30+
31+
testCases := []struct {
32+
name string
33+
wantErr error
34+
command *cobra.Command
35+
args []string
36+
mockSvc func(
37+
*mocks.LimaConfigApplier,
38+
afero.Fs,
39+
)
40+
}{
41+
{
42+
name: "should configure the instance for valid CPU and memory values",
43+
wantErr: nil,
44+
command: &cobra.Command{
45+
Use: "settings",
46+
},
47+
args: []string{
48+
"--cpus=1",
49+
"--memory=2GiB",
50+
},
51+
mockSvc: func(
52+
lca *mocks.LimaConfigApplier,
53+
fs afero.Fs,
54+
) {
55+
finchConfigPath := "/config.yaml"
56+
data := "cpus: 2\nmemory: 6GiB"
57+
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))
58+
59+
lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
60+
},
61+
},
62+
{
63+
name: "should configure the instance for valid CPU value",
64+
wantErr: nil,
65+
command: &cobra.Command{
66+
Use: "settings",
67+
},
68+
args: []string{
69+
"--cpus=1",
70+
},
71+
mockSvc: func(
72+
lca *mocks.LimaConfigApplier,
73+
fs afero.Fs,
74+
) {
75+
finchConfigPath := "/config.yaml"
76+
data := "cpus: 2\nmemory: 6GiB"
77+
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))
78+
79+
lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
80+
},
81+
},
82+
{
83+
name: "should configure the instance for valid memory value",
84+
wantErr: nil,
85+
command: &cobra.Command{
86+
Use: "settings",
87+
},
88+
args: []string{
89+
"--memory=2GiB",
90+
},
91+
mockSvc: func(
92+
lca *mocks.LimaConfigApplier,
93+
fs afero.Fs,
94+
) {
95+
finchConfigPath := "/config.yaml"
96+
data := "cpus: 2\nmemory: 6GiB"
97+
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))
98+
99+
lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
100+
},
101+
},
102+
}
103+
104+
for _, tc := range testCases {
105+
tc := tc
106+
t.Run(tc.name, func(t *testing.T) {
107+
t.Parallel()
108+
109+
ctrl := gomock.NewController(t)
110+
logger := mocks.NewLogger(ctrl)
111+
lca := mocks.NewLimaConfigApplier(ctrl)
112+
fs := afero.NewMemMapFs()
113+
stdout := bytes.Buffer{}
114+
115+
tc.mockSvc(lca, fs)
116+
117+
cmd := newSettingsVMCommand(logger, lca, fs, &stdout)
118+
cmd.SetArgs(tc.args)
119+
err := cmd.Execute()
120+
assert.Equal(t, err, tc.wantErr)
121+
})
122+
}
123+
}
124+
125+
func TestSettingsVMAction_run(t *testing.T) {
126+
t.Parallel()
127+
128+
testCases := []struct {
129+
name string
130+
wantErr error
131+
wantStatusOutput string
132+
mockSvc func(
133+
*mocks.LimaConfigApplier,
134+
afero.Fs,
135+
)
136+
cpus int
137+
memory string
138+
}{
139+
{
140+
name: "should update vm settings",
141+
wantErr: nil,
142+
wantStatusOutput: "Configurations have been successfully updated.\n",
143+
mockSvc: func(
144+
lca *mocks.LimaConfigApplier,
145+
fs afero.Fs,
146+
) {
147+
finchConfigPath := "/config.yaml"
148+
data := "cpus: 2\nmemory: 6GiB"
149+
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))
150+
151+
lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
152+
},
153+
cpus: 1,
154+
memory: "2GiB",
155+
},
156+
{
157+
name: "should return an error if the configuration of CPU or memory is invalid",
158+
wantErr: errors.New("the number of CPUs or the amount of memory should be at least one valid value"),
159+
wantStatusOutput: "",
160+
mockSvc: func(
161+
lca *mocks.LimaConfigApplier,
162+
fs afero.Fs,
163+
) {
164+
finchConfigPath := "/config.yaml"
165+
data := "cpus: 2\nmemory: 6GiB"
166+
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))
167+
168+
lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
169+
},
170+
cpus: 0,
171+
memory: "",
172+
},
173+
{
174+
name: "should return an error if the configuration of CPU or memory is invalid",
175+
wantErr: errors.New("the number of CPUs or the amount of memory should be at least one valid value"),
176+
wantStatusOutput: "",
177+
mockSvc: func(
178+
lca *mocks.LimaConfigApplier,
179+
fs afero.Fs,
180+
) {
181+
finchConfigPath := "/config.yaml"
182+
data := "cpus: 2\nmemory: 6GiB"
183+
require.NoError(t, afero.WriteFile(fs, finchConfigPath, []byte(data), 0o600))
184+
185+
lca.EXPECT().GetFinchConfigPath().Return(finchConfigPath)
186+
},
187+
cpus: 2,
188+
memory: "6GiB",
189+
},
190+
}
191+
192+
for _, tc := range testCases {
193+
tc := tc
194+
t.Run(tc.name, func(t *testing.T) {
195+
t.Parallel()
196+
197+
ctrl := gomock.NewController(t)
198+
logger := mocks.NewLogger(ctrl)
199+
lca := mocks.NewLimaConfigApplier(ctrl)
200+
fs := afero.NewMemMapFs()
201+
stdout := bytes.Buffer{}
202+
opts := config.VMConfigOpts{
203+
CPUs: tc.cpus,
204+
Memory: tc.memory,
205+
}
206+
207+
tc.mockSvc(lca, fs)
208+
209+
err := newSettingsVMAction(logger, lca, fs, &stdout).run(opts)
210+
assert.Equal(t, err, tc.wantErr)
211+
assert.Equal(t, tc.wantStatusOutput, stdout.String())
212+
})
213+
}
214+
}

Diff for: cmd/finch/virtual_machine_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func TestVirtualMachineCommand(t *testing.T) {
2121
assert.Equal(t, cmd.Use, virtualMachineRootCmd)
2222

2323
// check the number of subcommand for vm
24-
assert.Equal(t, len(cmd.Commands()), 5)
24+
assert.Equal(t, len(cmd.Commands()), 6)
2525
}
2626

2727
func TestPostVMStartInitAction_runAdapter(t *testing.T) {

0 commit comments

Comments
 (0)