Merge pull request #22028 from n1hility/fix-api-socket

Change API socket to be machine name isolated

pkg/machine/e2e/inspect_test.go

@@ -68,10 +68,10 @@ var _ = Describe("podman inspect stop", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		switch testProvider.VMType() {
-		case define.WSLVirt:
+		case define.HyperVVirt, define.WSLVirt:
 			Expect(inspectInfo[0].ConnectionInfo.PodmanPipe.GetPath()).To(ContainSubstring("podman-"))
 		default:
-			Expect(inspectInfo[0].ConnectionInfo.PodmanSocket.GetPath()).To(HaveSuffix("podman.sock"))
+			Expect(inspectInfo[0].ConnectionInfo.PodmanSocket.GetPath()).To(HaveSuffix("api.sock"))
 		}
 
 		inspect := new(inspectMachine)
@@ -89,4 +89,31 @@ var _ = Describe("podman inspect stop", func() {
 		Expect(inspectSession).To(Exit(125))
 		Expect(inspectSession.errorToString()).To(ContainSubstring("can't evaluate field Abcde in type machine.InspectInfo"))
 	})
+
+	It("inspect shows a unique socket name per machine", func() {
+		skipIfVmtype(define.WSLVirt, "test is only relevant for Unix based providers")
+		skipIfVmtype(define.HyperVVirt, "test is only relevant for Unix based machines")
+
+		var socks []string
+		for c := 0; c < 2; c++ {
+			name := randomString()
+			i := new(initMachine)
+			session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath)).run()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(session).To(Exit(0))
+
+			// regular inspect should
+			inspectJSON := new(inspectMachine)
+			inspectSession, err := mb.setName(name).setCmd(inspectJSON).run()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(inspectSession).To(Exit(0))
+
+			var inspectInfo []machine.InspectInfo
+			err = jsoniter.Unmarshal(inspectSession.Bytes(), &inspectInfo)
+			Expect(err).ToNot(HaveOccurred())
+			socks = append(socks, inspectInfo[0].ConnectionInfo.PodmanSocket.GetPath())
+		}
+
+		Expect(socks[0]).ToNot(Equal(socks[1]))
+	})
 })

pkg/machine/env/dir.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/containers/podman/v5/pkg/machine/define"
 	"github.com/containers/storage/pkg/homedir"
@@ -149,3 +150,10 @@ func GetSSHIdentityPath(name string) (string, error) {
 	}
 	return filepath.Join(datadir, name), nil
 }
+
+func WithPodmanPrefix(name string) string {
+	if !strings.HasPrefix(name, "podman") {
+		name = "podman-" + name
+	}
+	return name
+}

pkg/machine/machine_common.go

@@ -81,7 +81,7 @@ address can't be used by podman. `
 			fmtString = `You can %sconnect Docker API clients by setting DOCKER_HOST using the
 following command in your terminal session:
 
-        %s'
+        %s
 
 `
 			prefix := ""

pkg/machine/machine_windows.go

@@ -123,7 +123,7 @@ func LaunchWinProxy(opts WinProxyOpts, noInfo bool) {
 }
 
 func launchWinProxy(opts WinProxyOpts) (bool, string, error) {
-	machinePipe := ToDist(opts.Name)
+	machinePipe := env.WithPodmanPrefix(opts.Name)
 	if !PipeNameAvailable(machinePipe, MachineNameWait) {
 		return false, "", fmt.Errorf("could not start api proxy since expected pipe is not available: %s", machinePipe)
 	}
@@ -263,13 +263,6 @@ func GetWinProxyStateDir(name string, vmtype define.VMType) (string, error) {
 	return stateDir, nil
 }
 
-func ToDist(name string) string {
-	if !strings.HasPrefix(name, "podman") {
-		name = "podman-" + name
-	}
-	return name
-}
-
 func GetEnvSetString(env string, val string) string {
 	return fmt.Sprintf("$Env:%s=\"%s\"", env, val)
 }

pkg/machine/shim/networking.go

@@ -111,7 +111,7 @@ func startNetworking(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvider)
 		return "", 0, err
 	}
 
-	hostSocks, forwardSock, forwardingState, err := setupMachineSockets(mc.Name, dirs)
+	hostSocks, forwardSock, forwardingState, err := setupMachineSockets(mc, dirs)
 	if err != nil {
 		return "", 0, err
 	}

pkg/machine/shim/networking_unix.go

@@ -10,63 +10,92 @@ import (
 
 	"github.com/containers/podman/v5/pkg/machine"
 	"github.com/containers/podman/v5/pkg/machine/define"
+	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
 	"github.com/sirupsen/logrus"
 )
 
-func setupMachineSockets(name string, dirs *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) {
-	hostSocket, err := dirs.DataDir.AppendToNewVMFile("podman.sock", nil)
+func setupMachineSockets(mc *vmconfigs.MachineConfig, dirs *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) {
+	hostSocket, err := mc.APISocket()
 	if err != nil {
 		return nil, "", 0, err
 	}
 
-	linkSocketPath := filepath.Dir(dirs.DataDir.GetPath())
-	linkSocket, err := define.NewMachineFile(filepath.Join(linkSocketPath, "podman.sock"), nil)
+	forwardSock, state, err := setupForwardingLinks(hostSocket, dirs.DataDir)
 	if err != nil {
 		return nil, "", 0, err
 	}
-
-	forwardSock, state := setupForwardingLinks(hostSocket, linkSocket)
 	return []string{hostSocket.GetPath()}, forwardSock, state, nil
 }
 
-func setupForwardingLinks(hostSocket, linkSocket *define.VMFile) (string, machine.APIForwardingState) {
-	// The linking pattern is /var/run/docker.sock -> user global sock (link) -> machine sock (socket)
-	// This allows the helper to only have to maintain one constant target to the user, which can be
-	// repositioned without updating docker.sock.
+func setupForwardingLinks(hostSocket, dataDir *define.VMFile) (string, machine.APIForwardingState, error) {
+	// Sets up a cooperative link structure to help a separate privileged
+	// service manage /var/run/docker.sock (currently only on MacOS via
+	// podman-mac-helper, but potentially other OSs in the future).
+	//
+	// The linking pattern is:
+	//
+	// /var/run/docker.sock (link) -> user global sock (link) -> machine sock
+	//
+	// This allows the helper to only have to maintain one constant target to
+	// the user, which can be repositioned without updating docker.sock.
+	//
+	// Since these link locations are global/shared across multiple machine
+	// instances, they must coordinate on the winner. The scheme is first come
+	// first serve, whoever is actively answering on the socket first wins. All
+	// other machine instances backs off. As soon as the winner is no longer
+	// active another machine instance start will become the new active winner.
+	// The same applies to a competing container runtime trying to use
+	// /var/run/docker.sock, if the socket is in use by another runtime, podman
+	// machine will back off. In the start message "Losing" machine instances
+	// will instead advertise the direct machine socket, while "winning"
+	// instances will simply note they listen on the standard
+	// /var/run/docker.sock address. The APIForwardingState return value is
+	// returned by this function to indicate how the start message should behave
 
+	// Skip any OS not supported for helper usage
 	if !dockerClaimSupported() {
-		return hostSocket.GetPath(), machine.ClaimUnsupported
+		return hostSocket.GetPath(), machine.ClaimUnsupported, nil
 	}
 
+	// Verify the helper system service was installed and report back if not
 	if !dockerClaimHelperInstalled() {
-		return hostSocket.GetPath(), machine.NotInstalled
+		return hostSocket.GetPath(), machine.NotInstalled, nil
 	}
 
-	if !alreadyLinked(hostSocket.GetPath(), linkSocket.GetPath()) {
-		if checkSockInUse(linkSocket.GetPath()) {
-			return hostSocket.GetPath(), machine.MachineLocal
+	dataPath := filepath.Dir(dataDir.GetPath())
+	userGlobalSocket, err := define.NewMachineFile(filepath.Join(dataPath, "podman.sock"), nil)
+	if err != nil {
+		return "", 0, err
+	}
+
+	// Setup the user global socket if not in use
+	// (e.g ~/.local/share/containers/podman/machine/podman.sock)
+	if !alreadyLinked(hostSocket.GetPath(), userGlobalSocket.GetPath()) {
+		if checkSockInUse(userGlobalSocket.GetPath()) {
+			return hostSocket.GetPath(), machine.MachineLocal, nil
 		}
 
-		_ = linkSocket.Delete()
+		_ = userGlobalSocket.Delete()
 
-		if err := os.Symlink(hostSocket.GetPath(), linkSocket.GetPath()); err != nil {
+		if err := os.Symlink(hostSocket.GetPath(), userGlobalSocket.GetPath()); err != nil {
 			logrus.Warnf("could not create user global API forwarding link: %s", err.Error())
-			return hostSocket.GetPath(), machine.MachineLocal
+			return hostSocket.GetPath(), machine.MachineLocal, nil
 		}
 	}
 
-	if !alreadyLinked(linkSocket.GetPath(), dockerSock) {
+	// Setup /var/run/docker.sock if not in use
+	if !alreadyLinked(userGlobalSocket.GetPath(), dockerSock) {
 		if checkSockInUse(dockerSock) {
-			return hostSocket.GetPath(), machine.MachineLocal
+			return hostSocket.GetPath(), machine.MachineLocal, nil
 		}
 
 		if !claimDockerSock() {
 			logrus.Warn("podman helper is installed, but was not able to claim the global docker sock")
-			return hostSocket.GetPath(), machine.MachineLocal
+			return hostSocket.GetPath(), machine.MachineLocal, nil
 		}
 	}
 
-	return dockerSock, machine.DockerGlobal
+	return dockerSock, machine.DockerGlobal, nil
 }
 
 func alreadyLinked(target string, link string) bool {

pkg/machine/shim/networking_windows.go

@@ -5,10 +5,12 @@ import (
 
 	"github.com/containers/podman/v5/pkg/machine"
 	"github.com/containers/podman/v5/pkg/machine/define"
+	"github.com/containers/podman/v5/pkg/machine/env"
+	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
 )
 
-func setupMachineSockets(name string, dirs *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) {
-	machinePipe := machine.ToDist(name)
+func setupMachineSockets(mc *vmconfigs.MachineConfig, dirs *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) {
+	machinePipe := env.WithPodmanPrefix(mc.Name)
 	if !machine.PipeNameAvailable(machinePipe, machine.MachineNameWait) {
 		return nil, "", 0, fmt.Errorf("could not start api proxy since expected pipe is not available: %s", machinePipe)
 	}

pkg/machine/vmconfigs/machine.go

@@ -15,6 +15,7 @@ import (
 	"github.com/containers/podman/v5/pkg/errorhandling"
 	"github.com/containers/podman/v5/pkg/machine/connection"
 	"github.com/containers/podman/v5/pkg/machine/define"
+	"github.com/containers/podman/v5/pkg/machine/env"
 	"github.com/containers/podman/v5/pkg/machine/lock"
 	"github.com/containers/podman/v5/pkg/machine/ports"
 	"github.com/containers/storage/pkg/ioutils"
@@ -162,6 +163,16 @@ func (mc *MachineConfig) Remove(saveIgnition, saveImage bool) ([]string, func()
 		return nil, nil, err
 	}
 
+	gvProxySocket, err := mc.GVProxySocket()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	apiSocket, err := mc.APISocket()
+	if err != nil {
+		return nil, nil, err
+	}
+
 	logPath, err := mc.LogFile()
 	if err != nil {
 		return nil, nil, err
@@ -170,6 +181,8 @@ func (mc *MachineConfig) Remove(saveIgnition, saveImage bool) ([]string, func()
 	rmFiles := []string{
 		mc.configPath.GetPath(),
 		readySocket.GetPath(),
+		gvProxySocket.GetPath(),
+		apiSocket.GetPath(),
 		logPath.GetPath(),
 	}
 	if !saveImage {
@@ -198,6 +211,12 @@ func (mc *MachineConfig) Remove(saveIgnition, saveImage bool) ([]string, func()
 		if err := readySocket.Delete(); err != nil {
 			errs = append(errs, err)
 		}
+		if err := gvProxySocket.Delete(); err != nil {
+			errs = append(errs, err)
+		}
+		if err := apiSocket.Delete(); err != nil {
+			errs = append(errs, err)
+		}
 		if err := logPath.Delete(); err != nil {
 			errs = append(errs, err)
 		}
@@ -268,6 +287,14 @@ func (mc *MachineConfig) GVProxySocket() (*define.VMFile, error) {
 	return gvProxySocket(mc.Name, machineRuntimeDir)
 }
 
+func (mc *MachineConfig) APISocket() (*define.VMFile, error) {
+	machineRuntimeDir, err := mc.RuntimeDir()
+	if err != nil {
+		return nil, err
+	}
+	return apiSocket(mc.Name, machineRuntimeDir)
+}
+
 func (mc *MachineConfig) LogFile() (*define.VMFile, error) {
 	rtDir, err := mc.RuntimeDir()
 	if err != nil {
@@ -309,26 +336,13 @@ func (mc *MachineConfig) ConnectionInfo(vmtype define.VMType) (*define.VMFile, *
 	)
 
 	if vmtype == define.HyperVVirt || vmtype == define.WSLVirt {
-		pipeName := mc.Name
-		if !strings.HasPrefix(pipeName, "podman") {
-			pipeName = "podman-" + pipeName
-		}
+		pipeName := env.WithPodmanPrefix(mc.Name)
 		pipe = &define.VMFile{Path: `\\.\pipe\` + pipeName}
-	}
-
-	if vmtype == define.WSLVirt {
 		return nil, pipe, nil
 	}
 
-	sockName := "podman.sock"
-	dataDir, err := mc.DataDir()
-	if err != nil {
-		logrus.Errorf("Resolving data dir: %s", err.Error())
-		return nil, nil, err
-	}
-
-	socket, err = define.NewMachineFile(filepath.Join(dataDir.Path, sockName), &sockName)
-	return socket, pipe, err
+	socket, err := mc.APISocket()
+	return socket, nil, err
 }
 
 // LoadMachineByName returns a machine config based on the vm name and provider

pkg/machine/vmconfigs/sockets.go

@@ -15,3 +15,7 @@ func gvProxySocket(name string, machineRuntimeDir *define.VMFile) (*define.VMFil
 func readySocket(name string, machineRuntimeDir *define.VMFile) (*define.VMFile, error) {
 	return machineRuntimeDir.AppendToNewVMFile(name+".sock", nil)
 }
+
+func apiSocket(name string, socketDir *define.VMFile) (*define.VMFile, error) {
+	return socketDir.AppendToNewVMFile(name+"-api.sock", nil)
+}

pkg/machine/vmconfigs/sockets_darwin.go

@@ -15,3 +15,8 @@ func readySocket(name string, machineRuntimeDir *define.VMFile) (*define.VMFile,
 	socketName := name + ".sock"
 	return machineRuntimeDir.AppendToNewVMFile(socketName, &socketName)
 }
+
+func apiSocket(name string, socketDir *define.VMFile) (*define.VMFile, error) {
+	socketName := name + "-api.sock"
+	return socketDir.AppendToNewVMFile(socketName, &socketName)
+}

pkg/machine/wsl/machine.go

@@ -60,7 +60,7 @@ func getConfigPathExt(name string, extension string) (string, error) {
 // TODO like provisionWSL, i think this needs to be pushed to use common
 // paths and types where possible
 func unprovisionWSL(mc *vmconfigs.MachineConfig) error {
-	dist := machine.ToDist(mc.Name)
+	dist := env.WithPodmanPrefix(mc.Name)
 	if err := terminateDist(dist); err != nil {
 		logrus.Error(err)
 	}
@@ -92,7 +92,7 @@ func provisionWSLDist(name string, imagePath string, prompt string) (string, err
 		return "", fmt.Errorf("could not create wsldist directory: %w", err)
 	}
 
-	dist := machine.ToDist(name)
+	dist := env.WithPodmanPrefix(name)
 	fmt.Println(prompt)
 	if err = runCmdPassThrough(wutil.FindWSL(), "--import", dist, distTarget, imagePath, "--version", "2"); err != nil {
 		return "", fmt.Errorf("the WSL import of guest OS failed: %w", err)
@@ -684,7 +684,7 @@ func unregisterDist(dist string) error {
 }
 
 func isRunning(name string) (bool, error) {
-	dist := machine.ToDist(name)
+	dist := env.WithPodmanPrefix(name)
 	wsl, err := isWSLRunning(dist)
 	if err != nil {
 		return false, err
@@ -719,7 +719,7 @@ func getDiskSize(name string) strongunits.GiB {
 
 //nolint:unused
 func getCPUs(name string) (uint64, error) {
-	dist := machine.ToDist(name)
+	dist := env.WithPodmanPrefix(name)
 	if run, _ := isWSLRunning(dist); !run {
 		return 0, nil
 	}
@@ -744,7 +744,7 @@ func getCPUs(name string) (uint64, error) {
 
 //nolint:unused
 func getMem(name string) (strongunits.MiB, error) {
-	dist := machine.ToDist(name)
+	dist := env.WithPodmanPrefix(name)
 	if run, _ := isWSLRunning(dist); !run {
 		return 0, nil
 	}

pkg/machine/wsl/stubber.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/containers/podman/v5/pkg/machine/env"
 	"github.com/containers/podman/v5/pkg/machine/ocipull"
 	"github.com/containers/podman/v5/pkg/machine/shim/diskpull"
 	"github.com/containers/podman/v5/pkg/machine/stdpull"
@@ -91,7 +92,7 @@ func (w WSLStubber) PrepareIgnition(_ *vmconfigs.MachineConfig, _ *ignition.Igni
 }
 
 func (w WSLStubber) Exists(name string) (bool, error) {
-	return isWSLExist(machine.ToDist(name))
+	return isWSLExist(env.WithPodmanPrefix(name))
 }
 
 func (w WSLStubber) MountType() vmconfigs.VolumeMountType {
@@ -107,7 +108,7 @@ func (w WSLStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() error,
 	// below if we wanted to hard error on the wsl unregister
 	// of the vm
 	wslRemoveFunc := func() error {
-		if err := runCmdPassThrough(wutil.FindWSL(), "--unregister", machine.ToDist(mc.Name)); err != nil {
+		if err := runCmdPassThrough(wutil.FindWSL(), "--unregister", env.WithPodmanPrefix(mc.Name)); err != nil {
 			logrus.Error(err)
 		}
 		return nil
@@ -156,7 +157,7 @@ func (w WSLStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, opts define.Se
 			return errors.New("user-mode networking can only be changed when the machine is not running")
 		}
 
-		dist := machine.ToDist(mc.Name)
+		dist := env.WithPodmanPrefix(mc.Name)
 		if err := changeDistUserModeNetworking(dist, mc.SSH.RemoteUsername, mc.ImagePath.GetPath(), *opts.UserModeNetworking); err != nil {
 			return fmt.Errorf("failure changing state of user-mode networking setting: %w", err)
 		}
@@ -202,7 +203,7 @@ func (w WSLStubber) PostStartNetworking(mc *vmconfigs.MachineConfig, noInfo bool
 }
 
 func (w WSLStubber) StartVM(mc *vmconfigs.MachineConfig) (func() error, func() error, error) {
-	dist := machine.ToDist(mc.Name)
+	dist := env.WithPodmanPrefix(mc.Name)
 
 	err := wslInvoke(dist, "/root/bootstrap")
 	if err != nil {
@@ -236,7 +237,7 @@ func (w WSLStubber) StopVM(mc *vmconfigs.MachineConfig, hardStop bool) error {
 		return err
 	}
 
-	dist := machine.ToDist(mc.Name)
+	dist := env.WithPodmanPrefix(mc.Name)
 
 	// Stop user-mode networking if enabled
 	if err := stopUserModeNetworking(mc); err != nil {
@@ -270,7 +271,7 @@ func (w WSLStubber) StopHostNetworking(mc *vmconfigs.MachineConfig, vmType defin
 }
 
 func (w WSLStubber) UpdateSSHPort(mc *vmconfigs.MachineConfig, port int) error {
-	dist := machine.ToDist(mc.Name)
+	dist := env.WithPodmanPrefix(mc.Name)
 
 	if err := wslInvoke(dist, "sh", "-c", fmt.Sprintf(changePort, port)); err != nil {
 		return fmt.Errorf("could not change SSH port for guest OS: %w", err)

pkg/machine/wsl/usermodenet.go

@@ -102,7 +102,7 @@ func startUserModeNetworking(mc *vmconfigs.MachineConfig) error {
 		}
 	}
 
-	if err := createUserModeResolvConf(machine.ToDist(mc.Name)); err != nil {
+	if err := createUserModeResolvConf(env.WithPodmanPrefix(mc.Name)); err != nil {
 		return err
 	}
 
@@ -260,7 +260,7 @@ func addUserModeNetEntry(mc *vmconfigs.MachineConfig) error {
 		return err
 	}
 
-	path := filepath.Join(entriesDir, machine.ToDist(mc.Name))
+	path := filepath.Join(entriesDir, env.WithPodmanPrefix(mc.Name))
 	file, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644)
 	if err != nil {
 		return fmt.Errorf("could not add user-mode networking registration: %w", err)
@@ -275,7 +275,7 @@ func removeUserModeNetEntry(name string) error {
 		return err
 	}
 
-	path := filepath.Join(entriesDir, machine.ToDist(name))
+	path := filepath.Join(entriesDir, env.WithPodmanPrefix(name))
 	return os.Remove(path)
 }