From d3328d4f32c6c5e1a69c0c8954c0a8550a5a419b Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Wed, 7 Feb 2024 09:15:19 -0600 Subject: [PATCH] Don't panic on podman4 machine configs we should not panic podman when it has to deal with a podman4 machine config. instead, we throw a soft error for `machine ls` and in all other cases, we throw a hard error stating that the machine config is incompatible. a future PR will provide instructions on how to recover from this. current idea is something like `podman machine reset` which blows everything away machine-wise. Signed-off-by: Brent Baude --- pkg/machine/define/errors.go | 9 +++ pkg/machine/e2e/config_test.go | 15 ---- pkg/machine/e2e/init_test.go | 132 +++++++++++++++++++++++++++++++ pkg/machine/shim/host.go | 2 + pkg/machine/vmconfigs/config.go | 2 + pkg/machine/vmconfigs/machine.go | 20 +++++ 6 files changed, 165 insertions(+), 15 deletions(-) diff --git a/pkg/machine/define/errors.go b/pkg/machine/define/errors.go index 46af3ec81b..1a32bc5165 100644 --- a/pkg/machine/define/errors.go +++ b/pkg/machine/define/errors.go @@ -40,3 +40,12 @@ type ErrNewDiskSizeTooSmall struct { func (err *ErrNewDiskSizeTooSmall) Error() string { return fmt.Sprintf("invalid disk size %d: new disk must be larger than %dGB", err.OldSize, err.NewSize) } + +type ErrIncompatibleMachineConfig struct { + Name string + Path string +} + +func (err *ErrIncompatibleMachineConfig) Error() string { + return fmt.Sprintf("incompatible machine config %q (%s) for this version of Podman", err.Path, err.Name) +} diff --git a/pkg/machine/e2e/config_test.go b/pkg/machine/e2e/config_test.go index 8a1b432c1b..2dd56bc217 100644 --- a/pkg/machine/e2e/config_test.go +++ b/pkg/machine/e2e/config_test.go @@ -235,18 +235,3 @@ func isVmtype(vmType define.VMType) bool { func isWSL() bool { return isVmtype(define.WSLVirt) } - -// TODO temporarily suspended -// func getFCOSDownloadLocation(p vmconfigs.VMStubber) string { -// dd, err := p.NewDownload("") -// if err != nil { -// Fail("unable to create new download") -// } -// -// fcd, err := dd.GetFCOSDownload(defaultStream) -// if err != nil { -// Fail("unable to get virtual machine image") -// } -// -// return fcd.Location -// } diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index 51f0f20abb..25b0f307e8 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -315,4 +315,136 @@ var _ = Describe("podman machine init", func() { Expect(err).To(HaveOccurred()) } }) + + It("verify a podman 4 config does not break podman 5", func() { + vmName := "foobar-machine" + configDir := filepath.Join(testDir, ".config", "containers", "podman", "machine", testProvider.VMType().String()) + if err := os.MkdirAll(configDir, 0755); err != nil { + Expect(err).ToNot(HaveOccurred()) + } + f, err := os.Create(filepath.Join(configDir, fmt.Sprintf("%s.json", vmName))) + Expect(err).ToNot(HaveOccurred()) + if _, err := f.Write(p4Config); err != nil { + Expect(err).ToNot(HaveOccurred()) + } + err = f.Close() + Expect(err).ToNot(HaveOccurred()) + + // At this point we have a p4 config in the config dir + // podman machine list should emit a "soft" error but complete + list := new(listMachine) + firstList, err := mb.setCmd(list).run() + Expect(err).NotTo(HaveOccurred()) + Expect(firstList).Should(Exit(0)) + Expect(firstList.errorToString()).To(ContainSubstring("incompatible machine config")) + + // podman machine inspect should fail because we are + // trying to work with the incompatible machine json + ins := inspectMachine{} + inspectShouldFail, err := mb.setName(vmName).setCmd(&ins).run() + Expect(err).ToNot(HaveOccurred()) + Expect(inspectShouldFail).To(Exit(125)) + Expect(inspectShouldFail.errorToString()).To(ContainSubstring("incompatible machine config")) + + // We should be able to init with a bad config present + 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)) + + // We should still be able to ls + secondList, err := mb.setCmd(list).run() + Expect(err).NotTo(HaveOccurred()) + Expect(secondList).Should(Exit(0)) + + // And inspecting the valid machine should not error + inspectShouldPass, err := mb.setName(name).setCmd(&ins).run() + Expect(err).ToNot(HaveOccurred()) + Expect(inspectShouldPass).To(Exit(0)) + }) }) + +var p4Config = []byte(`{ + "ConfigPath": { + "Path": "/home/baude/.config/containers/podman/machine/qemu/podman-machine-default.json" + }, + "CmdLine": [ + "/usr/bin/qemu-system-x86_64", + "-accel", + "kvm", + "-cpu", + "host", + "-m", + "2048", + "-smp", + "12", + "-fw_cfg", + "name=opt/com.coreos/config,file=/home/baude/.config/containers/podman/machine/qemu/podman-machine-default.ign", + "-qmp", + "unix:/run/user/1000/podman/qmp_podman-machine-default.sock,server=on,wait=off", + "-netdev", + "socket,id=vlan,fd=3", + "-device", + "virtio-net-pci,netdev=vlan,mac=5a:94:ef:e4:0c:ee", + "-device", + "virtio-serial", + "-chardev", + "socket,path=/run/user/1000/podman/podman-machine-default_ready.sock,server=on,wait=off,id=apodman-machine-default_ready", + "-device", + "virtserialport,chardev=apodman-machine-default_ready,name=org.fedoraproject.port.0", + "-pidfile", + "/run/user/1000/podman/podman-machine-default_vm.pid", + "-virtfs", + "local,path=/home/baude,mount_tag=vol0,security_model=none", + "-drive", + "if=virtio,file=/home/baude/.local/share/containers/podman/machine/qemu/podman-machine-default_fedora-coreos-39.20240128.2.2-qemu.x86_64.qcow2" + ], + "Rootful": false, + "UID": 1000, + "HostUserModified": false, + "IgnitionFilePath": { + "Path": "/home/baude/.config/containers/podman/machine/qemu/podman-machine-default.ign" + }, + "ImageStream": "testing", + "ImagePath": { + "Path": "/home/baude/.local/share/containers/podman/machine/qemu/podman-machine-default_fedora-coreos-39.20240128.2.2-qemu.x86_64.qcow2" + }, + "Mounts": [ + { + "ReadOnly": false, + "Source": "/home/baude", + "Tag": "vol0", + "Target": "/home/baude", + "Type": "9p" + } + ], + "Name": "podman-machine-default", + "PidFilePath": { + "Path": "/run/user/1000/podman/podman-machine-default_proxy.pid" + }, + "VMPidFilePath": { + "Path": "/run/user/1000/podman/podman-machine-default_vm.pid" + }, + "QMPMonitor": { + "Address": { + "Path": "/run/user/1000/podman/qmp_podman-machine-default.sock" + }, + "Network": "unix", + "Timeout": 2000000000 + }, + "ReadySocket": { + "Path": "/run/user/1000/podman/podman-machine-default_ready.sock" + }, + "CPUs": 12, + "DiskSize": 100, + "Memory": 2048, + "USBs": [], + "IdentityPath": "/home/baude/.local/share/containers/podman/machine/machine", + "Port": 38419, + "RemoteUsername": "core", + "Starting": false, + "Created": "2024-02-08T10:34:14.067604999-06:00", + "LastUp": "0001-01-01T00:00:00Z" +} +`) diff --git a/pkg/machine/shim/host.go b/pkg/machine/shim/host.go index ece54f4dd5..8c04afeeeb 100644 --- a/pkg/machine/shim/host.go +++ b/pkg/machine/shim/host.go @@ -109,6 +109,8 @@ func Init(opts machineDefine.InitOptions, mp vmconfigs.VMProvider) (*vmconfigs.M return nil, err } + mc.Version = vmconfigs.MachineConfigVersion + createOpts := machineDefine.CreateVMOpts{ Name: opts.Name, Dirs: dirs, diff --git a/pkg/machine/vmconfigs/config.go b/pkg/machine/vmconfigs/config.go index 44179c0fc0..a1cf90005d 100644 --- a/pkg/machine/vmconfigs/config.go +++ b/pkg/machine/vmconfigs/config.go @@ -12,6 +12,8 @@ import ( "github.com/containers/storage/pkg/lockfile" ) +const MachineConfigVersion = 1 + type MachineConfig struct { // Common stuff Created time.Time diff --git a/pkg/machine/vmconfigs/machine.go b/pkg/machine/vmconfigs/machine.go index 1176c92109..95d154aae4 100644 --- a/pkg/machine/vmconfigs/machine.go +++ b/pkg/machine/vmconfigs/machine.go @@ -312,6 +312,16 @@ func LoadMachineByName(name string, dirs *define.MachineDirs) (*MachineConfig, e } mc.dirs = dirs mc.configPath = fullPath + + // If we find an incompatible configuration, we return a hard + // error because the user wants to deal directly with this + // machine + if mc.Version == 0 { + return mc, &define.ErrIncompatibleMachineConfig{ + Name: name, + Path: fullPath.GetPath(), + } + } return mc, nil } @@ -345,6 +355,16 @@ func LoadMachinesInDir(dirs *define.MachineDirs) (map[string]*MachineConfig, err if err != nil { return err } + // if we find an incompatible machine configuration file, we emit and error + // + if mc.Version == 0 { + tmpErr := &define.ErrIncompatibleMachineConfig{ + Name: mc.Name, + Path: fullPath.GetPath(), + } + logrus.Error(tmpErr) + return nil + } mc.configPath = fullPath mc.dirs = dirs mcs[mc.Name] = mc