Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adds option to remove persistent user data #214

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: adds option to remove persistent user data #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40802e3
feat: adds option to remove persistent user data
sam-berning Feb 8, 2023
27ff143
Merge branch 'main' into delete-disk
ahsan-z-khan Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/finch/virtual_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func newVirtualMachineCommand(
virtualMachineCommand.AddCommand(
newStartVMCommand(limaCmdCreator, logger, optionalDepGroups, lca, nca, fs, fp.LimaSSHPrivateKeyPath(), diskManager),
newStopVMCommand(limaCmdCreator, logger),
newRemoveVMCommand(limaCmdCreator, logger),
newRemoveVMCommand(limaCmdCreator, logger, diskManager),
newStatusVMCommand(limaCmdCreator, logger, os.Stdout),
newInitVMCommand(limaCmdCreator, logger, optionalDepGroups, lca, nca, fp.BaseYamlFilePath(), fs,
fp.LimaSSHPrivateKeyPath(), diskManager),
Expand Down
56 changes: 46 additions & 10 deletions cmd/finch/virtual_machine_remove.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package main
import (
"fmt"

"github.com/runfinch/finch/pkg/disk"

"github.com/spf13/cobra"

"github.com/runfinch/finch/pkg/command"
Expand All @@ -14,46 +16,80 @@ import (
"github.com/runfinch/finch/pkg/flog"
)

func newRemoveVMCommand(limaCmdCreator command.LimaCmdCreator, logger flog.Logger) *cobra.Command {
func newRemoveVMCommand(limaCmdCreator command.LimaCmdCreator, logger flog.Logger, dm disk.UserDataDiskManager) *cobra.Command {
removeVMCommand := &cobra.Command{
Use: "remove",
Short: "Remove the virtual machine instance",
RunE: newRemoveVMAction(limaCmdCreator, logger).runAdapter,
RunE: newRemoveVMAction(limaCmdCreator, logger, dm).runAdapter,
}

removeVMCommand.Flags().BoolP("force", "f", false, "forcibly remove finch VM")
removeVMCommand.Flags().Bool("user-data", false, "remove persistent containerd and nerdctl data")

return removeVMCommand
}

type removeVMAction struct {
creator command.LimaCmdCreator
logger flog.Logger
creator command.LimaCmdCreator
logger flog.Logger
diskManager disk.UserDataDiskManager
}

func newRemoveVMAction(creator command.LimaCmdCreator, logger flog.Logger) *removeVMAction {
return &removeVMAction{creator: creator, logger: logger}
func newRemoveVMAction(creator command.LimaCmdCreator, logger flog.Logger, dm disk.UserDataDiskManager) *removeVMAction {
return &removeVMAction{creator: creator, logger: logger, diskManager: dm}
}

func (rva *removeVMAction) runAdapter(cmd *cobra.Command, args []string) error {
force, err := cmd.Flags().GetBool("force")
if err != nil {
return err
}
return rva.run(force)
deleteDisk, err := cmd.Flags().GetBool("user-data")
if err != nil {
return err
}
return rva.run(force, deleteDisk)
}

func (rva *removeVMAction) run(force bool) error {
func (rva *removeVMAction) run(force bool, deleteDisk bool) error {
if force {
return rva.removeVM(force)
err := rva.removeVM(force)
if err != nil {
return err
}

if deleteDisk {
rva.logger.Info("Forcibly deleting user data...")
err := rva.diskManager.DeleteUserDataDisk(force)
if err != nil {
return err
}
rva.logger.Info("Successfully deleted user data.")
}

return nil
}

err := rva.assertVMIsStopped(rva.creator, rva.logger)
if err != nil {
return err
}

return rva.removeVM(false)
err = rva.removeVM(false)
if err != nil {
return err
}

if deleteDisk {
rva.logger.Info("Deleting user data...")
err := rva.diskManager.DeleteUserDataDisk(false)
if err != nil {
return err
}
rva.logger.Info("Successfully deleted user data.")
}

return nil
}

func (rva *removeVMAction) assertVMIsStopped(creator command.LimaCmdCreator, logger flog.Logger) error {
Expand Down
137 changes: 111 additions & 26 deletions cmd/finch/virtual_machine_remove_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
func TestNewRemoveVMCommand(t *testing.T) {
t.Parallel()

cmd := newRemoveVMCommand(nil, nil)
cmd := newRemoveVMCommand(nil, nil, nil)
assert.Equal(t, cmd.Name(), "remove")
}

Expand All @@ -26,13 +26,13 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {

testCases := []struct {
name string
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller)
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller, *mocks.MockUserDataDiskManager)
args []string
}{
{
name: "should remove the instance",
args: []string{},
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
Expand All @@ -49,13 +49,51 @@ func TestRemoveVMAction_runAdapter(t *testing.T) {
args: []string{
"--force",
},
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
command := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("remove", "--force", limaInstanceName).Return(command)
command.EXPECT().CombinedOutput()
logger.EXPECT().Info(gomock.Any()).AnyTimes()
},
},
{
name: "should remove the instance and user data",
args: []string{
"--user-data",
},
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")

command := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("remove", limaInstanceName).Return(command)
command.EXPECT().CombinedOutput()
logger.EXPECT().Info("Removing existing Finch virtual machine...")
logger.EXPECT().Info("Finch virtual machine removed successfully")
logger.EXPECT().Info("Deleting user data...")
dm.EXPECT().DeleteUserDataDisk(false).Return(nil)
logger.EXPECT().Info("Successfully deleted user data.")
},
},
{
name: "should forcibly remove the instance and user data",
args: []string{
"--force",
"--user-data",
},
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
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")
logger.EXPECT().Info("Forcibly deleting user data...")
dm.EXPECT().DeleteUserDataDisk(true).Return(nil)
logger.EXPECT().Info("Successfully deleted user data.")
},
},
}

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

cmd := newRemoveVMCommand(lcc, logger)
cmd := newRemoveVMCommand(lcc, logger, dm)
cmd.SetArgs(tc.args)
assert.NoError(t, cmd.Execute())
})
Expand All @@ -79,15 +118,16 @@ func TestRemoveVMAction_run(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
wantErr error
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller)
force bool
name string
wantErr error
mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller, *mocks.MockUserDataDiskManager)
force bool
deleteDisk bool
}{
{
name: "should remove the instance",
wantErr: nil,
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
Expand All @@ -99,56 +139,61 @@ func TestRemoveVMAction_run(t *testing.T) {
logger.EXPECT().Info("Removing existing Finch virtual machine...")
logger.EXPECT().Info("Finch virtual machine removed successfully")
},
force: false,
force: false,
deleteDisk: false,
},
{
name: "running VM",
wantErr: fmt.Errorf("the instance %q is running, run `finch %s stop` to stop the instance first",
limaInstanceName, virtualMachineRootCmd),
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
},
force: false,
force: false,
deleteDisk: false,
},
{
name: "nonexistent VM",
wantErr: fmt.Errorf("the instance %q does not exist", limaInstanceName),
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte(""), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "")
},
force: false,
force: false,
deleteDisk: false,
},
{
name: "unknown VM status",
wantErr: errors.New("unrecognized system status"),
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Broken"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Broken")
},
force: false,
force: false,
deleteDisk: false,
},
{
name: "status command returns an error",
wantErr: errors.New("get status error"),
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Broken"), errors.New("get status error"))
},
force: false,
force: false,
deleteDisk: false,
},
{
name: "should print error if virtual machine failed to remove",
wantErr: errors.New("failed to remove instance"),
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
Expand All @@ -161,19 +206,58 @@ 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,
force: false,
deleteDisk: false,
},
{
name: "should forcibly remove the instance",
wantErr: nil,
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
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,
deleteDisk: false,
},
{
name: "should remove the instance and user data",
wantErr: nil,
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
getVMStatusC := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Stopped"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Stopped")

command := mocks.NewCommand(ctrl)
creator.EXPECT().CreateWithoutStdio("remove", limaInstanceName).Return(command)
command.EXPECT().CombinedOutput()
logger.EXPECT().Info("Removing existing Finch virtual machine...")
logger.EXPECT().Info("Finch virtual machine removed successfully")
logger.EXPECT().Info("Deleting user data...")
dm.EXPECT().DeleteUserDataDisk(false).Return(nil)
logger.EXPECT().Info("Successfully deleted user data.")
},
force: false,
deleteDisk: true,
},
{
name: "should forcibly remove the instance and user data",
wantErr: nil,
mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller, dm *mocks.MockUserDataDiskManager) {
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")
logger.EXPECT().Info("Forcibly deleting user data...")
dm.EXPECT().DeleteUserDataDisk(true).Return(nil)
logger.EXPECT().Info("Successfully deleted user data.")
},
force: true,
force: true,
deleteDisk: true,
},
}

Expand All @@ -185,9 +269,10 @@ func TestRemoveVMAction_run(t *testing.T) {
ctrl := gomock.NewController(t)
logger := mocks.NewLogger(ctrl)
lcc := mocks.NewLimaCmdCreator(ctrl)
dm := mocks.NewMockUserDataDiskManager(ctrl)

tc.mockSvc(logger, lcc, ctrl)
err := newRemoveVMAction(lcc, logger).run(tc.force)
tc.mockSvc(logger, lcc, ctrl, dm)
err := newRemoveVMAction(lcc, logger, dm).run(tc.force, tc.deleteDisk)
assert.Equal(t, tc.wantErr, err)
})
}
Expand Down
13 changes: 13 additions & 0 deletions e2e/vm/additional_disk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,18 @@ var testAdditionalDisk = func(o *option.Option) {
newPsOutput := command.StdoutStr(o, "ps", "--all", "--format", "{{.Names}}")
gomega.Expect(newPsOutput).Should(gomega.Equal(oldPsOutput))
})
ginkgo.It("Removes the user data on `finch vm remove --user-data`", func() {
command.Run(o, "pull", savedImage)
oldImagesOutput := command.StdoutStr(o, "images", "--format", "{{.Name}}")
gomega.Expect(oldImagesOutput).Should(gomega.ContainSubstring(savedImage))

command.New(o, virtualMachineRootCmd, "stop").WithoutCheckingExitCode().WithTimeoutInSeconds(90).Run()
command.Run(o, virtualMachineRootCmd, "remove", "--user-data")

command.New(o, virtualMachineRootCmd, "init").WithTimeoutInSeconds(240).Run()

newImagesOutput := command.StdoutStr(o, "images", "--format", "{{.Name}}")
gomega.Expect(newImagesOutput).ShouldNot(gomega.Equal(oldImagesOutput))
})
})
}
Loading