Reuse SSH connections while a machine is in the rescue system

[guettli]

Plan: Reuse SSH connections while a machine is in the rescue system

Problem

sshClient (pkg/services/baremetal/client/ssh/ssh_client.go) opens a brand new
TCP connection and does a full SSH handshake for every single remote command,
then closes the connection again (getSSHClient at ssh_client.go:615, called
from runSSH at ssh_client.go:660, ExecutePreProvisionCommand at
ssh_client.go:767, StartImageURLCommand at ssh_client.go:811, and
ReadOutputJSON at ssh_client.go:942). There is no reuse at all, not even
within a single Go call.

This is expensive precisely in the phase where it happens most often: while a
bare-metal or HCloud machine is booted into the rescue system during
provisioning.

Concrete multiplication:

• actionRegistering (pkg/services/baremetal/host/host.go:641) calls
  GetHostName, then GetHardwareDetailsDebug, then (once)
  getHardwareDetails (host.go:825), which fans out into obtainHardwareDetailsRAM,
  obtainHardwareDetailsNics, obtainHardwareDetailsStorage, and
  obtainHardwareDetailsCPU (5 more SSH calls: Arch, Model, ClockGigahertz,
  Threads, Flags). That's up to 11 sequential TCP+SSH handshakes in one
  Reconcile() call.
• The state machine polls the rescue system roughly every 10s while a host is
  in StateRegistering, StatePreProvisioning, or StateImageInstalling
  (actionContinue{delay: 10 * time.Second}, e.g. host.go:684, host.go:1210,
  host.go:1258, host.go:1286, host.go:1336, host.go:1362, host.go:1372,
  host.go:1407, host.go:1782). The same pattern exists for HCloud machines
  booted into rescue for the imageURL flow
  (pkg/services/hcloud/server/server.go, RequeueAfter: 10 * time.Second
  around lines 1105-1311, using the same sshclient package via
  getSSHClient at server.go:2432).
• Every one of those polls re-does a TCP dial + SSH handshake (asymmetric
  crypto, several round trips) just to run one trivial command such as
  hostname or a ps aux | grep installimage.

With many machines provisioning concurrently, this adds up to a lot of wasted
CPU/network and unnecessary load on the rescue system's sshd (which is a
shared resource we don't control, on Hetzner's side).

Goal

Keep one already-established SSH connection alive and reuse it for repeated
commands to the same machine as long as it is in the rescue system, instead
of reconnecting for every command and every poll.

Evaluation: keep the connection alive after the rescue phase too?

No — not useful. Reasoning, based on what actually happens after a host
leaves the rescue-related states:

• actionProvisioned (host.go:2276), the steady-state handler once a host
  is StateProvisioned, does not use SSH at all in the common case (no
  reboot annotation). It talks to the workload cluster's Kubernetes API
  (WorkloadClusterClientFactory) to read the node's BootID instead. SSH
  only gets used there for a single one-off sshClient.Reboot(ctx) call
  (host.go:2454) when a user explicitly sets a reboot annotation — a rare,
  isolated event, not a tight poll loop.
• actionDeprovisioning (host.go:2588) opens SSH exactly once, to run
  ResetKubeadm (host.go:2648), then never again for that host.
• actionEnsureProvisioned (host.go:1993) does use SSH in a short poll
  loop (checkCloudInitStatus, 5s delay) right after leaving the rescue
  system, but this is bounded to the time cloud-init needs to finish, not an
  indefinite steady state.

So outside the rescue window, SSH usage is rare and one-shot, not a hot loop —
there's nothing to amortize a persistent connection over. Keeping SSH
connections open for machines that are already fully provisioned would mean:

• Holding open root-privileged SSH sessions to potentially hundreds of nodes
  indefinitely, for a benefit that saves, at most, a handshake before an
  occasional single command.
• Connections silently going stale (node reboots, firewall/security-group
  changes made intentionally after provisioning, IP changes) without ever
  being exercised, so failures would only surface much later, at the moment
  they're finally needed (e.g. a user-triggered reboot).
• Extra long-lived goroutines/file descriptors per node in the controller
  process for no measurable win.

Conclusion: scope the connection cache tightly to the rescue-related
states, and tear it down aggressively once a host leaves them (see "Cache
lifecycle" below). Do not attempt to keep a connection alive across the whole
machine lifecycle.

Design

Where

Everything lives inside pkg/services/baremetal/client/ssh/ssh_client.go. This
package is shared by both entry points that talk to a rescue system:

• pkg/services/baremetal/host/host.go (bare-metal Robot rescue system).
• pkg/services/hcloud/server/server.go (getSSHClient, server.go:2432 — the
  HCloud imageURL-via-rescue flow).

Both go through sshclient.Factory / sshclient.Client
(ssh_client.go:164 and ssh_client.go:230). If the pooling lives behind
those two existing interfaces, no caller needs to change, and the
generated mocks (pkg/services/baremetal/client/mocks/ssh/{Client,Factory}.go)
keep working unmodified since the exported interface shape doesn't change.

Connection pool in sshFactory

• Add a pool to sshFactory (ssh_client.go:234): a mutex-protected
  map[connKey]*pooledConn, where
  connKey = ip + ":" + port + ":" + sha256(privateKey).
  Hashing the private key into the cache key means a rotated SSH secret
  (rescue key changed, or the same IP now needs the OS key after
  installimage) naturally lands on a different cache entry instead of
  accidentally reusing a connection authenticated with the wrong key; the old
  entry just becomes idle and gets evicted (see below).
• pooledConn wraps *ssh.Client plus bookkeeping: lastUsed time.Time, and
  its own sync.Mutex to guard get-or-create/evict races for that entry.
• sshClient.getSSHClient(ctx) (ssh_client.go:615) changes from
  "always dial and handshake" to "get-or-create from the factory's pool":
  1. Look up connKey in the pool.
  2. If found, do a cheap liveness probe (SendRequest("keepalive@openssh.com", true, nil) with a short timeout). If it succeeds, reuse it.
  3. If not found, or the probe fails (evict the dead entry first), dial +
     handshake as today and store the result in the pool.
• runSSH (ssh_client.go:660) and other methods must stop unconditionally closing the client at the end
  (their current defer client.Close()). Only close/evict on a genuine
  transport-level failure (session creation error, Run returning something
  other than a remote exit status, e.g. io.EOF/network errors) — never on a
  successful command that merely exited non-zero.
• Reconnect-on-error: if a cached connection turns out to be dead when
  actually used (not just at the liveness-probe step), evict it and retry the
  operation once with a freshly dialed connection. This must only apply to
  transport errors, never to remote command failures.
• Context handling changes: today context.AfterFunc(ctx, func() { conn.Close() }) (ssh_client.go:647 and ssh_client.go:675) closes the whole connection
  when the caller's context is done. With a shared connection that must not
  happen — killing one caller's context would break every other user of the
  pooled connection. Cancellation must instead close only the session
  (sess.Close()), leaving the underlying *ssh.Client/TCP connection alone.
  ssh.Client.NewSession() is safe to call concurrently on the same
  *ssh.Client, so sharing it across sequential (and, if it ever happens,
  concurrent) calls for the same machine is safe.

Cache lifecycle (tightly scoped to the rescue window)

Two independent, cheap mechanisms, matching the "not useful afterwards"
conclusion above:

1. Idle timeout — a background sweep (e.g. every 30s) closes and evicts
   any pooled entry unused for longer than ~2 minutes. This is comfortably
   above the ~10s poll interval used during rescue, but short enough that an
   abandoned/failed/deleted host's connection doesn't linger.

2. Explicit eviction on state exit — call a new
   sshclient.Factory.Evict(ip string) (or similar) when the state machine
   leaves the rescue-related states:

   • bare metal: on the StateImageInstalling → StateEnsureProvisioned
     transition in handleImageInstalling (pkg/services/baremetal/host/state_machine.go:318),
     and in checkInitiateDelete/handleDeprovisioning
     (state_machine.go:119, state_machine.go:362) for early aborts.
   • HCloud: once the imageURL command reports
     ImageURLCommandStateFinishedSuccessfully or
     ImageURLCommandStateFailed in pkg/services/hcloud/server/server.go
     (around lines 1133 and 1171).

   This bounds the cache to the exact window that benefits, rather than
   relying solely on the idle timer.

Concurrency

Each host/machine has one IP, and controller-runtime reconciles a given
object serially by default, so true concurrent use of one pooled connection
is unlikely — but two different bare-metal hosts or HCloud machines
reconciling in parallel (MaxConcurrentReconciles) must not collide on the
pool map itself. A single mutex around map access (or sync.Map) plus the
per-entry mutex described above is sufficient; no per-entry command queueing
is needed since NewSession() is inherently safe for concurrent use.

Files / lines to change

File	What
pkg/services/baremetal/client/ssh/ssh_client.go	Core change. Add pool type + map to sshFactory (line 234); rewrite getSSHClient (line 615) to get-or-create from the pool with a liveness probe; stop unconditional client.Close() in runSSH (line 660-713), ExecutePreProvisionCommand (line 767-809), StartImageURLCommand (line 811-889), ReadOutputJSON (line 942-982); fix the context.AfterFunc calls at line 647 and line 675 to close only the session, not the shared client; add Evict/idle-sweep logic and a way to stop the sweep goroutine on shutdown.
pkg/services/baremetal/host/state_machine.go	Call the new eviction hook on transitions out of the rescue-related states: handleImageInstalling (line 318), checkInitiateDelete (line 119), handleDeprovisioning (line 362).
pkg/services/hcloud/server/server.go	Call the eviction hook once the imageURL-in-rescue flow finishes or fails (near lines 1133 and 1171); no other change needed since it already goes through sshclient.Factory/getSSHClient (line 2432).
pkg/services/baremetal/client/ssh/ssh_client_test.go	New tests for the pool: reuse across calls, reconnect after the remote end closes the connection, eviction after idle timeout, no accidental reuse across a private-key rotation. Needs a small in-process fake SSH server (golang.org/x/crypto/ssh server side, already vendored) rather than a real host, following the pattern already used by the disabled Test_ExecutePreProvisionCommand_withRealServer (ssh_client_test.go:109), but actually enabled.
main.go	Only if the factory needs an explicit shutdown call (SSHClientFactory.Stop()) to stop the idle-sweep goroutine cleanly on manager shutdown; otherwise no change — process exit already closes all sockets.
pkg/services/baremetal/client/mocks/ssh/{Client,Factory}.go, pkg/services/baremetal/client/mocks/factory.go	No change expected — sshclient.Client/sshclient.Factory interfaces stay the same, so generated mocks and every existing caller (host.go, server.go) keep compiling untouched.

Non-goals

• No change to SSH usage after a host reaches StateProvisioned /
  StateDeprovisioning (see evaluation above) beyond the passive idle-timeout
  eviction that already applies everywhere.
• No change to the sshclient.Client/sshclient.Factory public interfaces.
• No change to command semantics (exit codes, stdout/stderr handling) — only
  the underlying transport is reused.

Testing plan

• Unit tests in ssh_client_test.go against a local fake SSH server:
  • Two sequential runSSH calls with the same Input reuse one
    *ssh.Client (assert via a counter of accepted TCP connections on the
    fake server).
  • After the fake server closes the connection, the next call transparently
    reconnects instead of failing.
  • After the idle timeout elapses, the pooled connection is closed and a new
    dial happens on the next call.
  • Two Inputs with the same ip:port but different private keys never
    share a pooled entry.
• Existing pkg/services/baremetal/host/*_test.go and
  pkg/services/hcloud/server/*_test.go suites (which use the generated
  mocks, not the real sshClient) should pass unmodified — they exercise
  behavior, not transport, so no update should be needed there.
• Manual/E2E: run a bare-metal or HCloud e2e provisioning test
  (test/e2e) and confirm in logs/packet capture that only one SSH connection
  per host is opened during the rescue phase instead of one per command.

Risks

• A shared connection that goes half-broken (TCP still up, sshd wedged) could
  make failures show up as timeouts on a reused connection rather than a
  clean "connection refused" on a fresh dial — mitigated by the liveness
  probe plus retry-once-on-transport-error logic.
• Bugs in eviction bookkeeping could leak goroutines/connections — mitigated
  by keeping the idle-timeout sweep as a backstop independent of the explicit
  eviction hooks.