Merge pull request #21573 from baude/nofail

Don't panic on podman4 machine configs

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)
+}

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
-// }

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"
+}
+`)

pkg/machine/shim/host.go

@@ -90,6 +90,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,

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

pkg/machine/vmconfigs/machine.go

@@ -313,6 +313,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
 }
 
@@ -346,6 +356,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