From 8c7504c2b649c5d800c2b373a663a949a266e2fc Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Wed, 29 Jul 2026 15:41:27 +0200 Subject: [PATCH 1/2] 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 --- README.md | 55 +++++ implementation-notes.md | 32 +++ internal/cmd/run.go | 9 + internal/server/idle_controller_test.go | 8 +- internal/server/router.go | 13 +- internal/server/router_idle.go | 128 ++++++++++++ internal/server/router_idle_test.go | 263 ++++++++++++++++++++++++ internal/server/service.go | 11 - internal/server/service_cache.go | 17 ++ internal/server/service_idle.go | 7 + 10 files changed, 529 insertions(+), 14 deletions(-) create mode 100644 implementation-notes.md create mode 100644 internal/server/router_idle.go create mode 100644 internal/server/router_idle_test.go diff --git a/README.md b/README.md index 076e022..185ee48 100644 --- a/README.md +++ b/README.md @@ -687,6 +687,61 @@ or `--tls-domains-source` — serves every hostname no other service claims, so client CA applies to all of them. +### Scale to zero when idle + +A service can stop its containers after a period without traffic and start them +again on the next request. On a host running several low-traffic apps this is +the difference between paying for all of them all the time and paying for the +ones someone is actually using — an idle Rails app holds 200–300 MB it is not +using. + +Start the proxy with a container runtime socket, then deploy with `--sleep-after`: + + kamal-proxy run --docker-socket /var/run/docker.sock + kamal-proxy deploy service1 --target web-1:3000 --host app.example.com --sleep-after 30m + +The first request after the containers stop is held while they start and pass a +health check, then forwarded — body intact, including a chunked POST. Concurrent +requests coalesce into a single start. If the containers do not come up within +`--wake-timeout` (30s by default) the request fails with 503. + +> **`--docker-socket` gives the proxy root-equivalent access to the host.** Anyone +> who can execute code in the proxy can control every container on that machine. +> It is off by default and should stay off unless a service uses `--sleep-after`. +> The lifecycle is isolated behind a small interface, so a restricted host-side +> start/stop service can replace direct socket access later. + +**What does and does not count as traffic.** Health checks never wake a sleeping +service — an uptime monitor polling `/up` would otherwise pin it awake forever — +and neither do the proxy's own TLS probes. A sleeping service answers its own +health checks with `200`, and reports `503` once a wake has actually failed, so +monitoring reflects reality rather than the last good state. + +Requests rejected by `--allow-ip`, `--basic-auth` or `--rate-limit`, and paths +answered by `--redirect`, never reach the containers and so never start them. +Neither does a response served from `--cache`: a cached service can keep serving +while its containers stay stopped. + +An open WebSocket or event stream keeps a service awake for as long as it is +open, so a long-lived connection is never cut short by an idle timeout. + +**Which container gets stopped.** By default the proxy uses the target's hostname, +which is the container id under Kamal and the container name under Compose. When +a target names something else — a Compose service alias, or an IP address — say +so explicitly: + + kamal-proxy deploy service1 --target web:3000 --host app.example.com --sleep-after 30m --sleep-container myapp-web-1 + +The deploy checks the reference against the runtime and fails immediately if it +names nothing, rather than accepting the deploy and failing at the first idle +timeout an hour later. `kamal-proxy list` shows `sleeping` or `waking` in the +state column, and the state survives a proxy restart. + +**Known limitation.** `kamal deploy` prunes stopped containers, and a sleeping +container is stopped. Kamal 2 releases before the per-role prune fix can remove +one, after which every wake fails until the service is redeployed. + + ### Minimum TLS version The HTTPS listener negotiates TLS 1.2 and above by default. To refuse TLS 1.2 as diff --git a/implementation-notes.md b/implementation-notes.md new file mode 100644 index 0000000..f92b691 --- /dev/null +++ b/implementation-notes.md @@ -0,0 +1,32 @@ +# Implementation notes — #19 completion + +## Deviations + +## Discoveries + +- `statePersister` was added in #65 (field + call site) but nothing ever set it, so + sleep/wake state was never actually persisted. A defect in already-merged code, + fixed here rather than left. + +## Judgment calls + +- `Configure` treats a changed container set as a redeploy and forces the state back + to active. On a restored service the controller is brand new, so its refs always + look changed -- which silently undid `RestoreSleeping` and brought a sleeping + service back awake. Configure now runs BEFORE the restore, not after. + +- The idle gate moved from `serviceRequestWithTarget` to `sendRequestToTarget`, i.e. + below the response cache. A cache hit never reaches the target, so it must not + spend a container start; serving cached responses while the container sleeps is + the point of running both. Still below every auth/rate-limit/redirect gate and + still above target selection, so nothing else changes. + +## Judgment calls + +- `persistState` logs its error rather than returning it. The controller has already + moved by then, so a failed write costs a wrong state on the next boot, not a broken + proxy now. +- Preflight timeout is 10s, on the RPC path where a hung socket holds the operator's + terminal. +- `describeServiceState` puts pause/stop above sleeping/waking: a human decision + outranks anything traffic-driven. diff --git a/internal/cmd/run.go b/internal/cmd/run.go index 0029e8a..fb7abc2 100644 --- a/internal/cmd/run.go +++ b/internal/cmd/run.go @@ -135,6 +135,15 @@ func (c *runCommand) run(cmd *cobra.Command, args []string) error { router.SetCacheStore(cacheStore) + // Only when the operator asked for it: reaching this socket is + // root-equivalent on the host, so an unconfigured proxy holds no such handle + // at all. Without it a deploy carrying --sleep-after is refused outright + // rather than accepted and silently never acted on. + if globalConfig.DockerSocketPath != "" { + router.SetContainerLifecycle(server.NewDockerClient(globalConfig.DockerSocketPath)) + slog.Info("Scale-to-zero enabled", "docker_socket", globalConfig.DockerSocketPath) + } + var dynamicDomains *server.DynamicDomainManager if globalConfig.ACMEEmail != "" { diff --git a/internal/server/idle_controller_test.go b/internal/server/idle_controller_test.go index d04bd30..a7b7fbb 100644 --- a/internal/server/idle_controller_test.go +++ b/internal/server/idle_controller_test.go @@ -18,8 +18,9 @@ type fakeLifecycle struct { starts atomic.Int64 stops atomic.Int64 - startFunc func(ctx context.Context, ref string) error - stopFunc func(ctx context.Context, ref string) error + startFunc func(ctx context.Context, ref string) error + stopFunc func(ctx context.Context, ref string) error + existsFunc func(ctx context.Context, ref string) error } func (f *fakeLifecycle) StartContainer(ctx context.Context, ref string) error { @@ -39,6 +40,9 @@ func (f *fakeLifecycle) StopContainer(ctx context.Context, ref string) error { } func (f *fakeLifecycle) ContainerExists(ctx context.Context, ref string) error { + if f.existsFunc != nil { + return f.existsFunc(ctx, ref) + } return nil } diff --git a/internal/server/router.go b/internal/server/router.go index 63c7dec..cee67da 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -61,6 +61,7 @@ type Router struct { dynamicDomainManager *DynamicDomainManager certRegistry *CertificateRegistry cacheStore CacheStore + lifecycle ContainerLifecycle } type ServiceDescription struct { @@ -218,6 +219,8 @@ func (r *Router) RestoreLastSavedState() error { return nil }) + r.restoreSleepingServices(services) + if r.recheckOnRestore { for _, service := range services { service.RecheckTargetHealth() @@ -320,6 +323,12 @@ func (r *Router) DeployService(name string, targetURLs, readerURLs []string, opt return err } + // Before anything is installed: a reference the runtime does not know should + // fail on the operator's terminal, not at the first idle timeout. + if err := r.validateSleepConfiguration(options, targetURLs, targetOptions); err != nil { + return err + } + options.Normalize() slog.Info("Deploying", "service", name, "targets", targetURLs, "hosts", options.Hosts, "paths", options.PathPrefixes, "tls", options.TLSEnabled) @@ -476,7 +485,7 @@ func (r *Router) ListActiveServices() ServiceDescriptionMap { Path: path, Target: target, TLS: service.options.TLSEnabled, - State: service.pauseController.GetState().String(), + State: describeServiceState(service), Hosts: service.options.Hosts, PathPrefixes: service.options.PathPrefixes, Targets: service.active.WriteTargets().Names(), @@ -572,6 +581,8 @@ func (r *Router) createOrUpdateService(name string, options ServiceOptions, targ } service.SetCacheStore(r.cacheStore) + service.statePersister = r.persistState + service.SetContainerLifecycle(r.lifecycle) return service, nil } diff --git a/internal/server/router_idle.go b/internal/server/router_idle.go new file mode 100644 index 0000000..ce022bd --- /dev/null +++ b/internal/server/router_idle.go @@ -0,0 +1,128 @@ +package server + +import ( + "context" + "errors" + "fmt" + "log/slog" + "time" +) + +// containerPreflightTimeout bounds the deploy-time check. It runs on the RPC +// path, where a hung socket would otherwise hold the operator's terminal open. +const containerPreflightTimeout = 10 * time.Second + +// SetContainerLifecycle installs the runtime that starts and stops containers for +// scale-to-zero, and hands it to the services already restored — they were +// decoded before it existed, which is exactly why UnmarshalJSON never builds a +// controller itself. +func (r *Router) SetContainerLifecycle(lifecycle ContainerLifecycle) { + r.withWriteLock(func() error { + r.lifecycle = lifecycle + + for _, service := range r.services.All() { + service.SetContainerLifecycle(lifecycle) + } + return nil + }) +} + +// validateSleepConfiguration proves, in one round trip at deploy time, that a +// container runtime is reachable and that every reference names a container it +// knows. +// +// Upstream discovered all of that at the first idle timeout instead, marked the +// service asleep regardless, and then failed every request for containers that +// were running perfectly well. +func (r *Router) validateSleepConfiguration(options ServiceOptions, targetURLs []string, targetOptions TargetOptions) error { + if options.SleepAfter <= 0 { + return nil + } + + if r.lifecycle == nil { + return ErrNoContainerLifecycle + } + + refs := options.SleepContainers + if len(refs) == 0 { + targets, err := NewTargetList(targetURLs, nil, targetOptions) + if err != nil { + return err + } + + for _, target := range targets { + ref, ok := target.ContainerRef() + if !ok { + return fmt.Errorf("%w: target %s is an address, not a container; name the container with --sleep-container", + ErrNotAContainerRef, target.Address()) + } + refs = append(refs, ref) + } + } + + ctx, cancel := context.WithTimeout(context.Background(), containerPreflightTimeout) + defer cancel() + + for _, ref := range refs { + switch err := r.lifecycle.ContainerExists(ctx, ref); { + case err == nil: + + case errors.Is(err, ErrContainerNotFound): + return fmt.Errorf("%w: no container named %q; if the target names a network alias rather than a container, set --sleep-container", + ErrNotAContainerRef, ref) + + case errors.Is(err, ErrContainerInspectForbidden): + // A hardened socket proxy commonly allows start and stop while denying + // inspect. Refusing the deploy for that would lock out exactly the + // operators doing the right thing, so this is the one preflight failure + // that is a warning. + slog.Warn("Cannot verify container for --sleep-after; the socket denies inspect", + "container", ref, "error", err) + + default: + return fmt.Errorf("cannot manage container %q for --sleep-after: %w", ref, err) + } + } + + return nil +} + +// restoreSleepingServices gives every restored service a way to persist, and puts +// the ones that were sleeping back the way sleeping left them. +func (r *Router) restoreSleepingServices(services []*Service) { + for _, service := range services { + service.statePersister = r.persistState + + if r.lifecycle != nil { + service.SetContainerLifecycle(r.lifecycle) + } + } +} + +// persistState writes the routing state after a sleep or a wake. Errors are +// logged rather than returned: the controller has already moved, and failing to +// record it costs a wrong state on the next boot, not a broken proxy now. +func (r *Router) persistState() { + if err := r.SaveState(); err != nil { + slog.Error("Failed to persist idle state", "error", err) + } +} + +// describeServiceState reports the one state an operator most needs to see. A +// paused or stopped service says so -- that is a human decision, and it outranks +// anything traffic-driven -- and otherwise a sleeping or waking service says that +// rather than "running". +func describeServiceState(service *Service) string { + pauseState := service.pauseController.GetState() + if pauseState != PauseStateRunning { + return pauseState.String() + } + + if service.idleController != nil { + if idleState := service.idleController.State(); idleState != IdleStateActive { + return idleState.String() + } + } + + return pauseState.String() +} diff --git a/internal/server/router_idle_test.go b/internal/server/router_idle_test.go new file mode 100644 index 0000000..9658996 --- /dev/null +++ b/internal/server/router_idle_test.go @@ -0,0 +1,263 @@ +package server + +import ( + "context" + "errors" + "net/http" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func testIdleRouter(t *testing.T, lifecycle ContainerLifecycle) *Router { + t.Helper() + + router := testRouter(t) + router.SetContainerLifecycle(lifecycle) + + // Stop the controllers before t.TempDir is removed: a wake persists state from + // its own goroutine, which otherwise races the cleanup and fails the test with + // "directory not empty". + t.Cleanup(func() { + for _, service := range router.services.All() { + service.Dispose() + } + }) + + return router +} + +func sleepingDeployOptions(t *testing.T) ServiceOptions { + t.Helper() + + options := defaultServiceOptions + options.Hosts = []string{"app.example.com"} + options.SleepAfter = time.Hour + // A test backend listens on 127.0.0.1, which ContainerRef rightly refuses as + // an address rather than a container. + options.SleepContainers = []string{"web-1"} + return options +} + +// Without a socket the proxy cannot ever act on --sleep-after. Accepting the +// deploy and discovering that at the first idle timeout leaves an operator +// believing a service sleeps when it never will. +func TestRouter_DeployRejectsSleepAfterWithoutAContainerRuntime(t *testing.T) { + router := testRouter(t) // no lifecycle installed + _, target := testBackend(t, "first", http.StatusOK) + + err := router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions) + + require.ErrorIs(t, err, ErrNoContainerLifecycle) +} + +// The preflight turns a wrong container reference into an error on the +// operator's terminal instead of a 503 an hour later. +func TestRouter_DeployRejectsAnUnknownContainer(t *testing.T) { + lifecycle := &fakeLifecycle{ + existsFunc: func(ctx context.Context, ref string) error { return ErrContainerNotFound }, + } + router := testIdleRouter(t, lifecycle) + _, target := testBackend(t, "first", http.StatusOK) + + err := router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions) + + require.ErrorIs(t, err, ErrNotAContainerRef) + assert.ErrorContains(t, err, "--sleep-container", + "the error has to say how to fix it") +} + +// A hardened socket proxy commonly allows start and stop while denying inspect. +// Refusing the deploy for that would lock out exactly the operators doing the +// right thing. +func TestRouter_DeployWarnsButProceedsWhenInspectIsForbidden(t *testing.T) { + lifecycle := &fakeLifecycle{ + existsFunc: func(ctx context.Context, ref string) error { return ErrContainerInspectForbidden }, + } + router := testIdleRouter(t, lifecycle) + _, target := testBackend(t, "first", http.StatusOK) + + err := router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions) + + assert.NoError(t, err) +} + +func TestRouter_DeploySucceedsWithAKnownContainer(t *testing.T) { + lifecycle := &fakeLifecycle{} + router := testIdleRouter(t, lifecycle) + _, target := testBackend(t, "first", http.StatusOK) + + require.NoError(t, router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions)) + + service := router.serviceForName("sleepy") + require.NotNil(t, service) + require.NotNil(t, service.idleController, "the deployed service gets a controller") +} + +// A deploy that does not ask for sleep must not need a socket at all. +func TestRouter_DeployWithoutSleepNeedsNoContainerRuntime(t *testing.T) { + router := testRouter(t) + _, target := testBackend(t, "first", http.StatusOK) + + require.NoError(t, router.DeployService("plain", []string{target}, defaultEmptyReaders, + defaultServiceOptions, defaultTargetOptions, defaultDeploymentOptions)) +} + +// Sleeping has to survive a proxy restart, and the restored service's pool has to +// come back suspended: restore assumes every target healthy, which for a sleeping +// service is a healthy pool pointing at a stopped container. +func TestRouter_SleepingStateSurvivesARestart(t *testing.T) { + statePath := filepath.Join(t.TempDir(), "state.json") + lifecycle := &fakeLifecycle{} + + router := NewRouter(statePath) + router.SetContainerLifecycle(lifecycle) + _, target := testBackend(t, "first", http.StatusOK) + + require.NoError(t, router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions)) + + service := router.serviceForName("sleepy") + require.NotNil(t, service.idleController) + + // Sleep it, which must persist without anyone calling SaveState. + service.idleController.RestoreSleeping() + service.persistState() + + restored := NewRouter(statePath) + require.NoError(t, restored.RestoreLastSavedState()) + restored.SetContainerLifecycle(lifecycle) + + restoredService := restored.serviceForName("sleepy") + require.NotNil(t, restoredService) + require.NotNil(t, restoredService.idleController, + "SetContainerLifecycle builds the controller a restored service could not") + assert.Equal(t, IdleStateSleeping, restoredService.idleController.State()) + assert.Empty(t, restoredService.active.HealthyTargets(), + "a restored sleeping service must not route to its stopped container") +} + +// The controller persists on the sleep and wake edges, which only works if the +// router actually handed the service a persister. +func TestRouter_SleepEdgeIsPersistedWithoutAnExplicitSave(t *testing.T) { + statePath := filepath.Join(t.TempDir(), "state.json") + router := NewRouter(statePath) + router.SetContainerLifecycle(&fakeLifecycle{}) + _, target := testBackend(t, "first", http.StatusOK) + + require.NoError(t, router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions)) + + service := router.serviceForName("sleepy") + require.NotNil(t, service.statePersister, + "without this the sleep/wake edges are never written and a restart forgets") + + service.persistState() + + reloaded := NewRouter(statePath) + require.NoError(t, reloaded.RestoreLastSavedState()) + assert.NotNil(t, reloaded.serviceForName("sleepy")) +} + +func TestRouter_ListShowsSleepingAndPrefersPaused(t *testing.T) { + lifecycle := &fakeLifecycle{} + router := testIdleRouter(t, lifecycle) + _, target := testBackend(t, "first", http.StatusOK) + + require.NoError(t, router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions)) + + service := router.serviceForName("sleepy") + + assert.Equal(t, "running", router.ListActiveServices()["sleepy"].State) + + service.idleController.RestoreSleeping() + assert.Equal(t, "sleeping", router.ListActiveServices()["sleepy"].State) + + // A pause is a human decision and outranks anything traffic-driven. + require.NoError(t, router.PauseService("sleepy", time.Second, time.Second)) + assert.Equal(t, "paused", router.ListActiveServices()["sleepy"].State) +} + +// A cache hit never reaches the target, so it must not spend a container start. +// Serving stored responses while the container stays asleep is the whole point +// of running both features on one service. +func TestService_CacheHitDoesNotWakeASleepingService(t *testing.T) { + lifecycle := &fakeLifecycle{} + + options := sleepingDeployOptions(t) + options.Cache = CacheOptions{Enabled: true} + + var reached atomic.Int64 + _, target := testBackendWithHandler(t, countingHandler(&reached, publicHandler("cached"))) + + router := testIdleRouter(t, lifecycle) + router.SetCacheStore(testMemoryStore(t)) + require.NoError(t, router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + options, defaultTargetOptions, defaultDeploymentOptions)) + + // Warm the cache while awake. + statusCode, _ := sendGETRequest(router, "http://app.example.com/") + require.Equal(t, http.StatusOK, statusCode) + require.Equal(t, int64(1), reached.Load()) + + service := router.serviceForName("sleepy") + require.NotNil(t, service.idleController) + service.idleController.RestoreSleeping() + + statusCode, body := sendGETRequest(router, "http://app.example.com/") + + assert.Equal(t, http.StatusOK, statusCode) + assert.Equal(t, "cached", body) + assert.Equal(t, int64(1), reached.Load(), "the response came from the cache") + assert.Zero(t, lifecycle.starts.Load(), "a cache hit must not wake the containers") + assert.Equal(t, IdleStateSleeping, service.idleController.State()) +} + +// The other half: a request the cache cannot serve still needs the target, so it +// has to wake. +func TestService_CacheMissWakesASleepingService(t *testing.T) { + lifecycle := &fakeLifecycle{} + + options := sleepingDeployOptions(t) + options.Cache = CacheOptions{Enabled: true} + + var reached atomic.Int64 + _, target := testBackendWithHandler(t, countingHandler(&reached, publicHandler("fresh"))) + + router := testIdleRouter(t, lifecycle) + router.SetCacheStore(testMemoryStore(t)) + require.NoError(t, router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + options, defaultTargetOptions, defaultDeploymentOptions)) + + service := router.serviceForName("sleepy") + service.idleController.RestoreSleeping() + + statusCode, _ := sendGETRequest(router, "http://app.example.com/never-warmed") + + assert.Equal(t, http.StatusOK, statusCode) + assert.Equal(t, int64(1), lifecycle.starts.Load(), "a miss needs the target, so it wakes") + assert.Equal(t, IdleStateActive, service.idleController.State()) +} + +func TestRouter_DeployReportsALifecycleFailure(t *testing.T) { + lifecycle := &fakeLifecycle{ + existsFunc: func(ctx context.Context, ref string) error { return errors.New("socket is a directory") }, + } + router := testIdleRouter(t, lifecycle) + _, target := testBackend(t, "first", http.StatusOK) + + err := router.DeployService("sleepy", []string{target}, defaultEmptyReaders, + sleepingDeployOptions(t), defaultTargetOptions, defaultDeploymentOptions) + + require.Error(t, err) + assert.ErrorContains(t, err, "socket is a directory") +} diff --git a/internal/server/service.go b/internal/server/service.go index 3316231..c7fa9a1 100644 --- a/internal/server/service.go +++ b/internal/server/service.go @@ -864,17 +864,6 @@ func (s *Service) serviceRequestWithTarget(w http.ResponseWriter, r *http.Reques return } - // After every gate above, so that no blocked, throttled, unauthenticated or - // redirected request can spend a container start. Before target selection, so - // the request body is still unread when the hold begins. - handled, endIdleRequest := s.handleIdleRequest(w, r) - if endIdleRequest != nil { - defer endIdleRequest() - } - if handled { - return - } - // Last, so that everything above -- the health check exemptions, the // redirects, the allow list -- still sees the path the client asked for. r = s.rewriteRequest(r) diff --git a/internal/server/service_cache.go b/internal/server/service_cache.go index 3899e8c..6f76ea4 100644 --- a/internal/server/service_cache.go +++ b/internal/server/service_cache.go @@ -15,6 +15,23 @@ func (s *Service) SetCacheStore(store CacheStore) { } func (s *Service) sendRequestToTarget(w http.ResponseWriter, r *http.Request) { + // The idle gate lives here rather than beside the other checks, because this + // is the first point a request is known to actually need the target. A cache + // hit never reaches it, so serving stored responses does not wake a sleeping + // container -- which is the whole reason to run both features on one service. + // + // Everything above still applies: this sits below the auth, allow-list, rate + // limit and redirect gates, so none of those can spend a container start. It + // is also still above target selection, and net/http does not read a request + // body until the handler asks, so a request held here is handed on unread. + handled, endIdleRequest := s.handleIdleRequest(w, r) + if endIdleRequest != nil { + defer endIdleRequest() + } + if handled { + return + } + sendRequest := s.startLoadBalancerRequest(w, r) if sendRequest != nil { sendRequest() diff --git a/internal/server/service_idle.go b/internal/server/service_idle.go index c24863a..d54fce2 100644 --- a/internal/server/service_idle.go +++ b/internal/server/service_idle.go @@ -78,9 +78,16 @@ func (s *Service) configureIdleController(options ServiceOptions) { Persist: s.persistState, }) + // Configure first: it treats a changed container set as a redeploy and + // forces the state back to active, which would undo the restore below. A + // brand-new controller always sees its refs as changed. + s.idleController.Configure(options.SleepAfter, options.WakeTimeout, s.containerRefs(options)) + if s.restoredIdleState == IdleStateSleeping { s.idleController.RestoreSleeping() } + + return } s.idleController.Configure(options.SleepAfter, options.WakeTimeout, s.containerRefs(options)) From 411ca263c6f7282b132ab09c14bddaf4011751e3 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Wed, 29 Jul 2026 15:42:17 +0200 Subject: [PATCH 2/2] chore: drop the deviation log, its contents are in the PR body --- implementation-notes.md | 32 -------------------------------- 1 file changed, 32 deletions(-) delete mode 100644 implementation-notes.md diff --git a/implementation-notes.md b/implementation-notes.md deleted file mode 100644 index f92b691..0000000 --- a/implementation-notes.md +++ /dev/null @@ -1,32 +0,0 @@ -# Implementation notes — #19 completion - -## Deviations - -## Discoveries - -- `statePersister` was added in #65 (field + call site) but nothing ever set it, so - sleep/wake state was never actually persisted. A defect in already-merged code, - fixed here rather than left. - -## Judgment calls - -- `Configure` treats a changed container set as a redeploy and forces the state back - to active. On a restored service the controller is brand new, so its refs always - look changed -- which silently undid `RestoreSleeping` and brought a sleeping - service back awake. Configure now runs BEFORE the restore, not after. - -- The idle gate moved from `serviceRequestWithTarget` to `sendRequestToTarget`, i.e. - below the response cache. A cache hit never reaches the target, so it must not - spend a container start; serving cached responses while the container sleeps is - the point of running both. Still below every auth/rate-limit/redirect gate and - still above target selection, so nothing else changes. - -## Judgment calls - -- `persistState` logs its error rather than returning it. The controller has already - moved by then, so a failed write costs a wrong state on the next boot, not a broken - proxy now. -- Preflight timeout is 10s, on the RPC path where a hung socket holds the operator's - terminal. -- `describeServiceState` puts pause/stop above sleeping/waking: a human decision - outranks anything traffic-driven.