fix: close docker connection on connectDocker error paths
What this fixes
connectDocker builds a dockerConnection and then runs four checks against it (Info, ServerVersion, version parse, validateOSType) before assigning it to e.dockerConn. If any check fails, the function returns without assigning and without closing the connection.
For the docker-autoscaler and instance executors that connection wraps a live SSH tunnel to the instance. Leaving it unclosed strands the tunnel's goroutines, bufio read/write buffers, and cipher state. Cleanup() can't recover them: it closes e.dockerConn, which on this path still holds the previous (nil) connection. Preparation retries dial a fresh tunnel each attempt, so a single flaky instance leaks several connections, and they stay resident for the life of the manager process.
The fix closes the connection on each error path before returning.
Why it matters
On a busy manager this turns an intermittent, recoverable instance problem into permanent memory loss. We reproduced the failure path in a local docker-autoscaler + fleeting rig (SSH reachable, docker daemon down): about 1,446 complete leaked SSH clients after roughly 480 failed jobs, six goroutines each, which exhausted an 8 GB test VM in 17 minutes at a 100% failure rate. On a real 2 GB manager host a lower failure rate produces the same end state more slowly: the host runs out of memory and wedges.
Leaked goroutine breakdown (from the repro, quiesced)
Each orphaned connection kept six goroutines alive, none of which drained after the jobs finished:
1446 ssh.(*handshakeTransport).readPacket <- mux.onePacket <- mux.loop
1446 ssh.(*handshakeTransport).kexLoop
1446 ssh.(*Client).handleChannelOpens
1446 poll.(*FD).Read (connection reader)
1446 sync.(*Cond).Wait <- mux.Wait <- ssh.NewClient.func1
1446 time.Sleep <- connector.(*sshClient).keepalive (fleeting connector)The connector.(*sshClient).keepalive frame confirms these are fleeting connector dials, i.e. the dockerConnection's tunnel client.
Scope
This is one of several defects found while investigating a manager memory leak on GitLab Dedicated hosted runners. It's the one with a clean, self-contained fix. It fires on connectDocker-phase preparation failures. Deployments with intermittent instance boot or daemon-startup problems hit it hardest.
Backports
Please consider backporting to the active stable branches (the leak is present at least on 19.0.x and 19.1.x).
Testing
go build and go vet pass. The change is limited to error-return paths in one function. A regression test that injects an Info/ServerVersion failure and asserts the connection is closed would be a good follow-up; happy to add one if you'd like it in this MR.