Merge pull request #20561 from ashley-cui/rm

AppleHV: Fix machine rm error message
containers · Nov 1, 2023 · e622045 · e622045
2 parents 1ef61cf + f6ec210
commit e622045
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 13 deletions.
diff --git a/pkg/machine/applehv/config.go b/pkg/machine/applehv/config.go
@@ -53,12 +53,11 @@ func (v AppleHVVirtualization) CheckExclusiveActiveVM() (bool, string, error) {
 }
 
 func (v AppleHVVirtualization) IsValidVMName(name string) (bool, error) {
-	mm := MacMachine{Name: name}
 	configDir, err := machine.GetConfDir(machine.AppleHvVirt)
 	if err != nil {
 		return false, err
 	}
-	if err := loadMacMachineFromJSON(configDir, &mm); err != nil {
+	if _, err := loadMacMachineFromJSON(configDir); err != nil {
 		return false, err
 	}
 	return true, nil
@@ -183,14 +182,14 @@ func (v AppleHVVirtualization) loadFromLocalJson() ([]*MacMachine, error) {
 	}
 
 	for _, jsonFile := range jsonFiles {
-		mm := MacMachine{}
-		if err := loadMacMachineFromJSON(jsonFile, &mm); err != nil {
+		mm, err := loadMacMachineFromJSON(jsonFile)
+		if err != nil {
 			return nil, err
 		}
 		if err != nil {
 			return nil, err
 		}
-		mms = append(mms, &mm)
+		mms = append(mms, mm)
 	}
 	return mms, nil
 }
diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go
@@ -761,9 +761,9 @@ func (m *MacMachine) loadFromFile() (*MacMachine, error) {
 	if err != nil {
 		return nil, err
 	}
-	mm := MacMachine{}
 
-	if err := loadMacMachineFromJSON(jsonPath, &mm); err != nil {
+	mm, err := loadMacMachineFromJSON(jsonPath)
+	if err != nil {
 		return nil, err
 	}
 
@@ -773,18 +773,23 @@ func (m *MacMachine) loadFromFile() (*MacMachine, error) {
 	}
 	mm.lock = lock
 
-	return &mm, nil
+	return mm, nil
 }
 
-func loadMacMachineFromJSON(fqConfigPath string, macMachine *MacMachine) error {
+func loadMacMachineFromJSON(fqConfigPath string) (*MacMachine, error) {
 	b, err := os.ReadFile(fqConfigPath)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
-			return fmt.Errorf("%q: %w", fqConfigPath, machine.ErrNoSuchVM)
+			name := strings.TrimSuffix(filepath.Base(fqConfigPath), ".json")
+			return nil, fmt.Errorf("%s: %w", name, machine.ErrNoSuchVM)
 		}
-		return err
+		return nil, err
+	}
+	mm := new(MacMachine)
+	if err := json.Unmarshal(b, mm); err != nil {
+		return nil, err
 	}
-	return json.Unmarshal(b, macMachine)
+	return mm, nil
 }
 
 func (m *MacMachine) jsonConfigPath() (string, error) {

diff --git a/pkg/machine/e2e/rm_test.go b/pkg/machine/e2e/rm_test.go
@@ -32,8 +32,9 @@ var _ = Describe("podman machine rm", func() {
 	})
 
 	It("Remove machine", func() {
+		name := randomString()
 		i := new(initMachine)
-		session, err := mb.setCmd(i.withImagePath(mb.imagePath)).run()
+		session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
 		Expect(err).ToNot(HaveOccurred())
 		Expect(session).To(Exit(0))
 		rm := rmMachine{}
@@ -46,6 +47,12 @@ var _ = Describe("podman machine rm", func() {
 		_, ec, err := mb.toQemuInspectInfo()
 		Expect(err).ToNot(HaveOccurred())
 		Expect(ec).To(Equal(125))
+
+		// Removing non-existent machine should fail
+		removeSession2, err := mb.setCmd(rm.withForce()).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(removeSession2).To(Exit(125))
+		Expect(removeSession2.errorToString()).To(ContainSubstring(fmt.Sprintf("%s: VM does not exist", name)))
 	})
 
 	It("Remove running machine", func() {