Merge branch 'rm-test' into 'main'

bug: Fix rm on linux

Closes #29

See merge request bootc-org/podman-bootc-cli!64

Author: germag

cmd/rm.go

@@ -67,16 +67,6 @@ func prune(id string) error {
 
 	defer bootcVM.CloseConnection()
 
-	vmExists, err := bootcVM.Exists()
-	if err != nil {
-		return fmt.Errorf("unable to check if VM exists: %v", err)
-	}
-
-	if !vmExists {
-		logrus.Debugf("VM %s is not running", id)
-		return nil
-	}
-
 	if force {
 		err := forceKillVM(bootcVM)
 		if err != nil {
@@ -136,7 +126,7 @@ func killVM(bootcVM vm.BootcVM) (err error) {
 }
 
 func forceKillVM(bootcVM vm.BootcVM) (err error) {
-	err = bootcVM.ForceDelete()
+	err = bootcVM.Delete()
 	if err != nil {
 		return
 	}

cmd/run.go

@@ -184,7 +184,7 @@ func doRun(flags *cobra.Command, args []string) error {
 
 		// Always remove when executing a command
 		if vmConfig.RemoveVm || len(cmd) > 0 {
-			err = bootcVM.ForceDelete() //delete the VM, but keep the disk image
+			err = bootcVM.Delete() //delete the VM, but keep the disk image
 			if err != nil {
 				return fmt.Errorf("unable to remove VM from cache: %w", err)
 			}

cmd/stop.go

@@ -36,5 +36,5 @@ func doStop(_ *cobra.Command, args []string) (err error) {
 		return err
 	}
 	defer bootcVM.CloseConnection()
-	return bootcVM.ForceDelete()
+	return bootcVM.Delete()
 }

pkg/vm/vm.go

@@ -20,8 +20,8 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
-// getVMCachePath returns the path to the VM cache directory
-func getVMCachePath(imageId string, user user.User) (path string, err error) {
+// GetVMCachePath returns the path to the VM cache directory
+func GetVMCachePath(imageId string, user user.User) (path string, err error) {
 	files, err := os.ReadDir(user.CacheDir())
 	if err != nil {
 		return "", err
@@ -61,8 +61,6 @@ type RunVMParameters struct {
 
 type BootcVM interface {
 	Run(RunVMParameters) error
-	ForceDelete() error
-	Shutdown() error
 	Delete() error
 	IsRunning() (bool, error)
 	WriteConfig(bootc.BootcDisk) error

pkg/vm/vm_darwin.go

@@ -25,7 +25,7 @@ func NewVM(params NewVMParameters) (vm *BootcVMMac, err error) {
 		return nil, fmt.Errorf("image ID is required")
 	}
 
-	cacheDir, err := getVMCachePath(params.ImageID, params.User)
+	cacheDir, err := GetVMCachePath(params.ImageID, params.User)
 	if err != nil {
 		return nil, fmt.Errorf("unable to get VM cache path: %w", err)
 	}
@@ -163,48 +163,26 @@ func (b *BootcVMMac) Run(params RunVMParameters) (err error) {
 func (b *BootcVMMac) Delete() error {
 	logrus.Debugf("Deleting Mac VM %s", b.cacheDir)
 
-	pid, err := utils.ReadPidFile(b.pidFile)
-	if err != nil {
-		return fmt.Errorf("reading pid file: %w", err)
-	}
-
-	process, err := os.FindProcess(pid)
-	if err != nil {
-		return fmt.Errorf("process not found while attempting to delete VM: %w", err)
-	}
-
-	return process.Signal(os.Interrupt)
-}
-
-func (b *BootcVMMac) Shutdown() error {
-	b.SetUser("root") //TODO the stop command should accept a user parameter
-
 	isRunning, err := b.IsRunning()
 	if err != nil {
-		return fmt.Errorf("unable to determine if VM is running: %w", err)
+		return fmt.Errorf("checking if VM is running: %w", err)
 	}
 
-	if isRunning {
-		poweroff := []string{"poweroff"}
-		return b.RunSSH(poweroff)
-	} else {
-		logrus.Infof("Unable to shutdown VM. It is not not running.")
+	if !isRunning {
 		return nil
 	}
-}
 
-func (b *BootcVMMac) ForceDelete() error {
-	err := b.Shutdown()
+	pid, err := utils.ReadPidFile(b.pidFile)
 	if err != nil {
-		return fmt.Errorf("unable to shutdown VM: %w", err)
+		return fmt.Errorf("reading pid file: %w", err)
 	}
 
-	err = b.Delete()
+	process, err := os.FindProcess(pid)
 	if err != nil {
-		return fmt.Errorf("unable to delete VM: %w", err)
+		return fmt.Errorf("process not found while attempting to delete VM: %w", err)
 	}
 
-	return nil
+	return process.Signal(os.Interrupt)
 }
 
 func (b *BootcVMMac) IsRunning() (bool, error) {

pkg/vm/vm_linux.go

@@ -39,7 +39,7 @@ func NewVM(params NewVMParameters) (vm *BootcVMLinux, err error) {
 		return nil, fmt.Errorf("libvirt URI is required")
 	}
 
-	cacheDir, err := getVMCachePath(params.ImageID, params.User)
+	cacheDir, err := GetVMCachePath(params.ImageID, params.User)
 	if err != nil {
 		return nil, fmt.Errorf("unable to get VM cache path: %w", err)
 	}
@@ -275,6 +275,11 @@ func (v *BootcVMLinux) loadExistingDomain() (err error) {
 
 // Delete the VM definition
 func (v *BootcVMLinux) Delete() (err error) {
+	err = v.Shutdown()
+	if err != nil {
+		return fmt.Errorf("unable to shutdown VM: %w", err)
+	}
+
 	domainExists, err := v.Exists()
 	if err != nil {
 		return fmt.Errorf("unable to check if VM exists: %w", err)
@@ -312,21 +317,6 @@ func (v *BootcVMLinux) Shutdown() (err error) {
 	return
 }
 
-// ForceDelete stops and removes the VM
-func (v *BootcVMLinux) ForceDelete() (err error) {
-	err = v.Shutdown()
-	if err != nil {
-		return fmt.Errorf("unable to shutdown VM: %w", err)
-	}
-
-	err = v.Delete()
-	if err != nil {
-		return fmt.Errorf("unable to remove VM: %w", err)
-	}
-
-	return
-}
-
 func (v *BootcVMLinux) Exists() (bool, error) {
 	var flags libvirt.ConnectListAllDomainsFlags
 	domains, err := v.libvirtConnection.ListAllDomains(flags)

pkg/vm/vm_test.go

@@ -181,7 +181,7 @@ var _ = Describe("VM", func() {
 	})
 
 	Context("is running", func() {
-		It("should remove the VM from the hypervisor after calling ForceDelete", func() {
+		It("should remove the VM from the hypervisor after calling Delete", func() {
 			//create vm and start it
 			bootcVM := createTestVM(testImageID)
 			runTestVM(bootcVM)
@@ -192,7 +192,7 @@ var _ = Describe("VM", func() {
 			Expect(exists).To(BeTrue())
 
 			//attempt to stop and delete the VM
-			err = bootcVM.ForceDelete()
+			err = bootcVM.Delete()
 			Expect(err).To(Not(HaveOccurred()))
 
 			//assert that the VM is stopped and deleted