feat(server): Docker socket client behind the ContainerLifecycle seam - #61
Merged
Merged
Conversation
The backend for scale-to-zero (#19). Three Engine API endpoints over a unix socket with no external dependency -- the official SDK would add a large dependency tree to a proxy that has none. Still nothing calls it: the idle controller is only wired up when the request gate lands. Reaching this socket is root-equivalent on the host, so the client is only constructed when the operator passes --docker-socket. Two defects were found by attacking the first draft against a live daemon, and both are fixed here rather than left for review: /stop now sends `t`. Docker's SIGTERM-to-SIGKILL wait otherwise comes from the container's own StopTimeout, which the proxy cannot see -- kamal passes `options:` straight through to docker run, and compose has stop_grace_period. Measured against a real daemon: a container created with --stop-timeout 60 made StopContainer return a deadline error at 30s while the daemon carried on and killed the container at 60s. The controller reads that error as a failed stop, rolls the service back to active and puts its targets back -- for a container Docker is about to kill. The service then serves 502s from a live-looking target until the next full idle period. Deriving `t` from the caller's remaining budget puts the daemon's deadline inside ours instead of outside it. Redirects are no longer followed. Docker's router cleans the decoded path and answers 301 to the canonical form, and Go rewrites a redirected POST as a GET. Measured: `ContainerExists("web-1/")` returned nil while `StartContainer` on the same reference returned 404, because GET /json exists and GET /start does not -- so the deploy preflight accepts precisely the reference the wake path can never start. Against a redirecting socket proxy a stop could report success having stopped nothing. Nothing in the Engine API legitimately redirects. Version negotiation caches only a success, so a daemon that was briefly unreachable does not pin the client to the fallback for the life of the process. /version is read with a 1 MB limit where errors get 4 KB: a plugin-heavy host's Components array is far larger than an error body, and truncating that JSON would silently drop a healthy daemon to the fallback version. 304 is success on both start and stop, which is what makes a coalesced wake against an already-running container work. Refs #19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Docker backend for scale-to-zero (#19), behind the
ContainerLifecycleseam that merged in #58. Three Engine API endpoints over a unix socket, no external dependency — the official SDK would add a large dependency tree to a proxy that has none.Still inert. Nothing constructs or calls this yet; the idle controller only gets wired up when the request gate lands. Proxy behavior is unchanged.
Reaching
/var/run/docker.sockis root-equivalent on the host, so the client is only built when the operator passes--docker-socket(that flag lands with the wiring).Two defects found by attacking the first draft against a live daemon
I ran an adversarial pass over the first version — three lenses (wire contract, security, concurrency), 15 raw findings, each then independently re-verified by executing it. 13 were refuted. Two survived, both reproduced against a real Docker daemon, and both are fixed here.
1.
/stopsent not, so the daemon's kill deadline sat outside oursDocker's SIGTERM→SIGKILL wait comes from the container's
StopTimeout, which the proxy cannot see — kamal passesoptions:straight through todocker run, and compose hasstop_grace_period.Measured against a live daemon with a PID-1 that ignores SIGTERM:
The controller reads that error as a failed stop, rolls the service back to
activeand callsresume()— putting the targets back for a container Docker is about to kill. The service then serves 502s from a live-looking target until the next full idle period.Fixed by deriving
tfrom the caller's remaining budget, so the daemon's deadline lands inside ours with a margin. One correction to my own first diagnosis:http.Client.Timeoutis not the binding deadline here — the controller'scontainerStopTimeoutcontext is — so sendingtis the load-bearing half, not touching the client timeout.2. Redirects were followed, and Go downgrades a redirected POST to GET
Docker's router cleans the decoded path and answers
301to the canonical form. Measured:GET /jsonexists,GET /startdoes not (POST-only route), so the preflight accepts exactly the reference the wake path can never start. Against a redirecting socket proxy — the "hardened proxy" case this client is explicitly built for — a stop could returnnilhaving stopped nothing. Fixed withCheckRedirect: http.ErrUseLastResponse; nothing in the Engine API legitimately redirects.Other behavior worth knowing
/versiongets a 1 MB read limit, errors get 4 KB. Different on purpose: a plugin-heavy host'sComponentsarray is far larger than any error body, and truncating that JSON turns a healthy daemon into a parse failure that silently drops to the fallback version.304is success on both start and stop — that is what makes a coalesced wake against an already-running container work, and what lets a proxy whose state file said "sleeping" for a container that never stopped heal itself.Test plan
18 tests over a real unix listener, no Docker required, no network:
NegotiatesAndUsesVersionedPaths— the wire contract, and that the version is negotiated oncePathEscapesTheContainerReference— asserts onRequestURI, notURL.PathTreatsNotModifiedAsSuccess,ClassifiesMissingAndForbidden(404/403 → sentinels)FallsBackWhenVersionIsUnavailable— table over denied endpoint, non-JSON body, JSON without the fieldDoesNotCacheANegotiationFailure,ReadsALargeVersionPayload,TruncatesLongErrorBodiesBoundsTheDaemonStopInsideTheCallersDeadline+StopWithoutADeadlineStillBoundsTheDaemon— finding 1DoesNotFollowRedirects— finding 2SurfacesAnUnreachableSocket,RespectsContextCancellationmake test,go vet,gofmtclean;go test -race— 1174 passDeviations & judgment calls
PathEscapesTheContainerReferenceoriginally asserted onr.URL.Path, which Go hands the handler already decoded — so it passed whether or not the code escaped anything. Switched toRequestURIand confirmed by mutation: stripurl.PathEscapeand it now fails withwe/ird%20nameinstead ofwe%2Fird%20name. That class of test looks fine in review and tests nothing.dockerStopMarginis 5s anddefaultDockerStopTimeoutis 10s — both are judgment, not derivation. The margin has to cover a socket round trip and the daemon's own bookkeeping; 5s is generous. The default matches Docker's own, so containers that never configured aStopTimeoutbehave exactly as before. Easy to flip if either proves wrong in practice.t=1, nott=0. Docker reads0as "SIGKILL immediately", which denies the app any chance to shut down cleanly. One second is a poor deadline but a better default than none.GET /version. N simultaneous wakes on a cold client means up to N negotiation requests. Judged acceptable rather than worth a singleflight: it happens once per process, the requests are cheap, and the alternative adds a second synchronisation path to a client whose only shared state is one string. Flagged rather than silently accepted — the verifier agreed it is a thundering herd, not a correctness bug.Refs #19