feat(service): hold requests for a sleeping service and wake it on demand - #65
Merged
Conversation
…mand The Service half of scale-to-zero (#19): the request gate, the options, the persisted state, and the CLI surface. The Router still does not install a container lifecycle, so nothing sleeps yet -- that is the last chunk. The flag is --sleep-after, NOT --idle-timeout. `run --idle-timeout` already exists for HTTP keep-alive (config.go, DefaultIdleTimeout = 60s), and two flags sharing one name with two unrelated meanings is an operator trap no compiler catches. --sleep-after also puts the flag, the persisted state name, and the log lines in one vocabulary. The gem needs to know this: kamal#68 assumed --idle-timeout would be the name. Gate placement is the load-bearing decision. It sits in serviceRequestWithTarget, after handlePausedAndStoppedRequests and before target selection. Every gate above it -- basic auth, IP allow list, rate limit, redirects, request deadline, the ACME HTTP handler -- therefore runs first, so a blocked, throttled, unauthenticated or redirected request can never spend a container start. Upstream basecamp#228 put this a layer up in ServeHTTP, where an anonymous client, a rate-limited flood, or an ACME HTTP-01 challenge each start containers. It is also above target selection, so the request handed on is byte-for-byte the one that arrived, body still unread. Health checks are answered, never held and never allowed to wake: an uptime monitor polling /up would pin a service awake forever, and holding the probe would make a load balancer in front evict a service that is sleeping correctly. They report unhealthy once a wake has actually failed, so monitoring stops being told everything is fine while every real request 503s. The 503 body carries none of the wake error. That error holds container references and up to four kilobytes of daemon output, and the response is reachable by anyone who can open a connection, so it is logged instead. Refs are recomputed in UpdateLoadBalancer, not just initialize. initialize runs before a load balancer exists on a first deploy, so refs were empty and a "wake" succeeded having started nothing -- caught by a test asserting the helper actually derived some. A redeploy pointing at new containers lands here too, which is exactly when the controller needs telling. Normalize only defaults a zero wake-timeout. Defaulting anything <= 0 swallowed a negative, making its validation unreachable; a negative is a typo and is now rejected rather than silently turned into 30s. UnmarshalJSON records the restored idle state but never builds a controller: the lifecycle is nil at that point, so one built there could reach StopContainer on a nil interface. SetContainerLifecycle creates it after the whole state file is decoded. A state file written before this feature has no idle_state key, parses to active, and re-marshals byte-identically since every new key is omitempty. Refs #19
Three conflicts in internal/server/service.go, all unions of independent additions from the response cache (#63) and this branch: - Normalize: so.Cache.Normalize() alongside the wake-timeout default - Validate: Cache.Validate() before validateSleep, both in the chain - Service struct: cacheStore/cacheHandler alongside the idle controller fields Known interaction, deliberately NOT changed here: the idle gate sits above s.cacheHandler, so a cache hit wakes a sleeping service. That is conservative rather than wrong -- the request succeeds either way -- but it means a service that could have stayed asleep serving cached responses does not. Moving the gate into sendRequestToTarget (below the cache, still above target selection and still with the body unread) would fix it, and that is a behavioral change that belongs in its own reviewable commit rather than buried in a merge.
7 tasks
mhenrixon
added a commit
that referenced
this pull request
Jul 29, 2026
#66) * feat(router): install the container runtime and complete scale-to-zero Closes #19. The router now installs a ContainerLifecycle, so everything the previous chunks built actually runs: a service with --sleep-after stops its containers when idle and starts them on the next request. Deploy refuses --sleep-after when no runtime is configured, and proves every container reference resolves before installing anything. A reference that names nothing now fails on the operator's terminal instead of at the first idle timeout an hour later, with an error that says to use --sleep-container. The one exception is a socket that answers but denies inspect, which is what a hardened socket proxy does -- that warns and proceeds, so the operators doing the right thing are not locked out. Two defects in already-merged code are fixed here rather than left: statePersister was added in #65 with a call site but nothing ever set it, so sleep and wake edges were never written and a restart forgot everything. The router now hands every service -- deployed or restored -- a persister. Configure treats a changed container set as a redeploy and forces the state back to active. A restored service builds a brand-new controller whose refs always look changed, so Configure silently undid RestoreSleeping and a sleeping service came back awake with a pool pointing at stopped containers. Configure now runs before the restore. The idle gate also moved below the response cache, into sendRequestToTarget. A cache hit never reaches the target, so it must not spend a container start -- serving stored responses while the containers stay stopped is the whole reason to run both features on one service. The gate is still below every auth, allow-list, rate-limit and redirect check, and still above target selection, so a held request is handed on with its body unread. Refs #19 * chore: drop the deviation log, its contents are in the PR body
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 Service half of scale-to-zero (#19): the request gate, the options, the persisted state, and the CLI surface the gem is waiting on.
One chunk remains after this — the Router installing a
ContainerLifecycle— so nothing sleeps yet.The flag is
--sleep-after, not--idle-timeoutThe gem side needs this correction. kamal#68 assumed
kamal-proxywould grow--idle-timeout. It cannot:run --idle-timeoutalready exists for HTTP keep-alive (config.go,DefaultIdleTimeout = 60s). Two flags sharing one name with two unrelated meanings is an operator trap no compiler catches.deploy--sleep-after0(never)deploy--wake-timeout30sdeploy--sleep-container(repeatable)run--docker-socket(envDOCKER_SOCKET)--sleep-afteralso puts the flag, the persisted state name (sleeping) and the log lines in one vocabulary.Gate placement is the load-bearing decision
The gate sits in
serviceRequestWithTarget, afterhandlePausedAndStoppedRequestsand before target selection.Everything above it therefore runs first — basic auth, IP allow list, rate limit, redirects, request deadline, the ACME HTTP handler. So a blocked, throttled, unauthenticated or redirected request can never spend a container start. Upstream basecamp#228 puts this a layer up in
ServeHTTP, where an anonymous client, a rate-limited flood, or an ACME HTTP-01 challenge each start containers — a denial-of-wallet vector this fork does not inherit.Being above target selection is what keeps the request byte-for-byte intact:
net/httpdoes not read a body until the handler asks, so a chunked POST parked for three seconds is handed on unread. No buffering, noTeeReader.Health checks: answered, never held, never waking
An uptime monitor polling
/upwould otherwise pin a service awake forever. Holding the probe instead would make a load balancer in front evict a service that is sleeping exactly as intended. So a sleeping service answers200itself.The other half matters as much: once a wake has actually failed, the health check reports unhealthy. Otherwise a service that can no longer start reports green to its monitoring forever while 503ing every real request.
The 503 leaks nothing
The wake error carries container references and up to 4 KB of daemon output, and that response is reachable by anyone who can open a connection. It is logged, not rendered — with a test asserting neither the container name nor
docker.sockappears in the body.Test plan
ValidateSleep— table: negatives,--sleep-containerwithout--sleep-after, and the TLS on-demand combination (a sleeping backend cannot answer an on-demand check, and waking one would let any SNI on the internet start a container)NormalizeDefaultsTheWakeTimeoutSleepingServiceWakesOnARequest— one start, state returns to activeHealthCheckNeitherWakesNorIsHeld+HealthCheckReportsUnhealthyAfterAFailedWakeWakeFailureDoesNotLeakDaemonDetailToTheClientInternalRequestsDoNotWake— the proxy's own TLS probeContainerRefsPreferTheExplicitOverride,TestTarget_ContainerRef(table; IP literals rejected)IdleStateRoundTrip+StateFileWithoutIdleStateRestoresAwakegofmt,go vetclean;go test -race— 1301 passDeviations & judgment calls
UpdateLoadBalancer, not onlyinitialize.initializeruns before a load balancer exists on a first deploy, so refs were empty and a "wake" reported success having started nothing. I caught it because a test asserted the helper had actually derived some refs — worth noting, since the symptom is a silent success, not a failure. A redeploy pointing at new containers lands here too, which is exactly when the controller needs telling.Normalizeonly defaults a zero wake-timeout. It first defaulted anything<= 0, which swallowed a negative and made its own validation unreachable. A negative is a typo; rejecting it beats silently turning it into 30s.UnmarshalJSONdeliberately builds no controller. The lifecycle is nil at that point, so one built there could reachStopContaineron a nil interface.SetContainerLifecyclecreates it after the whole state file is decoded. Add opt-in scale-to-zero for idle services basecamp/kamal-proxy#228 built it in unmarshal and had exactly that exposure.--sleep-containerexplicitly.httptestbackends listen on127.0.0.1, whichContainerRefcorrectly refuses as an address rather than a container. That is the override's real purpose, so the tests exercise the real path rather than a fixture.MarshalJSONstill assumess.activeis non-nil. I hit the nil deref writing a test that built aServicewith no load balancer, and fixed the test rather than adding a defensive check — the router always installs one before state is saved, and the coding rules say not to nil-check states that cannot happen. Flagging it because I did consider the guard.--sleep-containeronly. The selector — which also closes thekamal rollbackhole kamal#68 names — is the immediate follow-up, additive: oneDockerClientmethod and one flag.Refs #19