From b01a330d371e924ef9f70011960d169f58faf867 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Thu, 4 Jan 2024 13:35:36 -0500 Subject: [PATCH 1/2] Use single persistent ssh key for all machines Changes SSH key behavior such that there is a single persisted key for all machines across all providers. If there is no key that is located at `.local/share/containers/podman/machine/` then it is created. The keys are not deleted when the last machine on the host is removed. The main motivation for this change is it leads to fewer files created on the host as a result of vm configuration. Having `n` machines on your system doesn't result in `2n` machine-related files in `.ssh` on your system anymore. As a result of ssh keys being persisted by default, the `--save-keys` flag on `podman machine rm` will no longer be supported. Signed-off-by: Jake Correnti --- cmd/podman/machine/rm.go | 3 - docs/source/locale/ja/LC_MESSAGES/markdown.po | 4 -- docs/source/markdown/podman-machine-rm.1.md | 9 +-- pkg/machine/applehv/machine.go | 17 +---- pkg/machine/config.go | 1 - pkg/machine/define/config.go | 1 + pkg/machine/e2e/config_rm_test.go | 10 --- pkg/machine/e2e/init_test.go | 2 +- pkg/machine/e2e/rm_test.go | 72 ++++++++++++++++++- pkg/machine/hyperv/machine.go | 18 +---- pkg/machine/keys.go | 14 ++++ pkg/machine/qemu/machine.go | 17 +---- pkg/machine/wsl/machine.go | 17 +---- pkg/util/utils.go | 2 +- pkg/util/utils_test.go | 2 +- 15 files changed, 101 insertions(+), 88 deletions(-) diff --git a/cmd/podman/machine/rm.go b/cmd/podman/machine/rm.go index ca1128e5d4..d25922be11 100644 --- a/cmd/podman/machine/rm.go +++ b/cmd/podman/machine/rm.go @@ -41,9 +41,6 @@ func init() { formatFlagName := "force" flags.BoolVarP(&destroyOptions.Force, formatFlagName, "f", false, "Stop and do not prompt before rming") - keysFlagName := "save-keys" - flags.BoolVar(&destroyOptions.SaveKeys, keysFlagName, false, "Do not delete SSH keys") - ignitionFlagName := "save-ignition" flags.BoolVar(&destroyOptions.SaveIgnition, ignitionFlagName, false, "Do not delete ignition file") diff --git a/docs/source/locale/ja/LC_MESSAGES/markdown.po b/docs/source/locale/ja/LC_MESSAGES/markdown.po index d625c9c3e5..daba666e6d 100644 --- a/docs/source/locale/ja/LC_MESSAGES/markdown.po +++ b/docs/source/locale/ja/LC_MESSAGES/markdown.po @@ -18478,10 +18478,6 @@ msgstr "" msgid "Do not delete the VM image." msgstr "" -#: ../../source/markdown/podman-machine-rm.1.md:43 -msgid "**--save-keys**" -msgstr "" - #: ../../source/markdown/podman-machine-rm.1.md:45 msgid "" "Do not delete the SSH keys for the VM. The system connection is always " diff --git a/docs/source/markdown/podman-machine-rm.1.md b/docs/source/markdown/podman-machine-rm.1.md index 1224adbe1d..9aa90c35ad 100644 --- a/docs/source/markdown/podman-machine-rm.1.md +++ b/docs/source/markdown/podman-machine-rm.1.md @@ -10,7 +10,7 @@ podman\-machine\-rm - Remove a virtual machine Remove a virtual machine and its related files. What is actually deleted depends on the virtual machine type. For all virtual machines, the generated -SSH keys and the podman system connection are deleted. The ignition files +podman system connections are deleted. The ignition files generated for that VM are also removed as is its image file on the filesystem. Users get a display of what is deleted and are required to confirm unless the option `--force` @@ -39,11 +39,6 @@ Do not delete the generated ignition file. Do not delete the VM image. -#### **--save-keys** - -Do not delete the SSH keys for the VM. The system connection is always -deleted. - ## EXAMPLES Remove a VM named "test1": @@ -53,8 +48,6 @@ $ podman machine rm test1 The following files will be deleted: -/home/user/.ssh/test1 -/home/user/.ssh/test1.pub /home/user/.config/containers/podman/machine/qemu/test1.ign /home/user/.local/share/containers/podman/machine/qemu/test1_fedora-coreos-33.20210315.1.0-qemu.x86_64.qcow2 /home/user/.config/containers/podman/machine/qemu/test1.json diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 280ada1e84..44960e26ca 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -22,9 +22,9 @@ import ( "github.com/containers/podman/v4/pkg/machine" "github.com/containers/podman/v4/pkg/machine/applehv/vfkit" "github.com/containers/podman/v4/pkg/machine/define" + "github.com/containers/podman/v4/pkg/machine/ignition" "github.com/containers/podman/v4/pkg/machine/sockets" "github.com/containers/podman/v4/pkg/machine/vmconfigs" - "github.com/containers/podman/v4/pkg/machine/ignition" "github.com/containers/podman/v4/pkg/strongunits" "github.com/containers/podman/v4/pkg/systemd/parser" "github.com/containers/podman/v4/pkg/util" @@ -202,7 +202,7 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) { return false, err } - m.IdentityPath = util.GetIdentityPath(m.Name) + m.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName) m.Rootful = opts.Rootful m.RemoteUsername = opts.Username @@ -241,11 +241,10 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) { } if len(opts.IgnitionPath) < 1 { - key, err = machine.CreateSSHKeys(m.IdentityPath) + key, err = machine.GetSSHKeys(m.IdentityPath) if err != nil { return false, err } - callbackFuncs.Add(m.removeSSHKeys) } builder := ignition.NewIgnitionBuilder(ignition.DynamicIgnition{ @@ -293,13 +292,6 @@ func createReadyUnitFile() (string, error) { return readyUnit.ToString() } -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)) } @@ -344,9 +336,6 @@ func (m *MacMachine) Inspect() (*machine.InspectInfo, error) { // collectFilesToDestroy retrieves the files that will be destroyed by `Remove` func (m *MacMachine) collectFilesToDestroy(opts machine.RemoveOptions) []string { files := []string{} - if !opts.SaveKeys { - files = append(files, m.IdentityPath, m.IdentityPath+".pub") - } if !opts.SaveIgnition { files = append(files, m.IgnitionFile.GetPath()) } diff --git a/pkg/machine/config.go b/pkg/machine/config.go index 57491ab94f..9698e20d7c 100644 --- a/pkg/machine/config.go +++ b/pkg/machine/config.go @@ -113,7 +113,6 @@ type StopOptions struct{} type RemoveOptions struct { Force bool - SaveKeys bool SaveImage bool SaveIgnition bool } diff --git a/pkg/machine/define/config.go b/pkg/machine/define/config.go index ba98908be1..f7a8bbd48d 100644 --- a/pkg/machine/define/config.go +++ b/pkg/machine/define/config.go @@ -1,3 +1,4 @@ package define const UserCertsTargetPath = "/etc/containers/certs.d" +const DefaultIdentityName = "machine" diff --git a/pkg/machine/e2e/config_rm_test.go b/pkg/machine/e2e/config_rm_test.go index f0cf15a06f..c638e6ac05 100644 --- a/pkg/machine/e2e/config_rm_test.go +++ b/pkg/machine/e2e/config_rm_test.go @@ -5,13 +5,11 @@ type rmMachine struct { -f, --force Stop and do not prompt before rming --save-ignition Do not delete ignition file --save-image Do not delete the image file - --save-keys Do not delete SSH keys */ force bool saveIgnition bool saveImage bool - saveKeys bool cmd []string } @@ -27,9 +25,6 @@ func (i *rmMachine) buildCmd(m *machineTestBuilder) []string { if i.saveImage { cmd = append(cmd, "--save-image") } - if i.saveKeys { - cmd = append(cmd, "--save-keys") - } cmd = append(cmd, m.name) i.cmd = cmd return cmd @@ -49,8 +44,3 @@ func (i *rmMachine) withSaveImage() *rmMachine { i.saveImage = true return i } - -func (i *rmMachine) withSaveKeys() *rmMachine { - i.saveKeys = true - return i -} diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index 84f064777b..c6bb379ad8 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -303,7 +303,7 @@ var _ = Describe("podman machine init", func() { img := inspectSession.outputToString() rm := rmMachine{} - removeSession, err := mb.setCmd(rm.withForce().withSaveKeys()).run() + removeSession, err := mb.setCmd(rm.withForce()).run() Expect(err).ToNot(HaveOccurred()) Expect(removeSession).To(Exit(0)) diff --git a/pkg/machine/e2e/rm_test.go b/pkg/machine/e2e/rm_test.go index a3a1ab6ef0..11f76de25b 100644 --- a/pkg/machine/e2e/rm_test.go +++ b/pkg/machine/e2e/rm_test.go @@ -3,6 +3,7 @@ package e2e_test import ( "fmt" "os" + "path/filepath" "github.com/containers/podman/v4/pkg/machine/define" . "github.com/onsi/ginkgo/v2" @@ -80,7 +81,7 @@ var _ = Describe("podman machine rm", func() { Expect(ec).To(Equal(125)) }) - It("machine rm --save-keys, --save-ignition, --save-image", func() { + It("machine rm --save-ignition --save-image", func() { i := new(initMachine) session, err := mb.setCmd(i.withImagePath(mb.imagePath)).run() Expect(err).ToNot(HaveOccurred()) @@ -104,7 +105,7 @@ var _ = Describe("podman machine rm", func() { img := inspectSession.outputToString() rm := rmMachine{} - removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage().withSaveKeys()).run() + removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage()).run() Expect(err).ToNot(HaveOccurred()) Expect(removeSession).To(Exit(0)) @@ -127,4 +128,71 @@ var _ = Describe("podman machine rm", func() { _, err = os.Stat(img) Expect(err).ToNot(HaveOccurred()) }) + + It("Remove machine sharing ssh key with another machine", func() { + expectedIdentityPathSuffix := filepath.Join(".local", "share", "containers", "podman", "machine", define.DefaultIdentityName) + + fooName := "foo" + foo := new(initMachine) + session, err := mb.setName(fooName).setCmd(foo.withImagePath(mb.imagePath)).run() + Expect(err).ToNot(HaveOccurred()) + Expect(session).To(Exit(0)) + + barName := "bar" + bar := new(initMachine) + session, err = mb.setName(barName).setCmd(bar.withImagePath(mb.imagePath).withNow()).run() + Expect(err).ToNot(HaveOccurred()) + Expect(session).To(Exit(0)) + + inspectFoo := new(inspectMachine) + inspectFoo = inspectFoo.withFormat("{{.SSHConfig.IdentityPath}}") + inspectSession, err := mb.setName(fooName).setCmd(inspectFoo).run() + Expect(err).ToNot(HaveOccurred()) + Expect(inspectSession).To(Exit(0)) + Expect(inspectSession.outputToString()).To(ContainSubstring(expectedIdentityPathSuffix)) + fooIdentityPath := inspectSession.outputToString() + + inspectBar := new(inspectMachine) + inspectBar = inspectBar.withFormat("{{.SSHConfig.IdentityPath}}") + inspectSession, err = mb.setName(barName).setCmd(inspectBar).run() + Expect(err).ToNot(HaveOccurred()) + Expect(inspectSession).To(Exit(0)) + Expect(inspectSession.outputToString()).To(Equal(fooIdentityPath)) + + rmFoo := new(rmMachine) + stop, err := mb.setName(fooName).setCmd(rmFoo.withForce()).run() + Expect(err).ToNot(HaveOccurred()) + Expect(stop).To(Exit(0)) + + // removal of foo should not affect the ability to ssh into the bar machine + sshBar := new(sshMachine) + sshSession, err := mb.setName(barName).setCmd(sshBar.withSSHCommand([]string{"echo", "foo"})).run() + Expect(err).ToNot(HaveOccurred()) + Expect(sshSession).To(Exit(0)) + }) + + It("Removing all machines doesn't delete ssh keys", func() { + fooName := "foo" + foo := new(initMachine) + session, err := mb.setName(fooName).setCmd(foo.withImagePath(mb.imagePath)).run() + Expect(err).ToNot(HaveOccurred()) + Expect(session).To(Exit(0)) + + inspectFoo := new(inspectMachine) + inspectFoo = inspectFoo.withFormat("{{.SSHConfig.IdentityPath}}") + inspectSession, err := mb.setName(fooName).setCmd(inspectFoo).run() + Expect(err).ToNot(HaveOccurred()) + Expect(inspectSession).To(Exit(0)) + fooIdentityPath := inspectSession.outputToString() + + rmFoo := new(rmMachine) + stop, err := mb.setName(fooName).setCmd(rmFoo.withForce()).run() + Expect(err).ToNot(HaveOccurred()) + Expect(stop).To(Exit(0)) + + _, err = os.Stat(fooIdentityPath) + Expect(err).ToNot(HaveOccurred()) + _, err = os.Stat(fooIdentityPath + ".pub") + Expect(err).ToNot(HaveOccurred()) + }) }) diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 8ff0791358..f2d4c0b3a6 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -21,8 +21,8 @@ import ( "github.com/containers/podman/v4/pkg/machine" "github.com/containers/podman/v4/pkg/machine/define" "github.com/containers/podman/v4/pkg/machine/hyperv/vsock" - "github.com/containers/podman/v4/pkg/machine/vmconfigs" "github.com/containers/podman/v4/pkg/machine/ignition" + "github.com/containers/podman/v4/pkg/machine/vmconfigs" "github.com/containers/podman/v4/pkg/strongunits" "github.com/containers/podman/v4/pkg/systemd/parser" "github.com/containers/podman/v4/pkg/util" @@ -178,7 +178,7 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) { return nil }) - m.IdentityPath = util.GetIdentityPath(m.Name) + m.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName) if m.UID == 0 { m.UID = 1000 @@ -205,11 +205,10 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) { callbackFuncs.Add(m.removeSystemConnections) if len(opts.IgnitionPath) < 1 { - key, err = machine.CreateSSHKeys(m.IdentityPath) + key, err = machine.GetSSHKeys(m.IdentityPath) if err != nil { return false, err } - callbackFuncs.Add(m.removeSSHKeys) } m.ResourceConfig = vmconfigs.ResourceConfig{ @@ -319,13 +318,6 @@ func (m *HyperVMachine) unregisterMachine() error { 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)) } @@ -378,10 +370,6 @@ func (m *HyperVMachine) Inspect() (*machine.InspectInfo, error) { // collectFilesToDestroy retrieves the files that will be destroyed by `Remove` func (m *HyperVMachine) collectFilesToDestroy(opts machine.RemoveOptions, diskPath *string) []string { files := []string{} - - if !opts.SaveKeys { - files = append(files, m.IdentityPath, m.IdentityPath+".pub") - } if !opts.SaveIgnition { files = append(files, m.IgnitionFile.GetPath()) } diff --git a/pkg/machine/keys.go b/pkg/machine/keys.go index d46f759f71..e151aa1174 100644 --- a/pkg/machine/keys.go +++ b/pkg/machine/keys.go @@ -36,6 +36,20 @@ func CreateSSHKeys(writeLocation string) (string, error) { return strings.TrimSuffix(string(b), "\n"), nil } +// GetSSHKeys checks to see if there is a ssh key at the provided location. +// If not, we create the priv and pub keys. The ssh key is then returned. +func GetSSHKeys(identityPath string) (string, error) { + if _, err := os.Stat(identityPath); err == nil { + b, err := os.ReadFile(identityPath + ".pub") + if err != nil { + return "", err + } + return strings.TrimSuffix(string(b), "\n"), nil + } + + return CreateSSHKeys(identityPath) +} + func CreateSSHKeysPrefix(identityPath string, passThru bool, skipExisting bool, prefix ...string) (string, error) { _, e := os.Stat(identityPath) if !skipExisting || errors.Is(e, os.ErrNotExist) { diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 0845626ae6..f06b02f30c 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -124,7 +124,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { defer callbackFuncs.CleanIfErr(&err) go callbackFuncs.CleanOnSignal() - v.IdentityPath = util.GetIdentityPath(v.Name) + v.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName) v.Rootful = opts.Rootful imagePath, strm, err := machine.Pull(opts.ImagePath, opts.Name, VirtualizationProvider()) @@ -169,11 +169,10 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { // User has provided ignition file so keygen // will be skipped. if len(opts.IgnitionPath) < 1 { - key, err = machine.CreateSSHKeys(v.IdentityPath) + key, err = machine.GetSSHKeys(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 { @@ -238,13 +237,6 @@ func createReadyUnitFile() (string, error) { return readyUnit.ToString() } -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)) } @@ -923,9 +915,6 @@ func NewQMPMonitor(network, name string, timeout time.Duration) (command.Monitor func (v *MachineVM) collectFilesToDestroy(opts machine.RemoveOptions) ([]string, error) { files := []string{} // Collect all the files that need to be destroyed - if !opts.SaveKeys { - files = append(files, v.IdentityPath, v.IdentityPath+".pub") - } if !opts.SaveIgnition { files = append(files, v.getIgnitionFile()) } @@ -968,7 +957,7 @@ func (v *MachineVM) removeQMPMonitorSocketAndVMPidFile() { } } -// Remove deletes all the files associated with a machine including ssh keys, the image itself +// Remove deletes all the files associated with a machine including the image itself func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() error, error) { var ( files []string diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index efb5485239..ee0301070e 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -19,8 +19,8 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/podman/v4/pkg/machine" "github.com/containers/podman/v4/pkg/machine/define" - "github.com/containers/podman/v4/pkg/machine/vmconfigs" "github.com/containers/podman/v4/pkg/machine/ignition" + "github.com/containers/podman/v4/pkg/machine/vmconfigs" "github.com/containers/podman/v4/pkg/machine/wsl/wutil" "github.com/containers/podman/v4/pkg/util" "github.com/containers/podman/v4/utils" @@ -411,7 +411,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { } _ = setupWslProxyEnv() - v.IdentityPath = util.GetIdentityPath(v.Name) + v.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName) v.Rootful = opts.Rootful v.Version = currentMachineVersion @@ -452,7 +452,6 @@ 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) @@ -494,13 +493,6 @@ func (v *MachineVM) removeMachineImage() error { return utils.GuardedRemoveAll(v.ImagePath) } -func (v *MachineVM) removeSSHKeys() error { - if err := utils.GuardedRemoveAll(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil { - logrus.Error(err) - } - return utils.GuardedRemoveAll(v.IdentityPath) -} - func (v *MachineVM) removeSystemConnections() error { return machine.RemoveConnections(v.Name, fmt.Sprintf("%s-root", v.Name)) } @@ -1057,7 +1049,7 @@ func wslPipe(input string, dist string, arg ...string) error { } func wslCreateKeys(identityPath string, dist string) (string, error) { - return machine.CreateSSHKeysPrefix(identityPath, true, false, "wsl", "-u", "root", "-d", dist) + return machine.CreateSSHKeysPrefix(identityPath, true, true, "wsl", "-u", "root", "-d", dist) } func runCmdPassThrough(name string, arg ...string) error { @@ -1605,9 +1597,6 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun defer v.lock.Unlock() // Collect all the files that need to be destroyed - if !opts.SaveKeys { - files = append(files, v.IdentityPath, v.IdentityPath+".pub") - } if !opts.SaveImage { files = append(files, v.ImagePath) } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 04abffa36f..b3ccbb4c23 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -1184,7 +1184,7 @@ func ExitCode(err error) int { } func GetIdentityPath(name string) string { - return filepath.Join(homedir.Get(), ".ssh", name) + return filepath.Join(homedir.Get(), ".local", "share", "containers", "podman", "machine", name) } func Tmpdir() string { diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go index e3c303a6e9..38a65f453f 100644 --- a/pkg/util/utils_test.go +++ b/pkg/util/utils_test.go @@ -531,7 +531,7 @@ func TestValidateSysctlBadSysctlWithExtraSpaces(t *testing.T) { func TestGetIdentityPath(t *testing.T) { name := "p-test" identityPath := GetIdentityPath(name) - assert.Equal(t, identityPath, filepath.Join(homedir.Get(), ".ssh", name)) + assert.Equal(t, identityPath, filepath.Join(homedir.Get(), ".local", "share", "containers", "podman", "machine", name)) } func TestCoresToPeriodAndQuota(t *testing.T) { From 3bfdd791502b725494f5c43eafc94e34a3ff6d40 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Thu, 4 Jan 2024 20:35:45 -0500 Subject: [PATCH 2/2] Fix init teardown on bad ignition path Fixes a bug where if a machine failed during init due to a bad ignition path, it would not be properly torn down. Signed-off-by: Jake Correnti --- pkg/machine/applehv/machine.go | 3 ++- pkg/machine/e2e/init_test.go | 25 +++++++++++++------------ pkg/machine/hyperv/machine.go | 3 ++- pkg/machine/qemu/machine.go | 3 ++- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 44960e26ca..91feb8e29a 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -259,7 +259,8 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) { }) if len(opts.IgnitionPath) > 0 { - return false, builder.BuildWithIgnitionFile(opts.IgnitionPath) + err = builder.BuildWithIgnitionFile(opts.IgnitionPath) + return false, err } if err := builder.GenerateIgnitionConfig(); err != nil { diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index c6bb379ad8..27b0b3d060 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -313,20 +313,21 @@ var _ = Describe("podman machine init", func() { 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() != define.WSLVirt { + // Bad ignition path - init fails + i = new(initMachine) + i.ignitionPath = "/bad/path" + 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()) + _, err = os.Stat(ign) Expect(err).To(HaveOccurred()) } diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index f2d4c0b3a6..ea8d522144 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -232,7 +232,8 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) { // If the user provides an ignition file, we need to // copy it into the conf dir if len(opts.IgnitionPath) > 0 { - return false, builder.BuildWithIgnitionFile(opts.IgnitionPath) + err = builder.BuildWithIgnitionFile(opts.IgnitionPath) + return false, err } callbackFuncs.Add(m.IgnitionFile.Delete) diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index f06b02f30c..14ab01d790 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -205,7 +205,8 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { // If the user provides an ignition file, we need to // copy it into the conf dir if len(opts.IgnitionPath) > 0 { - return false, builder.BuildWithIgnitionFile(opts.IgnitionPath) + err = builder.BuildWithIgnitionFile(opts.IgnitionPath) + return false, err } if err := builder.GenerateIgnitionConfig(); err != nil {