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 <[email protected]>
jakecorrenti · Feb 6, 2024 · b39be0c · b39be0c
1 parent 82c654a
commit b39be0c
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
 }