Skip to content

Commit d499a7d

Browse files
sam-berningsam-berning
authored
feat: adds a --force flag to vm stop and remove (#178)
Signed-off-by: Sam Berning <[email protected]> Issue #, if available: #171 *Description of changes:* Adds a force flag to `finch vm stop` and `finch vm remove`. This gives us a native way to recover from "unrecognized system status" as seen in #171. *Testing done:* Unit testing, E2E testing. Force "unrecognized system status" by messing with the Lima instance ``` $ ./_output/bin/finch vm stop FATA[0000] unrecognized system status $ ./_output/bin/finch vm stop --force INFO[0000] Forcibly stopping Finch virtual machine... INFO[0001] Finch virtual machine stopped successfully $ ./_output/bin/finch vm start INFO[0000] Starting existing Finch virtual machine... INFO[0038] Finch virtual machine started successfully ``` ``` $ ./_output/bin/finch vm stop FATA[0000] unrecognized system status $ ./_output/bin/finch vm remove --force INFO[0000] Forcibly removing Finch virtual machine... INFO[0000] Finch virtual machine removed successfully $ ./_output/bin/finch vm status Nonexistent ``` - [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: Sam Berning <[email protected]>
1 parent 903a1d4 commit d499a7d

5 files changed

+165
-39
lines changed

Diff for: ‎cmd/finch/virtual_machine_remove.go

+36-11
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ func newRemoveVMCommand(limaCmdCreator command.LimaCmdCreator, logger flog.Logge
2121
RunE: newRemoveVMAction(limaCmdCreator, logger).runAdapter,
2222
}
2323

24+
removeVMCommand.Flags().BoolP("force", "f", false, "forcibly remove finch VM")
25+
2426
return removeVMCommand
2527
}
2628

@@ -34,24 +36,24 @@ func newRemoveVMAction(creator command.LimaCmdCreator, logger flog.Logger) *remo
3436
}
3537

3638
func (rva *removeVMAction) runAdapter(cmd *cobra.Command, args []string) error {
37-
return rva.run()
38-
}
39-
40-
func (rva *removeVMAction) run() error {
41-
err := rva.assertVMIsStopped(rva.creator, rva.logger)
39+
force, err := cmd.Flags().GetBool("force")
4240
if err != nil {
4341
return err
4442
}
43+
return rva.run(force)
44+
}
4545

46-
limaCmd := rva.creator.CreateWithoutStdio("remove", limaInstanceName)
47-
rva.logger.Info("Removing existing Finch virtual machine...")
48-
logs, err := limaCmd.CombinedOutput()
46+
func (rva *removeVMAction) run(force bool) error {
47+
if force {
48+
return rva.removeVM(force)
49+
}
50+
51+
err := rva.assertVMIsStopped(rva.creator, rva.logger)
4952
if err != nil {
50-
rva.logger.Errorf("Finch virtual machine failed to remove, debug logs: %s", logs)
5153
return err
5254
}
53-
rva.logger.Info("Finch virtual machine removed successfully")
54-
return nil
55+
56+
return rva.removeVM(false)
5557
}
5658

5759
func (rva *removeVMAction) assertVMIsStopped(creator command.LimaCmdCreator, logger flog.Logger) error {
@@ -69,3 +71,26 @@ func (rva *removeVMAction) assertVMIsStopped(creator command.LimaCmdCreator, log
6971
return nil
7072
}
7173
}
74+
75+
func (rva *removeVMAction) removeVM(force bool) error {
76+
limaCmd := rva.createVMRemoveCommand(force)
77+
if force {
78+
rva.logger.Info("Forcibly removing Finch virtual machine...")
79+
} else {
80+
rva.logger.Info("Removing existing Finch virtual machine...")
81+
}
82+
logs, err := limaCmd.CombinedOutput()
83+
if err != nil {
84+
rva.logger.Errorf("Finch virtual machine failed to remove, debug logs: %s", logs)
85+
return err
86+
}
87+
rva.logger.Info("Finch virtual machine removed successfully")
88+
return nil
89+
}
90+
91+
func (rva *removeVMAction) createVMRemoveCommand(force bool) command.Command {
92+
if force {
93+
return rva.creator.CreateWithoutStdio("remove", "--force", limaInstanceName)
94+
}
95+
return rva.creator.CreateWithoutStdio("remove", limaInstanceName)
96+
}

Diff for: ‎cmd/finch/virtual_machine_remove_test.go

+36-8
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/runfinch/finch/pkg/mocks"
1212

1313
"github.com/golang/mock/gomock"
14-
"github.com/spf13/cobra"
1514
"github.com/stretchr/testify/assert"
1615
)
1716

@@ -28,14 +27,10 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {
2827
testCases := []struct {
2928
name string
3029
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller)
31-
cmd *cobra.Command
3230
args []string
3331
}{
3432
{
3533
name: "should remove the instance",
36-
cmd: &cobra.Command{
37-
Use: "remove",
38-
},
3934
args: []string{},
4035
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
4136
getVMStatusC := mocks.NewCommand(ctrl)
@@ -49,6 +44,18 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {
4944
logger.EXPECT().Info(gomock.Any()).AnyTimes()
5045
},
5146
},
47+
{
48+
name: "should forcibly remove the instance",
49+
args: []string{
50+
"--force",
51+
},
52+
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
53+
command := mocks.NewCommand(ctrl)
54+
creator.EXPECT().CreateWithoutStdio("remove", "--force", limaInstanceName).Return(command)
55+
command.EXPECT().CombinedOutput()
56+
logger.EXPECT().Info(gomock.Any()).AnyTimes()
57+
},
58+
},
5259
}
5360

5461
for _, tc := range testCases {
@@ -59,9 +66,11 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {
5966
ctrl := gomock.NewController(t)
6067
logger := mocks.NewLogger(ctrl)
6168
lcc := mocks.NewLimaCmdCreator(ctrl)
62-
6369
tc.mockSvc(logger, lcc, ctrl)
64-
assert.NoError(t, newRemoveVMAction(lcc, logger).runAdapter(tc.cmd, tc.args))
70+
71+
cmd := newRemoveVMCommand(lcc, logger)
72+
cmd.SetArgs(tc.args)
73+
assert.NoError(t, cmd.Execute())
6574
})
6675
}
6776
}
@@ -73,6 +82,7 @@ func TestRemoveVMAction_run(t *testing.T) {
7382
name string
7483
wantErr error
7584
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller)
85+
force bool
7686
}{
7787
{
7888
name: "should remove the instance",
@@ -89,6 +99,7 @@ func TestRemoveVMAction_run(t *testing.T) {
8999
logger.EXPECT().Info("Removing existing Finch virtual machine...")
90100
logger.EXPECT().Info("Finch virtual machine removed successfully")
91101
},
102+
force: false,
92103
},
93104
{
94105
name: "running VM",
@@ -100,6 +111,7 @@ func TestRemoveVMAction_run(t *testing.T) {
100111
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
101112
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
102113
},
114+
force: false,
103115
},
104116
{
105117
name: "nonexistent VM",
@@ -110,6 +122,7 @@ func TestRemoveVMAction_run(t *testing.T) {
110122
getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
111123
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
112124
},
125+
force: false,
113126
},
114127
{
115128
name: "unknown VM status",
@@ -120,6 +133,7 @@ func TestRemoveVMAction_run(t *testing.T) {
120133
getVMStatusC.EXPECT().Output().Return([]byte("Broken"), nil)
121134
logger.EXPECT().Debugf("Status of virtual machine: %s", "Broken")
122135
},
136+
force: false,
123137
},
124138
{
125139
name: "status command returns an error",
@@ -129,6 +143,7 @@ func TestRemoveVMAction_run(t *testing.T) {
129143
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
130144
getVMStatusC.EXPECT().Output().Return([]byte("Broken"), errors.New("get status error"))
131145
},
146+
force: false,
132147
},
133148
{
134149
name: "should print error if virtual machine failed to remove",
@@ -146,6 +161,19 @@ func TestRemoveVMAction_run(t *testing.T) {
146161
logger.EXPECT().Info("Removing existing Finch virtual machine...")
147162
logger.EXPECT().Errorf("Finch virtual machine failed to remove, debug logs: %s", logs)
148163
},
164+
force: false,
165+
},
166+
{
167+
name: "should forcibly remove the instance",
168+
wantErr: nil,
169+
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
170+
command := mocks.NewCommand(ctrl)
171+
creator.EXPECT().CreateWithoutStdio("remove", "--force", limaInstanceName).Return(command)
172+
command.EXPECT().CombinedOutput()
173+
logger.EXPECT().Info("Forcibly removing Finch virtual machine...")
174+
logger.EXPECT().Info("Finch virtual machine removed successfully")
175+
},
176+
force: true,
149177
},
150178
}
151179

@@ -159,7 +187,7 @@ func TestRemoveVMAction_run(t *testing.T) {
159187
lcc := mocks.NewLimaCmdCreator(ctrl)
160188

161189
tc.mockSvc(logger, lcc, ctrl)
162-
err := newRemoveVMAction(lcc, logger).run()
190+
err := newRemoveVMAction(lcc, logger).run(tc.force)
163191
assert.Equal(t, tc.wantErr, err)
164192
})
165193
}

Diff for: ‎cmd/finch/virtual_machine_stop.go

+36-11
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ func newStopVMCommand(limaCmdCreator command.LimaCmdCreator, logger flog.Logger)
2020
RunE: newStopVMAction(limaCmdCreator, logger).runAdapter,
2121
}
2222

23+
stopVMCommand.Flags().BoolP("force", "f", false, "forcibly stop finch VM")
24+
2325
return stopVMCommand
2426
}
2527

@@ -33,24 +35,24 @@ func newStopVMAction(creator command.LimaCmdCreator, logger flog.Logger) *stopVM
3335
}
3436

3537
func (sva *stopVMAction) runAdapter(cmd *cobra.Command, args []string) error {
36-
return sva.run()
37-
}
38-
39-
func (sva *stopVMAction) run() error {
40-
err := sva.assertVMIsRunning(sva.creator, sva.logger)
38+
force, err := cmd.Flags().GetBool("force")
4139
if err != nil {
4240
return err
4341
}
42+
return sva.run(force)
43+
}
4444

45-
limaCmd := sva.creator.CreateWithoutStdio("stop", limaInstanceName)
46-
sva.logger.Info("Stopping existing Finch virtual machine...")
47-
logs, err := limaCmd.CombinedOutput()
45+
func (sva *stopVMAction) run(force bool) error {
46+
if force {
47+
return sva.stopVM(force)
48+
}
49+
50+
err := sva.assertVMIsRunning(sva.creator, sva.logger)
4851
if err != nil {
49-
sva.logger.Errorf("Finch virtual machine failed to stop, debug logs: %s", logs)
5052
return err
5153
}
52-
sva.logger.Info("Finch virtual machine stopped successfully")
53-
return nil
54+
55+
return sva.stopVM(false)
5456
}
5557

5658
func (sva *stopVMAction) assertVMIsRunning(creator command.LimaCmdCreator, logger flog.Logger) error {
@@ -67,3 +69,26 @@ func (sva *stopVMAction) assertVMIsRunning(creator command.LimaCmdCreator, logge
6769
return nil
6870
}
6971
}
72+
73+
func (sva *stopVMAction) stopVM(force bool) error {
74+
limaCmd := sva.createLimaStopCommand(force)
75+
if force {
76+
sva.logger.Info("Forcibly stopping Finch virtual machine...")
77+
} else {
78+
sva.logger.Info("Stopping existing Finch virtual machine...")
79+
}
80+
logs, err := limaCmd.CombinedOutput()
81+
if err != nil {
82+
sva.logger.Errorf("Finch virtual machine failed to stop, debug logs: %s", logs)
83+
return err
84+
}
85+
sva.logger.Info("Finch virtual machine stopped successfully")
86+
return nil
87+
}
88+
89+
func (sva *stopVMAction) createLimaStopCommand(force bool) command.Command {
90+
if force {
91+
return sva.creator.CreateWithoutStdio("stop", "--force", limaInstanceName)
92+
}
93+
return sva.creator.CreateWithoutStdio("stop", limaInstanceName)
94+
}

0 commit comments

Comments
 (0)