diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 6541ee8dc7..1253b1c802 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -192,7 +192,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 @@ -211,6 +219,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 @@ -220,6 +229,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 { @@ -232,11 +243,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 } @@ -252,7 +263,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 } @@ -267,22 +278,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 { @@ -294,9 +306,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 { diff --git a/pkg/machine/cleanup.go b/pkg/machine/cleanup.go new file mode 100644 index 0000000000..ff23846cf7 --- /dev/null +++ b/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() +} diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index 4ea370dff0..948d67a836 100644 --- a/pkg/machine/e2e/init_test.go +++ b/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()) + } + }) }) diff --git a/pkg/machine/e2e/rm_test.go b/pkg/machine/e2e/rm_test.go index 8439632f47..f074961c14 100644 --- a/pkg/machine/e2e/rm_test.go +++ b/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()) - }) }) diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index b8ffa3bae4..dd1e73ff6f 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -203,11 +203,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) @@ -233,13 +247,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{ @@ -258,6 +273,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 @@ -268,7 +284,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 @@ -277,6 +293,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 { diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index e90eacf478..0c0bbbb044 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -249,7 +249,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 @@ -262,12 +269,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 } @@ -276,7 +284,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, @@ -286,23 +294,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()) @@ -310,7 +320,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 } @@ -329,9 +339,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 diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index cf1048b2e5..d29aa02365 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -394,6 +394,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 @@ -405,20 +413,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 { @@ -439,21 +449,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 @@ -1006,7 +1054,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 {