Skip to content

Commit

Permalink
Don't panic on podman4 machine configs
Browse files Browse the repository at this point in the history
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 <bbaude@redhat.com>
  • Loading branch information
baude committed Feb 12, 2024
1 parent a7b20b6 commit d3328d4
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 15 deletions.
9 changes: 9 additions & 0 deletions pkg/machine/define/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
15 changes: 0 additions & 15 deletions pkg/machine/e2e/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
// }
132 changes: 132 additions & 0 deletions pkg/machine/e2e/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
`)
2 changes: 2 additions & 0 deletions pkg/machine/shim/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions pkg/machine/vmconfigs/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/containers/storage/pkg/lockfile"
)

const MachineConfigVersion = 1

type MachineConfig struct {
// Common stuff
Created time.Time
Expand Down
20 changes: 20 additions & 0 deletions pkg/machine/vmconfigs/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d3328d4

Please sign in to comment.