Merge pull request #29765 from Xiaowen-Yang/fix-29474-orphan-proxy

machine: clean up orphaned gvproxy and win-sshproxy when starting Windows machines

pkg/machine/gvproxy.go

@@ -48,7 +48,11 @@ func CleanupGVProxy(f define.VMFile) error {
 	if err != nil {
 		return fmt.Errorf("unable to convert pid to integer: %w", err)
 	}
-	if err := waitOnProcess(proxyPid); err != nil {
+	return cleanupGVProxy(proxyPid, f)
+}
+
+func cleanupGVProxy(proxyPID int, f define.VMFile) error {
+	if err := waitOnProcess(proxyPID); err != nil {
 		return err
 	}
 	return removeGVProxyPIDFile(f)

pkg/machine/machine_windows.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
+	"math"
 	"net"
 	"os"
 	"os/exec"
@@ -102,6 +103,58 @@ func DialNamedPipe(ctx context.Context, path string) (net.Conn, error) {
 	return winio.DialPipeContext(ctx, path)
 }
 
+func cleanupStaleProxy(pipeName string, recordedPID uint32, cleanup func() error) error {
+	if err := cleanup(); err != nil {
+		return fmt.Errorf("cleaning up stale proxy process %d: %w", recordedPID, err)
+	}
+	if !PipeNameAvailable(pipeName, MachineNameWait) {
+		return fmt.Errorf("named pipe %q is still in use after cleaning up stale proxy process %d", pipeName, recordedPID)
+	}
+	return nil
+}
+
+// CleanupStaleGVProxy stops a gvproxy process left behind by an externally
+// stopped VM when its named pipe and PID file still exist.
+func CleanupStaleGVProxy(pipeName string, pidFile define.VMFile) error {
+	if PipeNameAvailable(pipeName, 0) {
+		return nil
+	}
+
+	pid, err := pidFile.ReadPIDFrom()
+	if err != nil {
+		return fmt.Errorf("reading gvproxy PID file while named pipe %q is in use: %w", pipeName, err)
+	}
+	// Accept proxy PIDs from 1 through 2^32-1 (4,294,967,295) to avoid truncation or invalidation during conversion.
+	if pid <= 0 || uint64(pid) > uint64(math.MaxUint32) {
+		return fmt.Errorf("invalid gvproxy PID %d while named pipe %q is in use", pid, pipeName)
+	}
+
+	return cleanupStaleProxy(pipeName, uint32(pid), func() error {
+		return cleanupGVProxy(pid, pidFile)
+	})
+}
+
+// CleanupStaleWinProxy stops a win-sshproxy process left behind by an
+// externally stopped WSL VM when its named pipe and PID/TID file still exist.
+func CleanupStaleWinProxy(name string, vmtype define.VMType) error {
+	pipeName := env.WithPodmanPrefix(name)
+	if PipeNameAvailable(pipeName, 0) {
+		return nil
+	}
+
+	pid, tid, tidFile, err := readWinProxyTid(name, vmtype)
+	if err != nil {
+		return fmt.Errorf("reading win-sshproxy state while named pipe %q is in use: %w", pipeName, err)
+	}
+	if pid == 0 || tid == 0 {
+		return fmt.Errorf("invalid win-sshproxy state %d:%d while named pipe %q is in use", pid, tid, pipeName)
+	}
+
+	return cleanupStaleProxy(pipeName, pid, func() error {
+		return stopWinProxy(pid, tid, tidFile)
+	})
+}
+
 func LaunchWinProxy(opts WinProxyOpts, noInfo bool) {
 	globalName, pipeName, err := launchWinProxy(opts)
 	if !noInfo {
@@ -194,7 +247,10 @@ func StopWinProxy(name string, vmtype define.VMType) error {
 	if err != nil {
 		return err
 	}
+	return stopWinProxy(pid, tid, tidFile)
+}
 
+func stopWinProxy(pid, tid uint32, tidFile string) error {
 	proc, err := os.FindProcess(int(pid))
 	if err != nil {
 		//nolint:nilerr

pkg/machine/machine_windows_test.go

@@ -3,13 +3,49 @@
 package machine
 
 import (
+	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"testing"
 
+	winio "github.com/Microsoft/go-winio"
 	"github.com/stretchr/testify/require"
+	"go.podman.io/podman/v6/pkg/machine/define"
 )
 
+func testNamedPipe(t *testing.T) (string, func() error) {
+	t.Helper()
+
+	pipeName := fmt.Sprintf("podman-machine-test-%d", os.Getpid())
+	listener, err := winio.ListenPipe(`\\.\pipe\`+pipeName, nil)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = listener.Close() })
+	return pipeName, listener.Close
+}
+
+// A stale proxy is cleaned up when its state file has already supplied the PID.
+func TestCleanupStaleProxy(t *testing.T) {
+	pipeName, closePipe := testNamedPipe(t)
+	cleaned := false
+	err := cleanupStaleProxy(pipeName, uint32(os.Getpid()), func() error {
+		cleaned = true
+		return closePipe()
+	})
+
+	require.NoError(t, err)
+	require.True(t, cleaned)
+}
+
+func TestCleanupStaleGVProxyFailsWithoutPIDFile(t *testing.T) {
+	pipeName, closePipe := testNamedPipe(t)
+	defer func() { _ = closePipe() }()
+
+	pidFile := define.VMFile{Path: filepath.Join(t.TempDir(), "missing.pid")}
+	err := CleanupStaleGVProxy(pipeName, pidFile)
+	require.ErrorContains(t, err, "reading gvproxy PID file")
+}
+
 // CreateNewItemWithPowerShell creates a new item using PowerShell.
 // It's an helper to easily create junctions on Windows (as well as other file types).
 // It constructs a PowerShell command to create a new item at the specified path with the given item type.

pkg/machine/shim/networking.go

@@ -92,6 +92,13 @@ func startHostForwarder(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvid
 }
 
 func startNetworking(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvider) (string, machine.APIForwardingState, error) {
+	// An externally stopped VM can leave its host proxy behind. On Windows,
+	// clean up a verified orphan before checking the SSH port; otherwise the
+	// orphan itself can cause an unnecessary port reassignment.
+	if err := cleanupStaleHostForwarder(mc, provider); err != nil {
+		return "", 0, err
+	}
+
 	// Check if SSH port is in use, and reassign if necessary
 	if !ports.IsLocalPortAvailable(mc.SSH.Port) {
 		logrus.Warnf("detected port conflict on machine ssh port [%d], reassigning", mc.SSH.Port)

pkg/machine/shim/networking_unix.go

@@ -17,6 +17,10 @@ import (
 
 func setGvproxyProcessAttributes(_ *exec.Cmd) {}
 
+func cleanupStaleHostForwarder(_ *vmconfigs.MachineConfig, _ vmconfigs.VMProvider) error {
+	return nil
+}
+
 func setupMachineSockets(mc *vmconfigs.MachineConfig, dirs *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) {
 	hostSocket, err := mc.APISocket()
 	if err != nil {

pkg/machine/shim/networking_windows.go

@@ -25,6 +25,33 @@ func setGvproxyProcessAttributes(c *exec.Cmd) {
 	}
 }
 
+func cleanupStaleHostForwarder(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvider) error {
+	if provider.VMType() == define.WSLVirt {
+		if err := machine.CleanupStaleWinProxy(mc.Name, provider.VMType()); err != nil {
+			return fmt.Errorf("could not recover api proxy for %s: %w", env.WithPodmanPrefix(mc.Name), err)
+		}
+		return nil
+	}
+	if provider.UseProviderNetworkSetup() {
+		return nil
+	}
+
+	dirs, err := env.GetMachineDirs(provider.VMType())
+	if err != nil {
+		return err
+	}
+	pidFile, err := dirs.RuntimeDir.AppendToNewVMFile("gvproxy.pid", nil)
+	if err != nil {
+		return err
+	}
+
+	pipeName := env.WithPodmanPrefix(mc.Name)
+	if err := machine.CleanupStaleGVProxy(pipeName, *pidFile); err != nil {
+		return fmt.Errorf("could not recover api proxy for %s: %w", pipeName, err)
+	}
+	return nil
+}
+
 func setupMachineSockets(mc *vmconfigs.MachineConfig, _ *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) {
 	machinePipe := env.WithPodmanPrefix(mc.Name)
 	if !machine.PipeNameAvailable(machinePipe, machine.MachineNameWait) {