Machine: Teardown on init failure

If init fails, or if a SIGINT is sent during init, podman machine should remove all files and configs
created during the init. This includes config jsons, image files, ssh
id's, and system connections. On Windows, the VM instances are also
unregistered.

Signed-off-by: Ashley Cui <acui@redhat.com>

pkg/machine/applehv/machine.go

@@ -188,7 +188,15 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	var (
 		key          string
 		virtiofsMnts []machine.VirtIoFs
+		err          error
 	)
+
+	// cleanup half-baked files if init fails at any point
+	callbackFuncs := machine.InitCleanup()
+	defer callbackFuncs.CleanIfErr(&err)
+	go callbackFuncs.CleanOnSignal()
+
+	callbackFuncs.Add(m.ConfigPath.Delete)
 	dataDir, err := machine.GetDataDir(machine.AppleHvVirt)
 	if err != nil {
 		return false, err
@@ -207,6 +215,7 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	if err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(imagePath.Delete)
 
 	// Set the values for imagePath and strm
 	m.ImagePath = *imagePath
@@ -216,6 +225,8 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	if err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(logPath.Delete)
+
 	m.LogPath = *logPath
 	runtimeDir, err := m.getRuntimeDir()
 	if err != nil {
@@ -228,11 +239,11 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	}
 	m.ReadySocket = *readySocket
 
-	if err := m.setGVProxyInfo(runtimeDir); err != nil {
+	if err = m.setGVProxyInfo(runtimeDir); err != nil {
 		return false, err
 	}
 
-	if err := m.setVfkitInfo(cfg, m.ReadySocket); err != nil {
+	if err = m.setVfkitInfo(cfg, m.ReadySocket); err != nil {
 		return false, err
 	}
 
@@ -248,7 +259,7 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	}
 	m.Port = sshPort
 
-	if err := m.addMountsToVM(opts, &virtiofsMnts); err != nil {
+	if err = m.addMountsToVM(opts, &virtiofsMnts); err != nil {
 		return false, err
 	}
 
@@ -263,22 +274,23 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	if err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(m.removeSystemConnections)
 
 	logrus.Debugf("resizing disk to %d GiB", opts.DiskSize)
-	if err := m.resizeDisk(strongunits.GiB(opts.DiskSize)); err != nil {
+	if err = m.resizeDisk(strongunits.GiB(opts.DiskSize)); err != nil {
 		return false, err
 	}
 
-	if err := m.writeConfig(); err != nil {
+	if err = m.writeConfig(); err != nil {
 		return false, err
 	}
 
 	if len(opts.IgnitionPath) < 1 {
-		var err error
 		key, err = machine.CreateSSHKeys(m.IdentityPath)
 		if err != nil {
 			return false, err
 		}
+		callbackFuncs.Add(m.removeSSHKeys)
 	}
 
 	if len(opts.IgnitionPath) > 0 {
@@ -290,9 +302,22 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	}
 	// TODO Ignition stuff goes here
 	err = m.writeIgnitionConfigFile(opts, key, &virtiofsMnts)
+	callbackFuncs.Add(m.IgnitionFile.Delete)
+
 	return err == nil, err
 }
 
+func (m *MacMachine) removeSSHKeys() error {
+	if err := os.Remove(fmt.Sprintf("%s.pub", m.IdentityPath)); err != nil {
+		logrus.Error(err)
+	}
+	return os.Remove(m.IdentityPath)
+}
+
+func (m *MacMachine) removeSystemConnections() error {
+	return machine.RemoveConnections(m.Name, fmt.Sprintf("%s-root", m.Name))
+}
+
 func (m *MacMachine) Inspect() (*machine.InspectInfo, error) {
 	vmState, err := m.Vfkit.state()
 	if err != nil {

pkg/machine/cleanup.go

@@ -0,0 +1,68 @@
+package machine
+
+import (
+	"os"
+	"os/signal"
+	"sync"
+	"syscall"
+
+	"github.com/sirupsen/logrus"
+)
+
+type CleanupCallback struct {
+	Funcs []func() error
+	mu    sync.Mutex
+}
+
+func (c *CleanupCallback) CleanIfErr(err *error) {
+	// Do not remove created files if the init is successful
+	if *err == nil {
+		return
+	}
+	c.clean()
+}
+
+func (c *CleanupCallback) CleanOnSignal() {
+	ch := make(chan os.Signal, 1)
+	signal.Notify(ch, os.Interrupt, syscall.SIGTERM)
+
+	_, ok := <-ch
+	if !ok {
+		return
+	}
+
+	c.clean()
+	os.Exit(1)
+}
+
+func (c *CleanupCallback) clean() {
+	c.mu.Lock()
+	// Claim exclusive usage by copy and resetting to nil
+	funcs := c.Funcs
+	c.Funcs = nil
+	c.mu.Unlock()
+
+	// Already claimed or none set
+	if funcs == nil {
+		return
+	}
+
+	// Cleanup functions can now exclusively be run
+	for _, cleanfunc := range funcs {
+		if err := cleanfunc(); err != nil {
+			logrus.Error(err)
+		}
+	}
+}
+
+func InitCleanup() CleanupCallback {
+	return CleanupCallback{
+		Funcs: []func() error{},
+	}
+}
+
+func (c *CleanupCallback) Add(anotherfunc func() error) {
+	c.mu.Lock()
+	c.Funcs = append(c.Funcs, anotherfunc)
+	c.mu.Unlock()
+}

pkg/machine/e2e/init_test.go

@@ -232,7 +232,7 @@ var _ = Describe("podman machine init", func() {
 		Expect(output).To(Equal("/run/podman/podman.sock"))
 	})
 
-	It("init with user mode networking ", func() {
+	It("init with user mode networking", func() {
 		if testProvider.VMType() != machine.WSLVirt {
 			Skip("test is only supported by WSL")
 		}
@@ -249,4 +249,58 @@ var _ = Describe("podman machine init", func() {
 		Expect(inspectSession).To(Exit(0))
 		Expect(inspectSession.outputToString()).To(Equal("true"))
 	})
+
+	It("init should cleanup on failure", func() {
+		i := new(initMachine)
+		name := randomString()
+		session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+
+		Expect(err).ToNot(HaveOccurred())
+		Expect(session).To(Exit(0))
+
+		inspect := new(inspectMachine)
+		inspect = inspect.withFormat("{{.ConfigPath.Path}}")
+		inspectSession, err := mb.setCmd(inspect).run()
+		Expect(err).ToNot(HaveOccurred())
+		cfgpth := inspectSession.outputToString()
+
+		inspect = inspect.withFormat("{{.Image.IgnitionFile.Path}}")
+		inspectSession, err = mb.setCmd(inspect).run()
+		Expect(err).ToNot(HaveOccurred())
+		ign := inspectSession.outputToString()
+
+		inspect = inspect.withFormat("{{.Image.ImagePath.Path}}")
+		inspectSession, err = mb.setCmd(inspect).run()
+		Expect(err).ToNot(HaveOccurred())
+		img := inspectSession.outputToString()
+
+		rm := rmMachine{}
+		removeSession, err := mb.setCmd(rm.withForce().withSaveKeys()).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(removeSession).To(Exit(0))
+
+		// Inspecting a non-existent machine should fail
+		// which means it is gone
+		_, ec, err := mb.toQemuInspectInfo()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(ec).To(Equal(125))
+
+		// Clashing keys - init fails
+		i = new(initMachine)
+		session, err = mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(session).To(Exit(125))
+
+		// ensure files created by init are cleaned up on init failure
+		_, err = os.Stat(img)
+		Expect(err).To(HaveOccurred())
+		_, err = os.Stat(cfgpth)
+		Expect(err).To(HaveOccurred())
+
+		// WSL does not use ignition
+		if testProvider.VMType() != machine.WSLVirt {
+			_, err = os.Stat(ign)
+			Expect(err).To(HaveOccurred())
+		}
+	})
 })

pkg/machine/e2e/rm_test.go

@@ -74,7 +74,6 @@ var _ = Describe("podman machine rm", func() {
 	})
 
 	It("machine rm --save-keys, --save-ignition, --save-image", func() {
-
 		i := new(initMachine)
 		session, err := mb.setCmd(i.withImagePath(mb.imagePath)).run()
 		Expect(err).ToNot(HaveOccurred())
@@ -120,6 +119,5 @@ var _ = Describe("podman machine rm", func() {
 		}
 		_, err = os.Stat(img)
 		Expect(err).ToNot(HaveOccurred())
-
 	})
 })

pkg/machine/hyperv/machine.go

@@ -200,11 +200,25 @@ func (m *HyperVMachine) readAndSplitIgnition() error {
 func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	var (
 		key string
+		err error
 	)
 
-	if err := m.addNetworkAndReadySocketsToRegistry(); err != nil {
+	// cleanup half-baked files if init fails at any point
+	callbackFuncs := machine.InitCleanup()
+	defer callbackFuncs.CleanIfErr(&err)
+	go callbackFuncs.CleanOnSignal()
+
+	callbackFuncs.Add(m.ImagePath.Delete)
+	callbackFuncs.Add(m.ConfigPath.Delete)
+	callbackFuncs.Add(m.unregisterMachine)
+
+	if err = m.addNetworkAndReadySocketsToRegistry(); err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(func() error {
+		m.removeNetworkAndReadySocketsFromRegistry()
+		return nil
+	})
 
 	m.IdentityPath = util.GetIdentityPath(m.Name)
 
@@ -230,13 +244,14 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	if err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(m.removeSystemConnections)
 
 	if len(opts.IgnitionPath) < 1 {
-		var err error
 		key, err = machine.CreateSSHKeys(m.IdentityPath)
 		if err != nil {
 			return false, err
 		}
+		callbackFuncs.Add(m.removeSSHKeys)
 	}
 
 	m.ResourceConfig = machine.ResourceConfig{
@@ -255,6 +270,7 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 		}
 		return false, os.WriteFile(m.IgnitionFile.GetPath(), inputIgnition, 0644)
 	}
+	callbackFuncs.Add(m.IgnitionFile.Delete)
 
 	if err := m.writeConfig(); err != nil {
 		return false, err
@@ -265,7 +281,7 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 		return false, err
 	}
 
-	if err := m.resizeDisk(strongunits.GiB(opts.DiskSize)); err != nil {
+	if err = m.resizeDisk(strongunits.GiB(opts.DiskSize)); err != nil {
 		return false, err
 	}
 	// The ignition file has been written. We now need to
@@ -274,6 +290,26 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	return err == nil, err
 }
 
+func (m *HyperVMachine) unregisterMachine() error {
+	vmm := hypervctl.NewVirtualMachineManager()
+	vm, err := vmm.GetMachine(m.Name)
+	if err != nil {
+		logrus.Error(err)
+	}
+	return vm.Remove("")
+}
+
+func (m *HyperVMachine) removeSSHKeys() error {
+	if err := os.Remove(fmt.Sprintf("%s.pub", m.IdentityPath)); err != nil {
+		logrus.Error(err)
+	}
+	return os.Remove(m.IdentityPath)
+}
+
+func (m *HyperVMachine) removeSystemConnections() error {
+	return machine.RemoveConnections(m.Name, fmt.Sprintf("%s-root", m.Name))
+}
+
 func (m *HyperVMachine) Inspect() (*machine.InspectInfo, error) {
 	vm, err := hypervctl.NewVirtualMachineManager().GetMachine(m.Name)
 	if err != nil {

pkg/machine/qemu/machine.go

@@ -272,7 +272,14 @@ RequiredBy=default.target
 func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	var (
 		key string
+		err error
 	)
+
+	// cleanup half-baked files if init fails at any point
+	callbackFuncs := machine.InitCleanup()
+	defer callbackFuncs.CleanIfErr(&err)
+	go callbackFuncs.CleanOnSignal()
+
 	v.IdentityPath = util.GetIdentityPath(v.Name)
 	v.Rootful = opts.Rootful
 
@@ -285,12 +292,13 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	if err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(imagePath.Delete)
 
 	// Assign values about the download
 	v.ImagePath = *imagePath
 	v.ImageStream = strm.String()
 
-	if err := v.addMountsToVM(opts); err != nil {
+	if err = v.addMountsToVM(opts); err != nil {
 		return false, err
 	}
 
@@ -299,7 +307,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	// Add location of bootable image
 	v.CmdLine.SetBootableImage(v.getImageFile())
 
-	if err := machine.AddSSHConnectionsToPodmanSocket(
+	if err = machine.AddSSHConnectionsToPodmanSocket(
 		v.UID,
 		v.Port,
 		v.IdentityPath,
@@ -309,23 +317,25 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	); err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(v.removeSystemConnections)
 
 	// Write the JSON file
-	if err := v.writeConfig(); err != nil {
+	if err = v.writeConfig(); err != nil {
 		return false, fmt.Errorf("writing JSON file: %w", err)
 	}
+	callbackFuncs.Add(v.ConfigPath.Delete)
 
 	// User has provided ignition file so keygen
 	// will be skipped.
 	if len(opts.IgnitionPath) < 1 {
-		var err error
 		key, err = machine.CreateSSHKeys(v.IdentityPath)
 		if err != nil {
 			return false, err
 		}
+		callbackFuncs.Add(v.removeSSHKeys)
 	}
 	// Run arch specific things that need to be done
-	if err := v.prepare(); err != nil {
+	if err = v.prepare(); err != nil {
 		return false, err
 	}
 	originalDiskSize, err := getDiskSize(v.getImageFile())
@@ -333,7 +343,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 		return false, err
 	}
 
-	if err := v.resizeDisk(opts.DiskSize, originalDiskSize>>(10*3)); err != nil {
+	if err = v.resizeDisk(opts.DiskSize, originalDiskSize>>(10*3)); err != nil {
 		return false, err
 	}
 
@@ -352,9 +362,22 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	}
 
 	err = v.writeIgnitionConfigFile(opts, key)
+	callbackFuncs.Add(v.IgnitionFile.Delete)
+
 	return err == nil, err
 }
 
+func (v *MachineVM) removeSSHKeys() error {
+	if err := os.Remove(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil {
+		logrus.Error(err)
+	}
+	return os.Remove(v.IdentityPath)
+}
+
+func (v *MachineVM) removeSystemConnections() error {
+	return machine.RemoveConnections(v.Name, fmt.Sprintf("%s-root", v.Name))
+}
+
 func (v *MachineVM) Set(_ string, opts machine.SetOptions) ([]error, error) {
 	// If one setting fails to be applied, the others settings will not fail and still be applied.
 	// The setting(s) that failed to be applied will have its errors returned in setErrors

pkg/machine/wsl/machine.go

@@ -391,6 +391,14 @@ func getLegacyLastStart(vm *MachineVM) time.Time {
 // Init writes the json configuration file to the filesystem for
 // other verbs (start, stop)
 func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
+	var (
+		err error
+	)
+	// cleanup half-baked files if init fails at any point
+	callbackFuncs := machine.InitCleanup()
+	defer callbackFuncs.CleanIfErr(&err)
+	go callbackFuncs.CleanOnSignal()
+
 	if cont, err := checkAndInstallWSL(opts); !cont {
 		appendOutputIfError(opts.ReExec, err)
 		return cont, err
@@ -402,20 +410,22 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	v.Version = currentMachineVersion
 
 	if v.UserModeNetworking {
-		if err := verifyWSLUserModeCompat(); err != nil {
+		if err = verifyWSLUserModeCompat(); err != nil {
 			return false, err
 		}
 	}
 
-	if err := downloadDistro(v, opts); err != nil {
+	if err = downloadDistro(v, opts); err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(v.removeMachineImage)
 
 	const prompt = "Importing operating system into WSL (this may take a few minutes on a new WSL install)..."
 	dist, err := provisionWSLDist(v.Name, v.ImagePath, prompt)
 	if err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(v.unprovisionWSL)
 
 	if v.UserModeNetworking {
 		if err = installUserModeDist(dist, v.ImagePath); err != nil {
@@ -436,21 +446,59 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	if err = createKeys(v, dist); err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(v.removeSSHKeys)
 
 	// Cycle so that user change goes into effect
 	_ = terminateDist(dist)
 
-	if err := v.writeConfig(); err != nil {
+	if err = v.writeConfig(); err != nil {
 		return false, err
 	}
+	callbackFuncs.Add(v.removeMachineConfig)
 
-	if err := setupConnections(v, opts); err != nil {
+	if err = setupConnections(v, opts); err != nil {
 		return false, err
 	}
-
+	callbackFuncs.Add(v.removeSystemConnections)
 	return true, nil
 }
 
+func (v *MachineVM) unprovisionWSL() error {
+	if err := terminateDist(toDist(v.Name)); err != nil {
+		logrus.Error(err)
+	}
+	if err := unregisterDist(toDist(v.Name)); err != nil {
+		logrus.Error(err)
+	}
+
+	vmDataDir, err := machine.GetDataDir(vmtype)
+	if err != nil {
+		return err
+	}
+	distDir := filepath.Join(vmDataDir, "wsldist")
+	distTarget := filepath.Join(distDir, v.Name)
+	return machine.GuardedRemoveAll(distTarget)
+}
+
+func (v *MachineVM) removeMachineConfig() error {
+	return machine.GuardedRemoveAll(v.ConfigPath)
+}
+
+func (v *MachineVM) removeMachineImage() error {
+	return machine.GuardedRemoveAll(v.ImagePath)
+}
+
+func (v *MachineVM) removeSSHKeys() error {
+	if err := machine.GuardedRemoveAll(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil {
+		logrus.Error(err)
+	}
+	return machine.GuardedRemoveAll(v.IdentityPath)
+}
+
+func (v *MachineVM) removeSystemConnections() error {
+	return machine.RemoveConnections(v.Name, fmt.Sprintf("%s-root", v.Name))
+}
+
 func downloadDistro(v *MachineVM, opts machine.InitOptions) error {
 	var (
 		dd  machine.DistributionDownload
@@ -1003,7 +1051,7 @@ func wslPipe(input string, dist string, arg ...string) error {
 }
 
 func wslCreateKeys(identityPath string, dist string) (string, error) {
-	return machine.CreateSSHKeysPrefix(identityPath, true, true, "wsl", "-u", "root", "-d", dist)
+	return machine.CreateSSHKeysPrefix(identityPath, true, false, "wsl", "-u", "root", "-d", dist)
 }
 
 func runCmdPassThrough(name string, arg ...string) error {