fix: always force stop when using Virtualization.framework (runfinch#350

) Issue #, if available: *Description of changes:* - always force stop when using Virtualization.framework, potential workaround for lima-vm/lima#1381 (needs more testing) *Testing done:* - local testing - [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: Justin Alvarez <[email protected]>
pendo324 · Apr 12, 2023 · c521f1f · c521f1f
1 parent 84c2634
commit c521f1f
Show file tree

Hide file tree

Showing 4 changed files with 161 additions and 1 deletion.
diff --git a/cmd/finch/virtual_machine_stop.go b/cmd/finch/virtual_machine_stop.go
@@ -39,6 +39,17 @@ func (sva *stopVMAction) runAdapter(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
+
+	if !force {
+		if vmType, err := lima.GetVMType(sva.creator, sva.logger, limaInstanceName); err == nil {
+			if vmType == lima.VZ {
+				force = true
+			}
+		} else {
+			return err
+		}
+	}
+
 	return sva.run(force)
 }
 

diff --git a/cmd/finch/virtual_machine_stop_test.go b/cmd/finch/virtual_machine_stop_test.go
@@ -28,11 +28,17 @@ func TestStopVMAction_runAdapter(t *testing.T) {
 		name    string
 		mockSvc func(*mocks.Logger, *mocks.LimaCmdCreator, *gomock.Controller)
 		args    []string
+		wantErr error
 	}{
 		{
 			name: "should stop the instance",
 			args: []string{},
 			mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
+				getVMTypeC := mocks.NewCommand(ctrl)
+				creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.VMType}}", limaInstanceName).Return(getVMTypeC)
+				getVMTypeC.EXPECT().Output().Return([]byte("qemu"), nil)
+				logger.EXPECT().Debugf("VMType of virtual machine: %s", "qemu")
+
 				getVMStatusC := mocks.NewCommand(ctrl)
 				creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
 				getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
@@ -43,6 +49,7 @@ func TestStopVMAction_runAdapter(t *testing.T) {
 				command.EXPECT().CombinedOutput()
 				logger.EXPECT().Info(gomock.Any()).AnyTimes()
 			},
+			wantErr: nil,
 		},
 		{
 			name: "should force stop the instance",
@@ -55,6 +62,33 @@ func TestStopVMAction_runAdapter(t *testing.T) {
 				command.EXPECT().CombinedOutput()
 				logger.EXPECT().Info(gomock.Any()).AnyTimes()
 			},
+			wantErr: nil,
+		},
+		{
+			name: "should stop the instance and use --force even when not specified if VMType == vz",
+			args: []string{},
+			mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
+				getVMTypeC := mocks.NewCommand(ctrl)
+				creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.VMType}}", limaInstanceName).Return(getVMTypeC)
+				getVMTypeC.EXPECT().Output().Return([]byte("vz"), nil)
+				logger.EXPECT().Debugf("VMType of virtual machine: %s", "vz")
+
+				command := mocks.NewCommand(ctrl)
+				creator.EXPECT().CreateWithoutStdio("stop", "--force", limaInstanceName).Return(command)
+				command.EXPECT().CombinedOutput()
+				logger.EXPECT().Info(gomock.Any()).AnyTimes()
+			},
+			wantErr: nil,
+		},
+		{
+			name: "get VMType returns an error",
+			args: []string{},
+			mockSvc: func(logger *mocks.Logger, creator *mocks.LimaCmdCreator, ctrl *gomock.Controller) {
+				getVMTypeC := mocks.NewCommand(ctrl)
+				creator.EXPECT().CreateWithoutStdio("ls", "-f", "{{.VMType}}", limaInstanceName).Return(getVMTypeC)
+				getVMTypeC.EXPECT().Output().Return([]byte("unknown"), errors.New("unrecognized VMType"))
+			},
+			wantErr: errors.New("unrecognized VMType"),
 		},
 	}
 
@@ -70,7 +104,8 @@ func TestStopVMAction_runAdapter(t *testing.T) {
 
 			cmd := newStopVMCommand(lcc, logger)
 			cmd.SetArgs(tc.args)
-			assert.NoError(t, cmd.Execute())
+			err := cmd.Execute()
+			assert.Equal(t, tc.wantErr, err)
 		})
 	}
 }

diff --git a/pkg/lima/lima.go b/pkg/lima/lima.go
@@ -17,12 +17,21 @@ import (
 // https://github.com/lima-vm/lima/blob/fc783ec455a91d70639f9a1d7f22e9890fe6b1cd/pkg/store/instance.go#L23.
 type VMStatus int64
 
+// VMType for Lima.
+// Relevant type defined in Lima upstream:
+// https://github.com/lima-vm/lima/blob/f0282b2dfc4b6795295fceb2c86acd1312cef436/pkg/limayaml/limayaml.go#L53-L54.
+type VMType string
+
 // Finch CLI assumes there are only 4 VM status below. Adding more statuses will need to make changes in the caller side.
 const (
 	Running VMStatus = iota
 	Stopped
 	Nonexistent
 	Unknown
+	QEMU              VMType = "qemu"
+	VZ                VMType = "vz"
+	NonexistentVMType VMType = "nonexistant"
+	UnknownVMType     VMType = "unknown"
 )
 
 // GetVMStatus returns the Lima VM status.
@@ -37,6 +46,18 @@ func GetVMStatus(creator command.LimaCmdCreator, logger flog.Logger, instanceNam
 	return toVMStatus(status, logger)
 }
 
+// GetVMType returns the Lima VMType for a running instance.
+func GetVMType(creator command.LimaCmdCreator, logger flog.Logger, instanceName string) (VMType, error) {
+	args := []string{"ls", "-f", "{{.VMType}}", instanceName}
+	cmd := creator.CreateWithoutStdio(args...)
+	out, err := cmd.Output()
+	if err != nil {
+		return UnknownVMType, err
+	}
+	status := strings.TrimSpace(string(out))
+	return toVMType(status, logger)
+}
+
 func toVMStatus(status string, logger flog.Logger) (VMStatus, error) {
 	logger.Debugf("Status of virtual machine: %s", status)
 	switch status {
@@ -50,3 +71,17 @@ func toVMStatus(status string, logger flog.Logger) (VMStatus, error) {
 		return Unknown, errors.New("unrecognized system status")
 	}
 }
+
+func toVMType(vmType string, logger flog.Logger) (VMType, error) {
+	logger.Debugf("VMType of virtual machine: %s", vmType)
+	switch vmType {
+	case "":
+		return NonexistentVMType, nil
+	case "qemu":
+		return QEMU, nil
+	case "vz":
+		return VZ, nil
+	default:
+		return UnknownVMType, errors.New("unrecognized VMType")
+	}
+}
diff --git a/pkg/lima/lima_test.go b/pkg/lima/lima_test.go
@@ -92,3 +92,82 @@ func TestGetVMStatus(t *testing.T) {
 		})
 	}
 }
+
+func TestGetVMType(t *testing.T) {
+	t.Parallel()
+
+	instanceName := "finch"
+	mockArgs := []string{"ls", "-f", "{{.VMType}}", instanceName}
+	testCases := []struct {
+		name    string
+		want    lima.VMType
+		wantErr error
+		mockSvc func(*mocks.LimaCmdCreator, *mocks.Logger, *mocks.Command)
+	}{
+		{
+			name:    "qemu VM",
+			want:    lima.QEMU,
+			wantErr: nil,
+			mockSvc: func(creator *mocks.LimaCmdCreator, logger *mocks.Logger, cmd *mocks.Command) {
+				creator.EXPECT().CreateWithoutStdio(mockArgs).Return(cmd)
+				cmd.EXPECT().Output().Return([]byte("qemu"), nil)
+				logger.EXPECT().Debugf("VMType of virtual machine: %s", "qemu")
+			},
+		},
+		{
+			name:    "vz VM",
+			want:    lima.VZ,
+			wantErr: nil,
+			mockSvc: func(creator *mocks.LimaCmdCreator, logger *mocks.Logger, cmd *mocks.Command) {
+				creator.EXPECT().CreateWithoutStdio(mockArgs).Return(cmd)
+				cmd.EXPECT().Output().Return([]byte("vz"), nil)
+				logger.EXPECT().Debugf("VMType of virtual machine: %s", "vz")
+			},
+		},
+		{
+			name:    "nonexistent VM",
+			want:    lima.NonexistentVMType,
+			wantErr: nil,
+			mockSvc: func(creator *mocks.LimaCmdCreator, logger *mocks.Logger, cmd *mocks.Command) {
+				creator.EXPECT().CreateWithoutStdio(mockArgs).Return(cmd)
+				cmd.EXPECT().Output().Return([]byte(" "), nil)
+				logger.EXPECT().Debugf("VMType of virtual machine: %s", "")
+			},
+		},
+		{
+			name:    "unknown VM type",
+			want:    lima.UnknownVMType,
+			wantErr: errors.New("unrecognized VMType"),
+			mockSvc: func(creator *mocks.LimaCmdCreator, logger *mocks.Logger, cmd *mocks.Command) {
+				creator.EXPECT().CreateWithoutStdio(mockArgs).Return(cmd)
+				cmd.EXPECT().Output().Return([]byte("Broken "), nil)
+				logger.EXPECT().Debugf("VMType of virtual machine: %s", "Broken")
+			},
+		},
+		{
+			name:    "type command returns an error",
+			want:    lima.UnknownVMType,
+			wantErr: errors.New("get VMType error"),
+			mockSvc: func(creator *mocks.LimaCmdCreator, logger *mocks.Logger, cmd *mocks.Command) {
+				creator.EXPECT().CreateWithoutStdio(mockArgs).Return(cmd)
+				cmd.EXPECT().Output().Return([]byte("Broken "), errors.New("get VMType error"))
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctrl := gomock.NewController(t)
+			creator := mocks.NewLimaCmdCreator(ctrl)
+			statusCmd := mocks.NewCommand(ctrl)
+			logger := mocks.NewLogger(ctrl)
+			tc.mockSvc(creator, logger, statusCmd)
+			got, err := lima.GetVMType(creator, logger, instanceName)
+			assert.Equal(t, tc.wantErr, err)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}