Resurrect auto-port reassignment, but for all providers

- Updates common to pull in new locked edit

[NO NEW TESTS NEEDED]

Signed-off-by: Jason T. Greene <[email protected]>

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/checkpoint-restore/go-criu/v7 v7.0.0
 	github.com/containernetworking/plugins v1.4.0
 	github.com/containers/buildah v1.34.1-0.20240229193131-f5d7689ef4cd
-	github.com/containers/common v0.57.1-0.20240229165734-cec09922602e
+	github.com/containers/common v0.57.1-0.20240304165751-a0d555c70d52
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/gvisor-tap-vsock v0.7.3
 	github.com/containers/image/v5 v5.29.3-0.20240229213915-cdc68020a24f

go.sum

@@ -76,8 +76,8 @@ github.com/containernetworking/plugins v1.4.0 h1:+w22VPYgk7nQHw7KT92lsRmuToHvb7w
 github.com/containernetworking/plugins v1.4.0/go.mod h1:UYhcOyjefnrQvKvmmyEKsUA+M9Nfn7tqULPpH0Pkcj0=
 github.com/containers/buildah v1.34.1-0.20240229193131-f5d7689ef4cd h1:4cHNzaywyyJsCAtwUKMZm8r/wqm/WuNC70GfnI3kh18=
 github.com/containers/buildah v1.34.1-0.20240229193131-f5d7689ef4cd/go.mod h1:3fn5edBIPpIOPJYdnxBdTF7bjnBHhqZwYK5a6ApNdyk=
-github.com/containers/common v0.57.1-0.20240229165734-cec09922602e h1:TPgCd6bWFyliJxCXEiCI1LnbB3kBUkpx1dw51ngDjWI=
-github.com/containers/common v0.57.1-0.20240229165734-cec09922602e/go.mod h1:8irlyBcVooYx0F+YmoY7PQPAIgdJvCj17bvL7PqeaxI=
+github.com/containers/common v0.57.1-0.20240304165751-a0d555c70d52 h1:+rq1qOOEv/2Sa1A9Tmv7yKuOzea8W2n6kFUH+bon61Y=
+github.com/containers/common v0.57.1-0.20240304165751-a0d555c70d52/go.mod h1:h92alKzSekxVC+VDaX4gt7RJpXvJKF79a9TnULZ5ZDc=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/gvisor-tap-vsock v0.7.3 h1:yORnf15sP+sLFhxLNLgmB5/lOhldn9dRMHx/tmYtSOQ=

pkg/machine/applehv/stubber.go

@@ -313,6 +313,11 @@ func (a AppleHVStubber) StopHostNetworking(_ *vmconfigs.MachineConfig, _ define.
 	return nil
 }
 
+func (a AppleHVStubber) UpdateSSHPort(mc *vmconfigs.MachineConfig, port int) error {
+	// managed by gvproxy on this backend, so nothing to do
+	return nil
+}
+
 func (a AppleHVStubber) VMType() define.VMType {
 	return define.AppleHvVirt
 }

pkg/machine/connection/add.go

@@ -1,3 +1,5 @@
+//go:build amd64 || arm64
+
 package connection
 
 import (
@@ -14,10 +16,23 @@ func AddSSHConnectionsToPodmanSocket(uid, port int, identityPath, name, remoteUs
 		fmt.Println("An ignition path was provided.  No SSH connection was added to Podman")
 		return nil
 	}
+
+	cons := createConnections(name, uid, port, remoteUsername)
+
+	// The first connection defined when connections is empty will become the default
+	// regardless of IsDefault, so order according to rootful
+	if opts.Rootful {
+		cons[0], cons[1] = cons[1], cons[0]
+	}
+
+	return addConnection(cons, identityPath, opts.IsDefault)
+}
+
+func createConnections(name string, uid, port int, remoteUsername string) []connection {
 	uri := makeSSHURL(LocalhostIP, fmt.Sprintf("/run/user/%d/podman/podman.sock", uid), strconv.Itoa(port), remoteUsername)
 	uriRoot := makeSSHURL(LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(port), "root")
 
-	cons := []connection{
+	return []connection{
 		{
 			name: name,
 			uri:  uri,
@@ -27,12 +42,4 @@ func AddSSHConnectionsToPodmanSocket(uid, port int, identityPath, name, remoteUs
 			uri:  uriRoot,
 		},
 	}
-
-	// The first connection defined when connections is empty will become the default
-	// regardless of IsDefault, so order according to rootful
-	if opts.Rootful {
-		cons[0], cons[1] = cons[1], cons[0]
-	}
-
-	return addConnection(cons, identityPath, opts.IsDefault)
 }

pkg/machine/connection/connection.go

@@ -55,14 +55,17 @@ func addConnection(cons []connection, identity string, isDefault bool) error {
 	})
 }
 
-func ChangeConnectionURI(name string, uri fmt.Stringer) error {
+func UpdateConnectionPairPort(name string, port, uid int, remoteUsername string, identityPath string) error {
+	cons := createConnections(name, uid, port, remoteUsername)
 	return config.EditConnectionConfig(func(cfg *config.ConnectionsFile) error {
-		dst, ok := cfg.Connection.Connections[name]
-		if !ok {
-			return errors.New("connection not found")
+		for _, con := range cons {
+			dst := config.Destination{
+				IsMachine: true,
+				URI:       con.uri.String(),
+				Identity:  identityPath,
+			}
+			cfg.Connection.Connections[name] = dst
 		}
-		dst.URI = uri.String()
-		cfg.Connection.Connections[name] = dst
 
 		return nil
 	})

pkg/machine/hyperv/stubber.go

@@ -446,6 +446,11 @@ func (h HyperVStubber) PostStartNetworking(mc *vmconfigs.MachineConfig, noInfo b
 	return err
 }
 
+func (h HyperVStubber) UpdateSSHPort(mc *vmconfigs.MachineConfig, port int) error {
+	// managed by gvproxy on this backend, so nothing to do
+	return nil
+}
+
 func (h HyperVStubber) GetDisk(userInputPath string, dirs *define.MachineDirs, mc *vmconfigs.MachineConfig) error {
 	return diskpull.GetDisk(userInputPath, dirs, mc.ImagePath, h.VMType(), mc.Name)
 }

pkg/machine/ignition/ready.go

@@ -1,3 +1,5 @@
+//go:build amd64 || arm64
+
 package ignition
 
 import (

pkg/machine/qemu/machine.go

@@ -1,4 +1,4 @@
-//go:build !darwin
+//go:build linux || freebsd
 
 package qemu
 

pkg/machine/qemu/options_windows_amd64.go

@@ -1,4 +1,4 @@
-//go:build windows && amd64
+//go:build tempoff
 
 package qemu
 

pkg/machine/qemu/stubber.go

@@ -1,4 +1,4 @@
-//go:build !darwin
+//go:build linux || freebsd
 
 package qemu
 
@@ -352,6 +352,11 @@ func (q *QEMUStubber) PostStartNetworking(mc *vmconfigs.MachineConfig, noInfo bo
 	return nil
 }
 
+func (q *QEMUStubber) UpdateSSHPort(mc *vmconfigs.MachineConfig, port int) error {
+	// managed by gvproxy on this backend, so nothing to do
+	return nil
+}
+
 func (q *QEMUStubber) GetDisk(userInputPath string, dirs *define.MachineDirs, mc *vmconfigs.MachineConfig) error {
 	return diskpull.GetDisk(userInputPath, dirs, mc.ImagePath, q.VMType(), mc.Name)
 }

pkg/machine/shim/networking.go

@@ -11,8 +11,10 @@ import (
 	"github.com/containers/common/pkg/config"
 	gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
 	"github.com/containers/podman/v5/pkg/machine"
+	"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/ports"
 	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
 	"github.com/sirupsen/logrus"
 )
@@ -91,6 +93,14 @@ func startHostForwarder(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvid
 }
 
 func startNetworking(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvider) (string, machine.APIForwardingState, error) {
+	// 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)
+		if err := reassignSSHPort(mc, provider); err != nil {
+			return "", 0, err
+		}
+	}
+
 	// Provider has its own networking code path (e.g. WSL)
 	if provider.UseProviderNetworkSetup() {
 		return "", 0, provider.StartNetworking(mc, nil)
@@ -153,6 +163,53 @@ func conductVMReadinessCheck(mc *vmconfigs.MachineConfig, maxBackoffs int, backo
 	return
 }
 
+func reassignSSHPort(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvider) error {
+	newPort, err := ports.AllocateMachinePort()
+	if err != nil {
+		return err
+	}
+
+	success := false
+	defer func() {
+		if !success {
+			if err := ports.ReleaseMachinePort(newPort); err != nil {
+				logrus.Warnf("could not release port allocation as part of failure rollback (%d): %s", newPort, err.Error())
+			}
+		}
+	}()
+
+	// Write a transient invalid port, to force a retry on failure
+	oldPort := mc.SSH.Port
+	mc.SSH.Port = 0
+	if err := mc.Write(); err != nil {
+		return err
+	}
+
+	if err := ports.ReleaseMachinePort(oldPort); err != nil {
+		logrus.Warnf("could not release current ssh port allocation (%d): %s", oldPort, err.Error())
+	}
+
+	// Update the backend's settings if relevant (e.g. WSL)
+	if err := provider.UpdateSSHPort(mc, newPort); err != nil {
+		return err
+	}
+
+	mc.SSH.Port = newPort
+	if err := connection.UpdateConnectionPairPort(mc.Name, newPort, mc.HostUser.UID, mc.SSH.RemoteUsername, mc.SSH.IdentityPath); err != nil {
+		return fmt.Errorf("could not update remote connection configuration: %w", err)
+	}
+
+	// Write updated port back
+	if err := mc.Write(); err != nil {
+		return err
+	}
+
+	// inform defer routine not to release the port
+	success = true
+
+	return nil
+}
+
 func isListening(port int) bool {
 	// Check if we can dial it
 	conn, err := net.DialTimeout("tcp", fmt.Sprintf("%s:%d", "127.0.0.1", port), 10*time.Millisecond)

pkg/machine/vmconfigs/config.go

@@ -96,6 +96,7 @@ type VMProvider interface { //nolint:interfacebloat
 	UserModeNetworkEnabled(mc *MachineConfig) bool
 	UseProviderNetworkSetup() bool
 	RequireExclusiveActive() bool
+	UpdateSSHPort(mc *MachineConfig, port int) error
 }
 
 // HostUser describes the host user

pkg/machine/vmconfigs/machine.go

@@ -15,7 +15,7 @@ import (
 	"github.com/containers/podman/v5/pkg/machine/connection"
 	"github.com/containers/podman/v5/pkg/machine/define"
 	"github.com/containers/podman/v5/pkg/machine/lock"
-	"github.com/containers/podman/v5/utils"
+	"github.com/containers/podman/v5/pkg/machine/ports"
 	"github.com/containers/storage/pkg/ioutils"
 	"github.com/sirupsen/logrus"
 )
@@ -78,8 +78,7 @@ func NewMachineConfig(opts define.InitOptions, dirs *define.MachineDirs, sshIden
 	}
 	mc.Resources = mrc
 
-	// TODO WSL had a locking port mechanism, we should consider this.
-	sshPort, err := utils.GetRandomPort()
+	sshPort, err := ports.AllocateMachinePort()
 	if err != nil {
 		return nil, err
 	}
@@ -204,6 +203,11 @@ func (mc *MachineConfig) Remove(saveIgnition, saveImage bool) ([]string, func()
 		if err := mc.configPath.Delete(); err != nil {
 			errs = append(errs, err)
 		}
+
+		if err := ports.ReleaseMachinePort(mc.SSH.Port); err != nil {
+			errs = append(errs, err)
+		}
+
 		return errorhandling.JoinErrors(errs)
 	}
 

pkg/machine/wsl/stubber.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"github.com/containers/podman/v5/pkg/machine/ocipull"
-	"github.com/containers/podman/v5/pkg/machine/ports"
 	"github.com/containers/podman/v5/pkg/machine/shim/diskpull"
 	"github.com/containers/podman/v5/pkg/machine/stdpull"
 	"github.com/containers/podman/v5/pkg/machine/wsl/wutil"
@@ -111,7 +110,7 @@ func (w WSLStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() error,
 		if err := runCmdPassThrough(wutil.FindWSL(), "--unregister", machine.ToDist(mc.Name)); err != nil {
 			logrus.Error(err)
 		}
-		return ports.ReleaseMachinePort(mc.SSH.Port)
+		return nil
 	}
 
 	return []string{}, wslRemoveFunc, nil
@@ -205,15 +204,6 @@ 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)
 
-	// TODO The original code checked to see if the SSH port was actually open and re-assigned if it was
-	// we could consider this but it should be higher up the stack
-	// if !machine.IsLocalPortAvailable(v.Port) {
-	// logrus.Warnf("SSH port conflict detected, reassigning a new port")
-	//	if err := v.reassignSshPort(); err != nil {
-	//		return err
-	//	}
-	// }
-
 	err := wslInvoke(dist, "/root/bootstrap")
 	if err != nil {
 		err = fmt.Errorf("the WSL bootstrap script failed: %w", err)
@@ -279,6 +269,16 @@ func (w WSLStubber) StopHostNetworking(mc *vmconfigs.MachineConfig, vmType defin
 	return stopUserModeNetworking(mc)
 }
 
+func (w WSLStubber) UpdateSSHPort(mc *vmconfigs.MachineConfig, port int) error {
+	dist := machine.ToDist(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)
+	}
+
+	return nil
+}
+
 func (w WSLStubber) VMType() define.VMType {
 	return define.WSLVirt
 }