Skip to content

Commit

Permalink
feat: adds a --force flag to vm stop and remove (runfinch#178)
Browse files Browse the repository at this point in the history 
Signed-off-by: Sam Berning <[email protected]>

Issue #, if available: runfinch#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
runfinch#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]>
  • Loading branch information
@sam-berning
sam-berning authored Jan 27, 2023
1 parent 903a1d4 commit d499a7d
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 39 deletions.
47 changes: 36 additions & 11 deletions cmd/finch/virtual_machine_remove.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ func newRemoveVMCommand(limaCmdCreator command.LimaCmdCreator, logger flog.Logge
RunE: newRemoveVMAction(limaCmdCreator, logger).runAdapter,
}

removeVMCommand.Flags().BoolP("force", "f", false, "forcibly remove finch VM")

return removeVMCommand
}

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

func (rva *removeVMAction) runAdapter(cmd *cobra.Command, args []string) error {
return rva.run()
}

func (rva *removeVMAction) run() error {
err := rva.assertVMIsStopped(rva.creator, rva.logger)
force, err := cmd.Flags().GetBool("force")
if err != nil {
return err
}
return rva.run(force)
}

limaCmd := rva.creator.CreateWithoutStdio("remove", limaInstanceName)
rva.logger.Info("Removing existing Finch virtual machine...")
logs, err := limaCmd.CombinedOutput()
func (rva *removeVMAction) run(force bool) error {
if force {
return rva.removeVM(force)
}

err := rva.assertVMIsStopped(rva.creator, rva.logger)
if err != nil {
rva.logger.Errorf("Finch virtual machine failed to remove, debug logs: %s", logs)
return err
}
rva.logger.Info("Finch virtual machine removed successfully")
return nil

return rva.removeVM(false)
}

func (rva *removeVMAction) assertVMIsStopped(creator command.LimaCmdCreator, logger flog.Logger) error {
Expand All @@ -69,3 +71,26 @@ func (rva *removeVMAction) assertVMIsStopped(creator command.LimaCmdCreator, log
return nil
}
}

func (rva *removeVMAction) removeVM(force bool) error {
limaCmd := rva.createVMRemoveCommand(force)
if force {
rva.logger.Info("Forcibly removing Finch virtual machine...")
} else {
rva.logger.Info("Removing existing Finch virtual machine...")
}
logs, err := limaCmd.CombinedOutput()
if err != nil {
rva.logger.Errorf("Finch virtual machine failed to remove, debug logs: %s", logs)
return err
}
rva.logger.Info("Finch virtual machine removed successfully")
return nil
}

func (rva *removeVMAction) createVMRemoveCommand(force bool) command.Command {
if force {
return rva.creator.CreateWithoutStdio("remove", "--force", limaInstanceName)
}
return rva.creator.CreateWithoutStdio("remove", limaInstanceName)
}
44 changes: 36 additions & 8 deletions cmd/finch/virtual_machine_remove_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/runfinch/finch/pkg/mocks"

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

Expand All @@ -28,14 +27,10 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {
testCases := []struct {
name string
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller)
cmd *cobra.Command
args []string
}{
{
name: "should remove the instance",
cmd: &cobra.Command{
Use: "remove",
},
args: []string{},
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
getVMStatusC := mocks.NewCommand(ctrl)
Expand All @@ -49,6 +44,18 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {
logger.EXPECT().Info(gomock.Any()).AnyTimes()
},
},
{
name: "should forcibly remove the instance",
args: []string{
"--force",
},
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
command := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("remove", "--force", limaInstanceName).Return(command)
command.EXPECT().CombinedOutput()
logger.EXPECT().Info(gomock.Any()).AnyTimes()
},
},
}

for _, tc := range testCases {
Expand All @@ -59,9 +66,11 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {
ctrl := gomock.NewController(t)
logger := mocks.NewLogger(ctrl)
lcc := mocks.NewLimaCmdCreator(ctrl)

tc.mockSvc(logger, lcc, ctrl)
assert.NoError(t, newRemoveVMAction(lcc, logger).runAdapter(tc.cmd, tc.args))

cmd := newRemoveVMCommand(lcc, logger)
cmd.SetArgs(tc.args)
assert.NoError(t, cmd.Execute())
})
}
}
Expand All @@ -73,6 +82,7 @@ func TestRemoveVMAction_run(t *testing.T) {
name string
wantErr error
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller)
force bool
}{
{
name: "should remove the instance",
Expand All @@ -89,6 +99,7 @@ func TestRemoveVMAction_run(t *testing.T) {
logger.EXPECT().Info("Removing existing Finch virtual machine...")
logger.EXPECT().Info("Finch virtual machine removed successfully")
},
force: false,
},
{
name: "running VM",
Expand All @@ -100,6 +111,7 @@ func TestRemoveVMAction_run(t *testing.T) {
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
},
force: false,
},
{
name: "nonexistent VM",
Expand All @@ -110,6 +122,7 @@ func TestRemoveVMAction_run(t *testing.T) {
getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
},
force: false,
},
{
name: "unknown VM status",
Expand All @@ -120,6 +133,7 @@ func TestRemoveVMAction_run(t *testing.T) {
getVMStatusC.EXPECT().Output().Return([]byte("Broken"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Broken")
},
force: false,
},
{
name: "status command returns an error",
Expand All @@ -129,6 +143,7 @@ func TestRemoveVMAction_run(t *testing.T) {
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Broken"), errors.New("get status error"))
},
force: false,
},
{
name: "should print error if virtual machine failed to remove",
Expand All @@ -146,6 +161,19 @@ func TestRemoveVMAction_run(t *testing.T) {
logger.EXPECT().Info("Removing existing Finch virtual machine...")
logger.EXPECT().Errorf("Finch virtual machine failed to remove, debug logs: %s", logs)
},
force: false,
},
{
name: "should forcibly remove the instance",
wantErr: nil,
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
command := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("remove", "--force", limaInstanceName).Return(command)
command.EXPECT().CombinedOutput()
logger.EXPECT().Info("Forcibly removing Finch virtual machine...")
logger.EXPECT().Info("Finch virtual machine removed successfully")
},
force: true,
},
}

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

tc.mockSvc(logger, lcc, ctrl)
err := newRemoveVMAction(lcc, logger).run()
err := newRemoveVMAction(lcc, logger).run(tc.force)
assert.Equal(t, tc.wantErr, err)
})
}
Expand Down
47 changes: 36 additions & 11 deletions cmd/finch/virtual_machine_stop.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ func newStopVMCommand(limaCmdCreator command.LimaCmdCreator, logger flog.Logger)
RunE: newStopVMAction(limaCmdCreator, logger).runAdapter,
}

stopVMCommand.Flags().BoolP("force", "f", false, "forcibly stop finch VM")

return stopVMCommand
}

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

func (sva *stopVMAction) runAdapter(cmd *cobra.Command, args []string) error {
return sva.run()
}

func (sva *stopVMAction) run() error {
err := sva.assertVMIsRunning(sva.creator, sva.logger)
force, err := cmd.Flags().GetBool("force")
if err != nil {
return err
}
return sva.run(force)
}

limaCmd := sva.creator.CreateWithoutStdio("stop", limaInstanceName)
sva.logger.Info("Stopping existing Finch virtual machine...")
logs, err := limaCmd.CombinedOutput()
func (sva *stopVMAction) run(force bool) error {
if force {
return sva.stopVM(force)
}

err := sva.assertVMIsRunning(sva.creator, sva.logger)
if err != nil {
sva.logger.Errorf("Finch virtual machine failed to stop, debug logs: %s", logs)
return err
}
sva.logger.Info("Finch virtual machine stopped successfully")
return nil

return sva.stopVM(false)
}

func (sva *stopVMAction) assertVMIsRunning(creator command.LimaCmdCreator, logger flog.Logger) error {
Expand All @@ -67,3 +69,26 @@ func (sva *stopVMAction) assertVMIsRunning(creator command.LimaCmdCreator, logge
return nil
}
}

func (sva *stopVMAction) stopVM(force bool) error {
limaCmd := sva.createLimaStopCommand(force)
if force {
sva.logger.Info("Forcibly stopping Finch virtual machine...")
} else {
sva.logger.Info("Stopping existing Finch virtual machine...")
}
logs, err := limaCmd.CombinedOutput()
if err != nil {
sva.logger.Errorf("Finch virtual machine failed to stop, debug logs: %s", logs)
return err
}
sva.logger.Info("Finch virtual machine stopped successfully")
return nil
}

func (sva *stopVMAction) createLimaStopCommand(force bool) command.Command {
if force {
return sva.creator.CreateWithoutStdio("stop", "--force", limaInstanceName)
}
return sva.creator.CreateWithoutStdio("stop", limaInstanceName)
}
Loading

0 comments on commit d499a7d

Please sign in to comment.