pkg/machine: rework RemoveConnection()

It really does not make sense to call RemoveConnection() twice and then update the config file a third time in updateDefaultMachineinConfig(). This results in unnecessary reads/writes and more code. Simplyfy this into one function that is only called once and do all updates at once. [NO NEW TESTS NEEDED] Signed-off-by: Paul Holzinger <[email protected]>
containers · Apr 26, 2023 · 64959b7 · 64959b7
1 parent 2296e71
commit 64959b7
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 37 deletions.
diff --git a/cmd/podman/machine/rm.go b/cmd/podman/machine/rm.go
@@ -9,7 +9,6 @@ import (
 	"os"
 	"strings"
 
-	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v4/cmd/podman/registry"
 	"github.com/containers/podman/v4/libpod/events"
 	"github.com/containers/podman/v4/pkg/machine"
@@ -91,24 +90,5 @@ func rm(_ *cobra.Command, args []string) error {
 		return err
 	}
 	newMachineEvent(events.Remove, events.Event{Name: vmName})
-	err = updateDefaultMachineInConfig(vmName)
-	if err != nil {
-		return fmt.Errorf("failed to update default machine: %v", err)
-	}
 	return nil
 }
-
-func updateDefaultMachineInConfig(vmName string) error {
-	cfg, err := config.ReadCustomConfig()
-	if err != nil {
-		return err
-	}
-	if cfg.Engine.ActiveService == vmName {
-		cfg.Engine.ActiveService = ""
-		for machine := range cfg.Engine.ServiceDestinations {
-			cfg.Engine.ActiveService = machine
-			break
-		}
-	}
-	return cfg.Write()
-}
diff --git a/pkg/machine/connection.go b/pkg/machine/connection.go
@@ -65,15 +65,25 @@ func ChangeDefault(name string) error {
 	return cfg.Write()
 }
 
-func RemoveConnection(name string) error {
+func RemoveConnections(names ...string) error {
 	cfg, err := config.ReadCustomConfig()
 	if err != nil {
 		return err
 	}
-	if _, ok := cfg.Engine.ServiceDestinations[name]; ok {
-		delete(cfg.Engine.ServiceDestinations, name)
-	} else {
-		return fmt.Errorf("unable to find connection named %q", name)
+	for _, name := range names {
+		if _, ok := cfg.Engine.ServiceDestinations[name]; ok {
+			delete(cfg.Engine.ServiceDestinations, name)
+		} else {
+			return fmt.Errorf("unable to find connection named %q", name)
+		}
+
+		if cfg.Engine.ActiveService == name {
+			cfg.Engine.ActiveService = ""
+			for service := range cfg.Engine.ServiceDestinations {
+				cfg.Engine.ActiveService = service
+				break
+			}
+		}
 	}
 	return cfg.Write()
 }
diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go
@@ -358,10 +358,7 @@ func (m *HyperVMachine) Remove(_ string, opts machine.RemoveOptions) (string, fu
 				logrus.Error(err)
 			}
 		}
-		if err := machine.RemoveConnection(m.Name); err != nil {
-			logrus.Error(err)
-		}
-		if err := machine.RemoveConnection(m.Name + "-root"); err != nil {
+		if err := machine.RemoveConnections(m.Name, m.Name+"-root"); err != nil {
 			logrus.Error(err)
 		}
 

diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go
@@ -994,10 +994,7 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func()
 				logrus.Error(err)
 			}
 		}
-		if err := machine.RemoveConnection(v.Name); err != nil {
-			logrus.Error(err)
-		}
-		if err := machine.RemoveConnection(v.Name + "-root"); err != nil {
+		if err := machine.RemoveConnections(v.Name, v.Name+"-root"); err != nil {
 			logrus.Error(err)
 		}
 		return nil

diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go
@@ -1368,10 +1368,7 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun
 
 	confirmationMessage += "\n"
 	return confirmationMessage, func() error {
-		if err := machine.RemoveConnection(v.Name); err != nil {
-			logrus.Error(err)
-		}
-		if err := machine.RemoveConnection(v.Name + "-root"); err != nil {
+		if err := machine.RemoveConnections(v.Name, v.Name+"-root"); err != nil {
 			logrus.Error(err)
 		}
 		if err := runCmdPassThrough("wsl", "--unregister", toDist(v.Name)); err != nil {