From b39be0c98aa933dc24ae3b15324988ed1dc4e6f3 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Mon, 5 Feb 2024 12:54:30 -0500 Subject: [PATCH] Add functionality for `podman machine set --rootful` Adds the functionality for `podman machine set --rootful` for AppleHV, QEMU, and HyperV. Abstracts the functionality out to a method of `MachineConfig`. WSL currently uses a function `SetRootful` that is provided by the `machine` package, which will eventually get changed when WSL moves to the refactored structure. Re-enables the "set rootful with docker sock change" test. [NO NEW TESTS NEEDED] Signed-off-by: Jake Correnti --- cmd/podman/machine/set.go | 5 +++-- pkg/machine/applehv/stubber.go | 9 ++++++++- pkg/machine/e2e/set_test.go | 2 -- pkg/machine/hyperv/stubber.go | 15 ++++++--------- pkg/machine/qemu/stubber.go | 9 ++++++++- pkg/machine/shim/host.go | 24 ++++++++++++------------ pkg/machine/vmconfigs/config.go | 2 +- pkg/machine/vmconfigs/machine.go | 9 +++++++++ 8 files changed, 47 insertions(+), 28 deletions(-) diff --git a/cmd/podman/machine/set.go b/cmd/podman/machine/set.go index 2a95867884..c0f5637ff0 100644 --- a/cmd/podman/machine/set.go +++ b/cmd/podman/machine/set.go @@ -92,6 +92,7 @@ func setMachine(cmd *cobra.Command, args []string) error { err error newCPUs, newMemory *uint64 newDiskSize *strongunits.GiB + newRootful *bool ) vmName := defaultMachineName @@ -110,7 +111,7 @@ func setMachine(cmd *cobra.Command, args []string) error { } if cmd.Flags().Changed("rootful") { - mc.HostUser.Rootful = setFlags.Rootful + newRootful = &setFlags.Rootful } if cmd.Flags().Changed("cpus") { mc.Resources.CPUs = setFlags.CPUs @@ -139,7 +140,7 @@ func setMachine(cmd *cobra.Command, args []string) error { // At this point, we have the known changed information, etc // Walk through changes to the providers if they need them - if err := provider.SetProviderAttrs(mc, newCPUs, newMemory, newDiskSize); err != nil { + if err := provider.SetProviderAttrs(mc, newCPUs, newMemory, newDiskSize, newRootful); err != nil { return err } diff --git a/pkg/machine/applehv/stubber.go b/pkg/machine/applehv/stubber.go index 5d8a7d3a98..880601c7af 100644 --- a/pkg/machine/applehv/stubber.go +++ b/pkg/machine/applehv/stubber.go @@ -79,12 +79,19 @@ func (a AppleHVStubber) RemoveAndCleanMachines(_ *define.MachineDirs) error { return nil } -func (a AppleHVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB) error { +func (a AppleHVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB, newRootful *bool) error { if newDiskSize != nil { if err := resizeDisk(mc, *newDiskSize); err != nil { return err } } + + if newRootful != nil && mc.HostUser.Rootful != *newRootful { + if err := mc.SetRootful(*newRootful); err != nil { + return err + } + } + // VFKit does not require saving memory, disk, or cpu return nil } diff --git a/pkg/machine/e2e/set_test.go b/pkg/machine/e2e/set_test.go index 793d2b9c34..5cc574fd44 100644 --- a/pkg/machine/e2e/set_test.go +++ b/pkg/machine/e2e/set_test.go @@ -136,8 +136,6 @@ var _ = Describe("podman machine set", func() { }) It("set rootful with docker sock change", func() { - // TODO pipes and docker socks need to plumbed into podman 5 still - Skip("Needs to be plumbed in still") name := randomString() i := new(initMachine) session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run() diff --git a/pkg/machine/hyperv/stubber.go b/pkg/machine/hyperv/stubber.go index 9ad5c2b023..5bb7ca2080 100644 --- a/pkg/machine/hyperv/stubber.go +++ b/pkg/machine/hyperv/stubber.go @@ -290,7 +290,7 @@ func stateConversion(s hypervctl.EnabledState) (define.Status, error) { return define.Unknown, fmt.Errorf("unknown state: %q", s.String()) } -func (h HyperVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB) error { +func (h HyperVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB, newRootful *bool) error { var ( cpuChanged, memoryChanged bool ) @@ -308,14 +308,11 @@ func (h HyperVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, cpus, memor return errors.New("unable to change settings unless vm is stopped") } - // Rootful still needs plumbing - //if opts.Rootful != nil && m.Rootful != *opts.Rootful { - // if err := m.setRootful(*opts.Rootful); err != nil { - // setErrors = append(setErrors, fmt.Errorf("failed to set rootful option: %w", err)) - // } else { - // m.Rootful = *opts.Rootful - // } - //} + if newRootful != nil && mc.HostUser.Rootful != *newRootful { + if err := mc.SetRootful(*newRootful); err != nil { + return err + } + } if newDiskSize != nil { if err := resizeDisk(*newDiskSize, mc.ImagePath); err != nil { diff --git a/pkg/machine/qemu/stubber.go b/pkg/machine/qemu/stubber.go index 192de19555..79ccda9f9a 100644 --- a/pkg/machine/qemu/stubber.go +++ b/pkg/machine/qemu/stubber.go @@ -243,12 +243,19 @@ func (q *QEMUStubber) resizeDisk(newSize strongunits.GiB, diskPath *define.VMFil return nil } -func (q *QEMUStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB) error { +func (q *QEMUStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB, newRootful *bool) error { if newDiskSize != nil { if err := q.resizeDisk(*newDiskSize, mc.ImagePath); err != nil { return err } } + + if newRootful != nil && mc.HostUser.Rootful != *newRootful { + if err := mc.SetRootful(*newRootful); err != nil { + return err + } + } + // Because QEMU does nothing with these hardware attributes, we can simply return return nil } diff --git a/pkg/machine/shim/host.go b/pkg/machine/shim/host.go index 04a5c1c22f..fac7df1eaa 100644 --- a/pkg/machine/shim/host.go +++ b/pkg/machine/shim/host.go @@ -373,18 +373,6 @@ func Start(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDe // if there are generic things that need to be done, a preStart function could be added here // should it be extensive - // update the podman/docker socket service if the host user has been modified at all (UID or Rootful) - if mc.HostUser.Modified { - if machine.UpdatePodmanDockerSockService(mc) == nil { - // Reset modification state if there are no errors, otherwise ignore errors - // which are already logged - mc.HostUser.Modified = false - if err := mc.Write(); err != nil { - logrus.Error(err) - } - } - } - // releaseFunc is if the provider starts a vm using a go command // and we still need control of it while it is booting until the ready // socket is tripped @@ -443,5 +431,17 @@ func Start(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDe opts.NoInfo, mc.HostUser.Rootful, ) + + // update the podman/docker socket service if the host user has been modified at all (UID or Rootful) + if mc.HostUser.Modified { + if machine.UpdatePodmanDockerSockService(mc) == nil { + // Reset modification state if there are no errors, otherwise ignore errors + // which are already logged + mc.HostUser.Modified = false + if err := mc.Write(); err != nil { + logrus.Error(err) + } + } + } return nil } diff --git a/pkg/machine/vmconfigs/config.go b/pkg/machine/vmconfigs/config.go index 71ed41ac12..418f1bbd35 100644 --- a/pkg/machine/vmconfigs/config.go +++ b/pkg/machine/vmconfigs/config.go @@ -114,7 +114,7 @@ type VMProvider interface { //nolint:interfacebloat MountVolumesToVM(mc *MachineConfig, quiet bool) error Remove(mc *MachineConfig) ([]string, func() error, error) RemoveAndCleanMachines(dirs *define.MachineDirs) error - SetProviderAttrs(mc *MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB) error + SetProviderAttrs(mc *MachineConfig, cpus, memory *uint64, newDiskSize *strongunits.GiB, newRootful *bool) error StartNetworking(mc *MachineConfig, cmd *gvproxy.GvproxyCommand) error PostStartNetworking(mc *MachineConfig) error StartVM(mc *MachineConfig) (func() error, func() error, error) diff --git a/pkg/machine/vmconfigs/machine.go b/pkg/machine/vmconfigs/machine.go index 0b1575e571..a85d24916f 100644 --- a/pkg/machine/vmconfigs/machine.go +++ b/pkg/machine/vmconfigs/machine.go @@ -125,6 +125,15 @@ func (mc *MachineConfig) write() error { return os.WriteFile(mc.configPath.GetPath(), b, define.DefaultFilePerm) } +func (mc *MachineConfig) SetRootful(rootful bool) error { + if err := connection.UpdateConnectionIfDefault(rootful, mc.Name, mc.Name+"-root"); err != nil { + return err + } + mc.HostUser.Rootful = rootful + mc.HostUser.Modified = true + return nil +} + func (mc *MachineConfig) removeSystemConnection() error { //nolint:unused return define2.ErrNotImplemented }