diff --git a/cmd/atenet/README.md b/cmd/atenet/README.md index 13738e4df5..e60aa669de 100644 --- a/cmd/atenet/README.md +++ b/cmd/atenet/README.md @@ -23,6 +23,14 @@ likely be split in the future for better scalability.) * Upstream: Envoy's `ORIGINAL_DST` actor cluster dials the actor's in-worker `atunnel` ingress server on the worker pod's port 443 over mTLS, using the address `atenet router`'s ext_proc resolved into `x-ate-original-dst`. +* Termination: the router drains gracefully on SIGTERM (readiness flip → + endpoint propagation → Envoy admin-API drain → ext_proc drain), and the + Envoy container's `preStop` hook waits for the router's drain-complete + marker on a pod-shared emptyDir — so established connections and parked + requests finish instead of resetting. The whole sequence must fit within + `terminationGracePeriodSeconds` (see the manifest comments). Upgrades are + whole-system swaps (#473) rather than per-Deployment rolling updates; the + drain is what makes the old system's termination lossless. RBAC permissions: * read, list on ActorTemplate diff --git a/cmd/atenet/internal/router/README.md b/cmd/atenet/internal/router/README.md index eebdefc0c2..8045f4f237 100644 --- a/cmd/atenet/internal/router/README.md +++ b/cmd/atenet/internal/router/README.md @@ -19,6 +19,13 @@ Router has several responsibilities: worker-pool saturation, retrying the resume until the actor is routable or a bounded wait elapses, instead of failing fast. See [docs/request-parking.md](../../../../../docs/request-parking.md). +* Drains gracefully on SIGTERM: flips `/readyz` so the Service stops sending + new connections, waits out endpoint propagation (`--drain-delay`), drives + Envoy's admin API to drain established connections, gracefully stops the + ext_proc server so parked requests finish normally (`--drain-timeout`, + derived from the parking budget), then writes a drain-complete marker that + releases the Envoy container's `preStop` hook. See `drain.go` and + `envoydrain.go`. ## status page diff --git a/cmd/atenet/internal/router/cmd.go b/cmd/atenet/internal/router/cmd.go index 936aca59f9..a3396d9ac0 100644 --- a/cmd/atenet/internal/router/cmd.go +++ b/cmd/atenet/internal/router/cmd.go @@ -76,6 +76,13 @@ func NewRouterCmd() *cobra.Command { cmd.Flags().Float64Var(&cfg.ParkedRequest.RetryFactor, "parked-request-retry-factor", defaultParkedRequestRetryFactor, "Multiplier applied to the retry delay after each attempt; must be >= 1") cmd.Flags().Float64Var(&cfg.ParkedRequest.RetryJitter, "parked-request-retry-jitter", defaultParkedRequestRetryJitter, "Random fraction in [0, 1) added to each retry delay to de-synchronize parked requests") cmd.Flags().IntVar(&cfg.ExtProcMaxRequests, "extproc-max-requests", 0, "Circuit-breaker max_requests for Envoy's ext_proc cluster; 0 (the default) derives it as twice --parked-request-max (minimum 1024). Explicit values must be >= --parked-request-max: every parked request holds one slot for its full wait, and the excess is fast-path headroom") + // Graceful shutdown knobs. The router sits behind a Service, so + // route-drain window is needed: after SIGTERM the readiness flip + // must propagate to the Service endpoints before the drain starts. + cmd.Flags().DurationVar(&cfg.DrainDelay, "drain-delay", 13*time.Second, "How long to keep serving after SIGTERM before starting the drain, covering readiness-probe detection and Service endpoint propagation") + cmd.Flags().DurationVar(&cfg.DrainTimeout, "drain-timeout", 0, "Deadline for the ext_proc drain on shutdown; streams still open past it (parked requests included) are forcefully cancelled. 0 (the default) derives --parked-request-budget + the actor route timeout + margin so parked requests always finish normally. Explicit values must be >= --parked-request-budget") + cmd.Flags().StringVar(&cfg.EnvoyAdminAddr, "envoy-admin-address", "127.0.0.1:9901", "Envoy admin interface the shutdown sequence drives to drain the sidecar (healthcheck/fail, drain_listeners, stats polling). Ignored with --atenet-router=agentgateway") + cmd.Flags().StringVar(&cfg.DrainCompleteFile, "drain-complete-file", defaultDrainCompleteFile, "Marker file created (on a pod-shared emptyDir) once the shutdown drain completes; the Envoy container's preStop hook polls for it so Envoy exits as soon as — and no sooner than — the drain is done. Removed at startup to defuse stale markers. Empty disables the handshake") return cmd } diff --git a/cmd/atenet/internal/router/config.go b/cmd/atenet/internal/router/config.go index 9360994288..833ea46304 100644 --- a/cmd/atenet/internal/router/config.go +++ b/cmd/atenet/internal/router/config.go @@ -99,6 +99,21 @@ type routerConfig struct { // excess is fast-path headroom for requests to already-running actors. // 0 derives it from the parking lot — see extProcMaxRequests. ExtProcMaxRequests int + + // DrainDelay is how long the router serves after SIGTERM before draining, + // allowing readiness flip propagation to Service endpoints. DrainTimeout + // bounds the ext_proc drain (0 derives it automatically — see drainTimeout). + DrainDelay time.Duration + DrainTimeout time.Duration + + // EnvoyAdminAddr is the Envoy admin interface the drain sequence drives + // (healthcheck/fail, drain_listeners, stats polling). Same-pod loopback. + EnvoyAdminAddr string + + // DrainCompleteFile is the marker file written once shutdown completes, + // releasing Envoy's preStop hook on the shared emptyDir. Removed at startup to + // defuse stale markers. Empty disables the handshake. + DrainCompleteFile string } func (c routerConfig) atenetRouter() atenetRouter { @@ -128,6 +143,24 @@ func (c routerConfig) extProcMaxRequests() int { return derived } +// drainTimeoutMargin is the slack added on top of the bounded in-flight work +// when deriving the drain timeout, mirroring the +5s Envoy ext_proc +// MessageTimeout margin so the router always sheds before a hard cut. +const drainTimeoutMargin = 5 * time.Second + +// drainTimeout resolves the effective ext_proc drain deadline: an explicit +// flag wins; 0 derives park budget + the DEFAULT route timeout + margin. The +// derivation deliberately ignores a configured --route-timeout so a raised +// route ceiling cannot silently stretch shutdown past the pod's grace period +// (see defaultRouteTimeout); operators pair a long route timeout with an +// explicit --drain-timeout instead. +func (c routerConfig) drainTimeout(parkCfg ParkedRequestConfig) time.Duration { + if c.DrainTimeout > 0 { + return c.DrainTimeout + } + return parkCfg.Budget + defaultRouteTimeout + drainTimeoutMargin +} + // validate rejects flag combinations that would make the router misbehave // rather than merely differ. func (c routerConfig) validate() error { @@ -147,5 +180,15 @@ func (c routerConfig) validate() error { return fmt.Errorf("--extproc-max-requests (%d) must be >= --parked-request-max (%d): a circuit breaker below the parking lot silently truncates it with Envoy-generated 503s", c.ExtProcMaxRequests, c.ParkedRequest.Max) } + if c.DrainDelay < 0 { + return fmt.Errorf("--drain-delay must not be negative, got %s", c.DrainDelay) + } + if c.DrainTimeout < 0 { + return fmt.Errorf("--drain-timeout must not be negative, got %s (0 derives it from --parked-request-budget)", c.DrainTimeout) + } + if c.DrainTimeout > 0 && c.ParkedRequest.enabled() && c.DrainTimeout < c.ParkedRequest.normalized().Budget { + return fmt.Errorf("--drain-timeout (%s) must be >= --parked-request-budget (%s): a drain shorter than the parking budget resets parked requests on shutdown instead of letting them finish", + c.DrainTimeout, c.ParkedRequest.normalized().Budget) + } return nil } diff --git a/cmd/atenet/internal/router/config_test.go b/cmd/atenet/internal/router/config_test.go index 85de49d9d2..2dfeae9664 100644 --- a/cmd/atenet/internal/router/config_test.go +++ b/cmd/atenet/internal/router/config_test.go @@ -17,6 +17,7 @@ package router import ( "strings" "testing" + "time" ) func TestRouterConfigValidate(t *testing.T) { @@ -60,6 +61,33 @@ func TestRouterConfigValidate(t *testing.T) { name: "parking disabled ignores the relation", cfg: routerConfig{ExtProcMaxRequests: 8, ParkedRequest: ParkedRequestConfig{Max: 0}}, }, + { + name: "drain-timeout below the parking budget rejected", + cfg: routerConfig{ParkedRequest: ParkedRequestConfig{Budget: 5 * time.Second, Max: 1024}, DrainTimeout: 2 * time.Second}, + wantErr: "must be >= --parked-request-budget", + }, + { + name: "drain-timeout equal to the parking budget accepted", + cfg: routerConfig{ParkedRequest: ParkedRequestConfig{Budget: 5 * time.Second, Max: 1024}, DrainTimeout: 5 * time.Second}, + }, + { + name: "drain-timeout above the parking budget accepted", + cfg: routerConfig{ParkedRequest: ParkedRequestConfig{Budget: 5 * time.Second, Max: 1024}, DrainTimeout: 30 * time.Second}, + }, + { + name: "short drain-timeout with parking disabled accepted", + cfg: routerConfig{ParkedRequest: ParkedRequestConfig{Max: 0}, DrainTimeout: time.Second}, + }, + { + name: "negative drain-timeout rejected", + cfg: routerConfig{ParkedRequest: ParkedRequestConfig{Max: defaultParkedRequestMax}, DrainTimeout: -time.Second}, + wantErr: "--drain-timeout must not be negative", + }, + { + name: "negative drain-delay rejected", + cfg: routerConfig{ParkedRequest: ParkedRequestConfig{Max: defaultParkedRequestMax}, DrainDelay: -time.Second}, + wantErr: "--drain-delay must not be negative", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -117,6 +145,50 @@ func TestRouterConfigExtProcMaxRequests(t *testing.T) { } } +func TestRouterConfigDrainTimeout(t *testing.T) { + tests := []struct { + name string + cfg routerConfig + parkCfg ParkedRequestConfig + want time.Duration + }{ + { + name: "auto derives budget + route timeout + margin", + cfg: routerConfig{DrainTimeout: 0}, + parkCfg: ParkedRequestConfig{Budget: 5 * time.Second, Max: 1024}.normalized(), + want: 5*time.Second + defaultRouteTimeout + drainTimeoutMargin, + }, + { + name: "auto scales with a larger budget", + cfg: routerConfig{DrainTimeout: 0}, + parkCfg: ParkedRequestConfig{Budget: 30 * time.Second, Max: 1024}.normalized(), + want: 30*time.Second + defaultRouteTimeout + drainTimeoutMargin, + }, + { + name: "parking disabled still derives from the normalized default budget", + cfg: routerConfig{DrainTimeout: 0}, + // normalized() fills Budget even when Max disables parking, so the + // derived drain still covers a later re-enable without a restart + // surprise. + parkCfg: ParkedRequestConfig{Max: 0}.normalized(), + want: defaultParkedRequestBudget + defaultRouteTimeout + drainTimeoutMargin, + }, + { + name: "explicit value wins over derivation", + cfg: routerConfig{DrainTimeout: 42 * time.Second}, + parkCfg: ParkedRequestConfig{Budget: 5 * time.Second, Max: 1024}.normalized(), + want: 42 * time.Second, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := tc.cfg.drainTimeout(tc.parkCfg); got != tc.want { + t.Errorf("drainTimeout() = %s, want %s", got, tc.want) + } + }) + } +} + func TestSetOtlpCollector(t *testing.T) { // No collector address may keep the router from starting. The address // defaults to OTEL_EXPORTER_OTLP_ENDPOINT, which also feeds the router's diff --git a/cmd/atenet/internal/router/drain.go b/cmd/atenet/internal/router/drain.go new file mode 100644 index 0000000000..1c4ef913b1 --- /dev/null +++ b/cmd/atenet/internal/router/drain.go @@ -0,0 +1,162 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package router + +import ( + "context" + "errors" + "io/fs" + "log/slog" + "os" + "path/filepath" + "time" + + "github.com/agent-substrate/substrate/internal/serverboot" +) + +// defaultDrainCompleteFile is where the drain sequence leaves its completion +// marker. The manifest mounts an emptyDir here in both containers; the +// dataplane container's preStop hook polls the same path. +const defaultDrainCompleteFile = "/var/run/atenet/drain-complete" + +// removeStaleDrainMarker deletes a leftover drain-complete marker at startup. +// The emptyDir the marker lives on survives container restarts within the +// pod, and a stale marker would let the dataplane container's preStop hook +// exit the instant a later drain begins — before any connection has drained. +func removeStaleDrainMarker(ctx context.Context, path string) { + if path == "" { + return + } + if err := os.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) { + slog.WarnContext(ctx, "Failed to remove stale drain-complete marker", slog.String("path", path), slog.Any("err", err)) + } +} + +// writeDrainMarker creates the drain-complete marker, releasing the dataplane +// container's preStop hook. Failure is logged, never fatal: the kubelet still bounds the +// hook at terminationGracePeriodSeconds, so a missing marker degrades to a +// slower exit, not a wedge. +func writeDrainMarker(ctx context.Context, path string) { + if path == "" { + return + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + slog.WarnContext(ctx, "Failed to create drain-complete marker directory", slog.String("path", path), slog.Any("err", err)) + return + } + if err := os.WriteFile(path, []byte(time.Now().UTC().Format(time.RFC3339)+"\n"), 0o644); err != nil { + slog.WarnContext(ctx, "Failed to write drain-complete marker", slog.String("path", path), slog.Any("err", err)) + return + } + slog.InfoContext(ctx, "Drain-complete marker written", slog.String("path", path)) +} + +// grpcStopper is the subset of *grpc.Server the drain sequence drives. +type grpcStopper interface { + // GracefulStop stops accepting new connections and RPCs and blocks until + // all in-flight RPCs (parked ext_proc streams, most of all) finish. + GracefulStop() + // Stop cancels in-flight RPCs and closes all connections immediately. + Stop() +} + +// dataplaneDrainer actively drains the dataplane proxy sidecar: established +// connections finish and their in-flight requests complete before the +// implementation returns. envoyDrainer is the Envoy implementation (driving +// the admin API); the orchestrator below stays agnostic of which proxy is +// deployed. +type dataplaneDrainer interface { + // Drain blocks until the dataplane has quiesced or ctx expires; an error + // reports an incomplete drain and the shutdown sequence continues. + Drain(ctx context.Context) error +} + +// drainParams wires the shutdown sequence. The order is forced by the +// ext_proc filter being failClosed in the dataplane: the ext_proc server must +// outlive the dataplane's drain, because any request the dataplane still +// accepts during its drain window needs ext_proc answering. +type drainParams struct { + readiness *serverboot.Readiness + // delay is the route-drain window: after the readiness flip, how long to + // keep serving while the Service endpoints drop this pod. + delay time.Duration + // dataplane, when non-nil, is drained after the delay and before the + // ext_proc server stops, bounded by dataplaneWindow. nil means the + // deployed dataplane offers the router no drain hook and manages its own + // termination; the sequence then proceeds directly to the ext_proc drain. + dataplane dataplaneDrainer + dataplaneWindow time.Duration + // extproc is the ext_proc gRPC server; timeout bounds its graceful drain + // (sized >= the parking budget so parked requests finish normally). + extproc grpcStopper + timeout time.Duration + // stopRest cancels the work context, stopping the remaining subsystems + // (xDS, controller, health checker, statusz) once no traffic depends on + // them. + stopRest func() +} + +// drainOnShutdown drives graceful shutdown when ctx is cancelled (SIGTERM or +// interrupt): flip readiness (Service stops sending new connections), wait +// out the propagation delay, drain the dataplane (established connections +// finish), then drain ext_proc so parked requests complete — force-stopping +// past the timeout — and finally stop everything else. The returned channel +// closes once the sequence completes, so Run can block on it before letting +// the deferred tracer/meter flushes run. +func drainOnShutdown(ctx context.Context, p drainParams) <-chan struct{} { + done := make(chan struct{}) + go func() { + defer close(done) + <-ctx.Done() + slog.InfoContext(ctx, "Shutdown signal received; draining") + p.readiness.MarkNotReady() + time.Sleep(p.delay) + + if p.dataplane != nil { + slog.InfoContext(ctx, "Draining dataplane", slog.Duration("window", p.dataplaneWindow)) + dpCtx, cancel := context.WithTimeout(context.Background(), p.dataplaneWindow) + if err := p.dataplane.Drain(dpCtx); err != nil { + // TODO: Add a shutdown-outcome metric (clean vs + // dataplane-drain-incomplete vs ext_proc force-stopped) so + // unclean shutdowns are visible in dashboards, not only logs. + slog.WarnContext(ctx, "Dataplane drain incomplete; continuing shutdown", slog.Any("err", err)) + } else { + slog.InfoContext(ctx, "Dataplane drained") + } + cancel() + } + + slog.InfoContext(ctx, "Starting ext_proc drain") + drainComplete := make(chan struct{}) + go func() { + p.extproc.GracefulStop() + close(drainComplete) + }() + select { + case <-drainComplete: + slog.InfoContext(ctx, "ext_proc drain completed within deadline") + case <-time.After(p.timeout): + // TODO: Count this in the shutdown-outcome metric above: a + // force-stop here means in-flight ext_proc streams (parked + // requests included) were cancelled — the unclean-shutdown signal + // operators most need to see. + slog.WarnContext(ctx, "ext_proc drain deadline exceeded; forcing stop") + p.extproc.Stop() + } + + p.stopRest() + }() + return done +} diff --git a/cmd/atenet/internal/router/drain_test.go b/cmd/atenet/internal/router/drain_test.go new file mode 100644 index 0000000000..160e1a887d --- /dev/null +++ b/cmd/atenet/internal/router/drain_test.go @@ -0,0 +1,348 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package router + +import ( + "context" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/agent-substrate/substrate/internal/serverboot" +) + +// drainFunc adapts a bare function to the dataplaneDrainer interface. +type drainFunc func(context.Context) error + +func (f drainFunc) Drain(ctx context.Context) error { return f(ctx) } + +// fakeStopper is a grpcStopper whose GracefulStop blocks until release is +// closed, simulating an in-flight (parked) ext_proc stream. +type fakeStopper struct { + release chan struct{} + gracefulCalled atomic.Bool + stopCalled atomic.Bool +} + +func newFakeStopper() *fakeStopper { + return &fakeStopper{release: make(chan struct{})} +} + +func (f *fakeStopper) GracefulStop() { + f.gracefulCalled.Store(true) + <-f.release +} + +func (f *fakeStopper) Stop() { f.stopCalled.Store(true) } + +// orderRecorder captures the sequence of drain steps so tests can assert the +// forced ordering: readiness → dataplane → ext_proc → stopRest. +type orderRecorder struct { + mu sync.Mutex + steps []string +} + +func (r *orderRecorder) add(step string) { + r.mu.Lock() + defer r.mu.Unlock() + r.steps = append(r.steps, step) +} + +func (r *orderRecorder) get() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.steps...) +} + +// TestDrainOnShutdownOrdering asserts the full sequence runs in the forced +// order and that an in-flight ext_proc stream releasing promptly lets the +// drain complete without a force stop. +func TestDrainOnShutdownOrdering(t *testing.T) { + rec := &orderRecorder{} + stopper := newFakeStopper() + readiness := &serverboot.Readiness{} + + // Release the "parked request" shortly after GracefulStop begins. + go func() { + for !stopper.gracefulCalled.Load() { + time.Sleep(5 * time.Millisecond) + } + rec.add("extproc-drain-started") + time.Sleep(20 * time.Millisecond) + close(stopper.release) + }() + + ctx, cancel := context.WithCancel(context.Background()) + done := drainOnShutdown(ctx, drainParams{ + readiness: readiness, + delay: 0, + dataplane: drainFunc(func(context.Context) error { + if readiness.Ready() { + t.Error("dataplane drain ran before the readiness flip") + } + rec.add("dataplane") + return nil + }), + dataplaneWindow: time.Second, + extproc: stopper, + timeout: 5 * time.Second, + stopRest: func() { + if !stopper.gracefulCalled.Load() { + t.Error("stopRest ran before the ext_proc drain") + } + rec.add("stopRest") + }, + }) + + cancel() // simulate SIGTERM + + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("drain did not complete") + } + + want := []string{"dataplane", "extproc-drain-started", "stopRest"} + got := rec.get() + if len(got) != len(want) { + t.Fatalf("drain steps = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("drain steps = %v, want %v", got, want) + } + } + if stopper.stopCalled.Load() { + t.Error("force Stop fired although the graceful drain completed in time") + } + if readiness.Ready() { + t.Error("readiness should be not-ready after drain") + } +} + +// TestDrainOnShutdownForceStopsAfterTimeout asserts a stream that never +// finishes is force-stopped at the drain timeout and the sequence still +// completes. +func TestDrainOnShutdownForceStopsAfterTimeout(t *testing.T) { + stopper := newFakeStopper() // release never closed → GracefulStop blocks forever + var stopRest atomic.Bool + + ctx, cancel := context.WithCancel(context.Background()) + start := time.Now() + done := drainOnShutdown(ctx, drainParams{ + readiness: &serverboot.Readiness{}, + delay: 0, + dataplane: nil, // dataplane offers no drain hook (agentgateway shape) + dataplaneWindow: time.Second, + extproc: stopper, + timeout: 100 * time.Millisecond, + stopRest: func() { stopRest.Store(true) }, + }) + cancel() + + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("drain did not force-stop within deadline") + } + if elapsed := time.Since(start); elapsed > 2*time.Second { + t.Fatalf("force stop took too long (%v); expected ~drain timeout", elapsed) + } + if !stopper.stopCalled.Load() { + t.Error("force Stop was not called for the wedged stream") + } + if !stopRest.Load() { + t.Error("stopRest did not run after the force stop") + } +} + +// TestDrainOnShutdownDataplaneFailureContinues asserts an incomplete +// dataplane drain (connections still active at its deadline) does not wedge +// the sequence. +func TestDrainOnShutdownDataplaneFailureContinues(t *testing.T) { + stopper := newFakeStopper() + close(stopper.release) // ext_proc idle + + ctx, cancel := context.WithCancel(context.Background()) + done := drainOnShutdown(ctx, drainParams{ + readiness: &serverboot.Readiness{}, + delay: 0, + dataplane: drainFunc(func(ctx context.Context) error { return context.DeadlineExceeded }), + dataplaneWindow: 50 * time.Millisecond, + extproc: stopper, + timeout: time.Second, + stopRest: func() {}, + }) + cancel() + + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("drain wedged on a failed dataplane drain") + } + if !stopper.gracefulCalled.Load() { + t.Error("ext_proc drain skipped after dataplane drain failure") + } +} + +// fakeEnvoyAdmin is an httptest stand-in for the Envoy admin interface. +type fakeEnvoyAdmin struct { + mu sync.Mutex + posts []string + statsCalls int + activeSeries []int // successive downstream_cx_active values to report +} + +func (f *fakeEnvoyAdmin) handler() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + switch { + case r.Method == http.MethodPost: + f.posts = append(f.posts, r.URL.RequestURI()) + case strings.HasPrefix(r.URL.Path, "/stats"): + // Report the series value for this poll; once exhausted, repeat the + // last value so timing-dependent extra polls see a stable count. + idx := f.statsCalls + if idx >= len(f.activeSeries) { + idx = len(f.activeSeries) - 1 + } + active := f.activeSeries[idx] + f.statsCalls++ + // Includes an admin line that must be excluded from the sum. + w.Write([]byte( + "listener.admin.downstream_cx_active: 1\n" + + "listener.0.0.0.0_8080.downstream_cx_active: " + itoa(active) + "\n" + + "http.ingress_http.downstream_cx_active_unrelated: 99\n")) + } + } +} + +func itoa(n int) string { + if n == 0 { + return "0" + } + var b []byte + for n > 0 { + b = append([]byte{byte('0' + n%10)}, b...) + n /= 10 + } + return string(b) +} + +// TestEnvoyDrainerDrainsToZero asserts the drainer POSTs the two admin calls +// and polls stats until active connections reach zero, excluding admin +// listeners from the count. +func TestEnvoyDrainerDrainsToZero(t *testing.T) { + admin := &fakeEnvoyAdmin{activeSeries: []int{2, 1, 0}} + srv := httptest.NewServer(admin.handler()) + defer srv.Close() + + d := newEnvoyDrainer(strings.TrimPrefix(srv.URL, "http://")) + d.pollInterval = 5 * time.Millisecond + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + if err := d.Drain(ctx); err != nil { + t.Fatalf("Drain: %v", err) + } + + admin.mu.Lock() + defer admin.mu.Unlock() + wantPosts := []string{"/healthcheck/fail", "/drain_listeners?graceful&skip_exit"} + if len(admin.posts) != len(wantPosts) { + t.Fatalf("admin POSTs = %v, want %v", admin.posts, wantPosts) + } + for i := range wantPosts { + if admin.posts[i] != wantPosts[i] { + t.Fatalf("admin POSTs = %v, want %v", admin.posts, wantPosts) + } + } + if admin.statsCalls < 3 { + t.Errorf("stats polled %d times, want >= 3 (series 2,1,0)", admin.statsCalls) + } +} + +// TestEnvoyDrainerAdminGone asserts an unreachable admin interface (Envoy +// already exited) is treated as a completed drain, quickly. +func TestEnvoyDrainerAdminGone(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + addr := strings.TrimPrefix(srv.URL, "http://") + srv.Close() // nothing listens there anymore + + d := newEnvoyDrainer(addr) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + start := time.Now() + if err := d.Drain(ctx); err != nil { + t.Fatalf("Drain against a gone admin: %v", err) + } + if time.Since(start) > time.Second { + t.Error("Drain against a gone admin should return promptly") + } +} + +// TestEnvoyDrainerDeadlineWithActiveConnections asserts the drainer reports +// the deadline error when connections never reach zero, so the orchestrator +// can log it and continue. +func TestEnvoyDrainerDeadlineWithActiveConnections(t *testing.T) { + admin := &fakeEnvoyAdmin{activeSeries: []int{5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5}} + srv := httptest.NewServer(admin.handler()) + defer srv.Close() + + d := newEnvoyDrainer(strings.TrimPrefix(srv.URL, "http://")) + d.pollInterval = 5 * time.Millisecond + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if err := d.Drain(ctx); err == nil { + t.Fatal("Drain with persistent connections should report the deadline") + } +} + +// TestDrainMarkerLifecycle pins the preStop handshake marker semantics: a +// stale marker is removed at startup (emptyDir survives container restarts, +// and a stale marker would release Envoy's preStop the moment a later drain +// begins), the marker is created on drain completion, and an empty path +// disables the handshake entirely. +func TestDrainMarkerLifecycle(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + marker := dir + "/nested/drain-complete" + + // Empty path: both operations are no-ops. + removeStaleDrainMarker(ctx, "") + writeDrainMarker(ctx, "") + + // Removing a marker that does not exist is quiet. + removeStaleDrainMarker(ctx, marker) + + // Writing creates parent directories and the file. + writeDrainMarker(ctx, marker) + if _, err := os.Stat(marker); err != nil { + t.Fatalf("marker not created: %v", err) + } + + // A restart must defuse the stale marker. + removeStaleDrainMarker(ctx, marker) + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("stale marker not removed: %v", err) + } +} diff --git a/cmd/atenet/internal/router/envoydrain.go b/cmd/atenet/internal/router/envoydrain.go new file mode 100644 index 0000000000..22aa3e591b --- /dev/null +++ b/cmd/atenet/internal/router/envoydrain.go @@ -0,0 +1,146 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package router + +import ( + "bufio" + "context" + "fmt" + "log/slog" + "net/http" + "strconv" + "strings" + "time" +) + +// envoyDrainPollInterval is how often the drainer re-reads Envoy's active +// downstream connection count while waiting for the drain to complete. +const envoyDrainPollInterval = 250 * time.Millisecond + +// envoyDrainer drives the Envoy sidecar's graceful drain over its admin +// interface (same-pod loopback). The drain sequence in router.go invokes it +// after the readiness flip has propagated (no new connections arrive) and +// before the ext_proc server stops (Envoy still needs ext_proc for any +// request it accepts while draining — the filter is failClosed). +// +// Every error path degrades instead of wedging: an unreachable admin +// interface means Envoy is already gone (its own SIGTERM raced us), which is +// itself a completed drain. +type envoyDrainer struct { + adminAddr string + client *http.Client + pollInterval time.Duration +} + +func newEnvoyDrainer(adminAddr string) *envoyDrainer { + return &envoyDrainer{ + adminAddr: adminAddr, + client: &http.Client{Timeout: 2 * time.Second}, + pollInterval: envoyDrainPollInterval, + } +} + +// Drain triggers Envoy's graceful listener drain via the admin interface and +// polls active downstream connections until zero or ctx expires. Returns nil +// when drained or if Envoy has already exited. +func (d *envoyDrainer) Drain(ctx context.Context) error { + if !d.post(ctx, "/healthcheck/fail") { + return d.adminGone(ctx) // unreachable: Envoy already exited (or the window expired mid-call) + } + if !d.post(ctx, "/drain_listeners?graceful&skip_exit") { + return d.adminGone(ctx) + } + + ticker := time.NewTicker(d.pollInterval) + defer ticker.Stop() + for { + active, ok := d.activeConnections(ctx) + if !ok { + return d.adminGone(ctx) + } + if active == 0 { + return nil + } + select { + case <-ctx.Done(): + return fmt.Errorf("%d downstream connections still active at the drain deadline: %w", active, ctx.Err()) + case <-ticker.C: + } + } +} + +// adminGone disambiguates an admin-call failure: a request failing because +// the drain window expired mid-call is a deadline, not evidence that Envoy +// exited — misreporting it as "drained" would hide an incomplete drain from +// the shutdown logs. +func (d *envoyDrainer) adminGone(ctx context.Context) error { + if err := ctx.Err(); err != nil { + return fmt.Errorf("drain window expired while querying the Envoy admin interface: %w", err) + } + return nil // genuinely unreachable: Envoy already exited, drain moot +} + +// post issues an admin POST and reports whether Envoy answered at all. +// Non-2xx answers are logged and treated as answered — the drain continues. +func (d *envoyDrainer) post(ctx context.Context, path string) bool { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "http://"+d.adminAddr+path, nil) + if err != nil { + slog.WarnContext(ctx, "Building Envoy admin request failed", slog.String("path", path), slog.Any("err", err)) + return true + } + resp, err := d.client.Do(req) + if err != nil { + slog.InfoContext(ctx, "Envoy admin unreachable; treating the sidecar as already stopped", slog.String("path", path), slog.Any("err", err)) + return false + } + defer resp.Body.Close() + if resp.StatusCode/100 != 2 { + slog.WarnContext(ctx, "Envoy admin call failed", slog.String("path", path), slog.Int("status", resp.StatusCode)) + } + return true +} + +// activeConnections sums Envoy's non-admin downstream_cx_active gauges. The +// admin listener's own connections (including this poll) are excluded, else +// the count could never reach zero. +func (d *envoyDrainer) activeConnections(ctx context.Context) (int, bool) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://"+d.adminAddr+"/stats?filter=downstream_cx_active", nil) + if err != nil { + return 0, true + } + resp, err := d.client.Do(req) + if err != nil { + return 0, false + } + defer resp.Body.Close() + + total := 0 + scanner := bufio.NewScanner(resp.Body) + for scanner.Scan() { + name, value, found := strings.Cut(scanner.Text(), ":") + if !found || strings.Contains(name, "admin") { + continue + } + if !strings.HasSuffix(strings.TrimSpace(name), "downstream_cx_active") { + continue + } + n, err := strconv.Atoi(strings.TrimSpace(value)) + if err != nil { + continue + } + total += n + } + return total, true +} diff --git a/cmd/atenet/internal/router/extproc.go b/cmd/atenet/internal/router/extproc.go index df57426d1f..a668a813d6 100644 --- a/cmd/atenet/internal/router/extproc.go +++ b/cmd/atenet/internal/router/extproc.go @@ -61,26 +61,16 @@ func NewExtProcServer(port int, apiClient ateapipb.ControlClient, routeDuration } } -func (s *ExtProcServer) Serve(ctx context.Context, lis net.Listener) error { +// NewGRPCServer builds the gRPC server with the ext_proc service registered. +// The caller owns its lifecycle: Run serves it, and the drain sequence in +// drain.go stops it — gracefully first so in-flight streams (parked requests +// above all) finish, forcefully past the drain timeout. +func (s *ExtProcServer) NewGRPCServer() *grpc.Server { grpcServer := grpc.NewServer( grpc.StatsHandler(otelgrpc.NewServerHandler()), ) extprocv3.RegisterExternalProcessorServer(grpcServer, s) - - errChan := make(chan error, 1) - go func() { - if err := grpcServer.Serve(lis); err != nil { - errChan <- err - } - }() - - select { - case <-ctx.Done(): - grpcServer.GracefulStop() - return nil - case err := <-errChan: - return err - } + return grpcServer } func (s *ExtProcServer) Process(stream extprocv3.ExternalProcessor_ProcessServer) error { diff --git a/cmd/atenet/internal/router/router.go b/cmd/atenet/internal/router/router.go index d8607b72fa..f22696c389 100644 --- a/cmd/atenet/internal/router/router.go +++ b/cmd/atenet/internal/router/router.go @@ -21,7 +21,6 @@ import ( "log/slog" "net" "net/http" - "os" "os/signal" "syscall" @@ -117,26 +116,33 @@ func NewRouterServer(cfg routerConfig) (*RouterServer, error) { } func (s *RouterServer) Run(ctx context.Context) error { - ctx, cancel := context.WithCancel(ctx) - defer cancel() + // shutdownCtx signals SIGTERM/SIGINT; kept separate from the work context + // so in-flight ext_proc streams (parked requests, most of all) are not + // cancelled the moment the signal arrives. drainOnShutdown drives the + // shutdown sequence: readiness flip → route-drain delay → Envoy drain → + // ext_proc drain → stop the rest. + shutdownCtx, stopSignals := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) + defer stopSignals() - sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) - go func() { - <-sigChan - cancel() - }() + ctx, cancelWork := context.WithCancel(ctx) + defer cancelWork() // Validate the configuration before doing any other work, so a bad flag // combination fails fast — no tracing, metrics, or connections are set up // for a router that is about to refuse to start. The parking config is - // resolved once here so every consumer — the resumer's retry loop and the - // Envoy ext_proc timeout — sees the same effective values. + // resolved once here so every consumer — the resumer's retry loop, the + // Envoy ext_proc timeout, and the drain timeout — sees the same effective + // values. if err := s.cfg.validate(); err != nil { return fmt.Errorf("invalid router configuration: %w", err) } parkCfg := s.cfg.ParkedRequest.normalized() + // The drain-complete marker persists container restarts (emptyDir); a + // stale one would release the Envoy preStop hook the moment a later drain + // begins. + removeStaleDrainMarker(ctx, s.cfg.DrainCompleteFile) + serverboot.InitLogger() if err := serverboot.SetLogLevel(s.cfg.LogLevel); err != nil { return err @@ -162,7 +168,15 @@ func (s *RouterServer) Run(ctx context.Context) error { } defer serverboot.ShutdownProvider("MeterProvider", mp.Shutdown) - go serverboot.StartMetricsServer(ctx, serverboot.MetricsServerOptions{Addr: s.cfg.MetricsAddr}) + // readiness flips to not-ready on SIGTERM so /readyz reports 503 while the + // pod drains — dropping it from the Service endpoints — while /healthz + // stays 200 for liveness. + readiness := &serverboot.Readiness{} + go serverboot.StartMetricsServer(ctx, serverboot.MetricsServerOptions{ + Addr: s.cfg.MetricsAddr, + Readiness: readiness, + EnableHealthz: true, + }) dialOpts, err := ateapiauth.DialOptions(ateapiauth.ClientConfig{ UseTokenAuth: s.cfg.Auth.AteapiUseTokenAuth, @@ -215,7 +229,9 @@ func (s *RouterServer) Run(ctx context.Context) error { return nil }) - // Start ExtProc Server + // Start ExtProc Server. Driven by the drain sequence rather than context + // cancel: ext_proc is failClosed, so it must outlive Envoy's drain. + extprocGRPC := s.extprocSrv.NewGRPCServer() g.Go(func() error { slog.InfoContext(ctx, "Starting ExtProc Server", slog.Int("port", s.cfg.ExtprocPort)) lis, err := net.Listen("tcp", fmt.Sprintf(":%d", s.cfg.ExtprocPort)) @@ -224,7 +240,8 @@ func (s *RouterServer) Run(ctx context.Context) error { } defer lis.Close() - return s.extprocSrv.Serve(ctx, lis) + // Serve returns nil after Stop/GracefulStop. + return extprocGRPC.Serve(lis) }) // Start HTTP status endpoint @@ -252,7 +269,33 @@ func (s *RouterServer) Run(ctx context.Context) error { }) } - return g.Wait() + // Only the Envoy dataplane offers the router an active drain hook (its + // admin API); agentgateway manages its own termination, so no drainer is + // wired and the sequence proceeds straight to the ext_proc drain. + var dataplane dataplaneDrainer + if s.cfg.atenetRouter() == atenetRouterEnvoy { + dataplane = newEnvoyDrainer(s.cfg.EnvoyAdminAddr) + } + drainDone := drainOnShutdown(shutdownCtx, drainParams{ + readiness: readiness, + delay: s.cfg.DrainDelay, + dataplane: dataplane, + dataplaneWindow: defaultRouteTimeout + drainTimeoutMargin, + extproc: extprocGRPC, + timeout: s.cfg.drainTimeout(parkCfg), + stopRest: func() { + // Written first so the dataplane container's preStop hook (polling + // this marker on the shared emptyDir) releases as soon as nothing + // client-visible remains; then stop the remaining subsystems. + writeDrainMarker(ctx, s.cfg.DrainCompleteFile) + cancelWork() + }, + }) + + err = g.Wait() + <-drainDone + slog.InfoContext(ctx, "Shutdown complete") + return err } // setOtlpCollector points Envoy's tracer at the configured collector, and diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 1ba6812c3b..bcf07c7e63 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -99,6 +99,13 @@ const defaultExtProcMaxRequests = 2048 // the ceiling on a single request from the ingress listener to the actor's // response. It bounds the actor's own handling time, not the resume that // precedes it — parking and the ext_proc timeout cover that part. +// +// The drain sequence also sizes its Envoy-drain window and derived +// drain-timeout from this DEFAULT — deliberately not from the configured +// --route-timeout, so raising the route ceiling for long-running actor turns +// does not silently stretch every shutdown past terminationGracePeriodSeconds. +// Operators who raise --route-timeout and want such turns to survive a drain +// must raise --drain-timeout (and the grace period) explicitly. const defaultRouteTimeout = 10 * time.Second // envoyDefaultStreamIdleTimeout is the stream idle timeout Envoy applies when @@ -440,7 +447,12 @@ func (x *XdsServer) Serve(ctx context.Context, lis net.Listener) error { select { case <-ctx.Done(): - grpcServer.GracefulStop() + // Hard stop, deliberately: ADS streams are open-ended, so GracefulStop + // would block until Envoy disconnects — which during shutdown it only + // does by dying. xDS clients treat a control-plane disconnect as benign + // (reconnect with backoff, keep the last delivered config), and the + // drain sequence only cancels this context after Envoy has drained. + grpcServer.Stop() return nil case err := <-errChan: return err diff --git a/docs/request-parking.md b/docs/request-parking.md index 35ed4b6ed2..8378c7f80b 100644 --- a/docs/request-parking.md +++ b/docs/request-parking.md @@ -96,6 +96,17 @@ When parking is **disabled** (`--parked-request-max=0`), the router fails fast: admission cap, and only `Aborted` (concurrent-resume) conflicts are retried, within a `15s` budget. +### Parked requests survive router shutdown + +A request parked when the router pod receives SIGTERM is **not** reset: the +shutdown sequence keeps the ext_proc server (and, via a preStop handshake, the +Envoy sidecar) alive until in-flight streams finish, and the ext_proc drain +deadline (`--drain-timeout`) defaults to a value derived from +`--parked-request-budget` and is validated at startup to be `>=` the budget — +so a parked request always gets its full budget and a normal verdict (routed +`200` or capacity `503`) even mid-termination. See the graceful-shutdown knobs +(`--drain-delay`, `--drain-timeout`) in `manifests/ate-install/atenet-router.yaml`. + ## Configuration | Flag | Default | Meaning | diff --git a/hack/verify-atenet-drain.sh b/hack/verify-atenet-drain.sh new file mode 100755 index 0000000000..263f232074 --- /dev/null +++ b/hack/verify-atenet-drain.sh @@ -0,0 +1,200 @@ +#!/usr/bin/env bash + +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Verifies the atenet-router graceful drain end to end on a live cluster: +# +# 1. Pins the counter demo's worker pool to a single worker and +# oversubscribes it with two actors, so a request for the second actor +# parks on the router. +# 2. Deletes the router pod WHILE the request is parked, then frees the +# worker — the parked request must complete (200) through the +# Terminating pod: park -> resume -> route rides out the shutdown. +# 3. Asserts /readyz flips to 503 while /healthz stays 200, the pod +# terminates via the drain sequence (after the drain-delay, well under +# terminationGracePeriodSeconds — i.e. the drain-complete marker +# released Envoy's preStop, not the SIGKILL path), and the router logs +# show the ordered drain sequence. +# +# This lives in hack/ rather than the e2e suites because it deletes the +# shared router pod, which breaks port-forward tunnels held by other suites — +# the e2e runner executes suites in parallel, so this check must run alone. +# +# Prerequisites: a cluster with the ate system and the counter demo installed +# (hack/install-ate-kind.sh --deploy-ate-system --deploy-demo-counter), and no +# actors currently running on the counter demo pool (the pool is scaled to 1 +# for the duration of the check and restored afterwards). +# +# Respects KUBECTL_CONTEXT like the other hack scripts. + +set -o errexit -o nounset -o pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "${ROOT}" + +ROUTER_NS="ate-system" +DEMO_NS="ate-demo-counter" +DEMO_POOL="counter" +ATESPACE="drain-check" +SUFFIX="$(date +%s)" +ACTOR_BUSY="busy-${SUFFIX}" # occupies the only worker +ACTOR_PARKED="parked-${SUFFIX}" # its request parks, then survives the drain +LOCAL_HTTP_PORT="${LOCAL_HTTP_PORT:-18080}" +LOCAL_METRICS_PORT="${LOCAL_METRICS_PORT:-19090}" +# Xs must be the trailing characters: BSD (macOS) mktemp rejects suffixes. +LOG_FILE="$(mktemp /tmp/atenet-drain-check-log.XXXXXX)" +CURL_OUT="$(mktemp /tmp/atenet-drain-check-out.XXXXXX)" + +run_kubectl() { kubectl ${KUBECTL_CONTEXT:+--context=${KUBECTL_CONTEXT}} "$@"; } +run_kubectl_ate() { go run ./cmd/kubectl-ate ${KUBECTL_CONTEXT:+--context=${KUBECTL_CONTEXT}} "$@"; } + +log_step() { echo; echo "[drain-check]: $*"; } +fail() { echo "FAIL: $*" >&2; exit 1; } + +BG_PIDS=() +ORIG_REPLICAS="" +cleanup() { + local code=$? + set +e + for pid in ${BG_PIDS[@]+"${BG_PIDS[@]}"}; do kill "${pid}" 2>/dev/null; done + log_step "cleanup: actors and pool" + # Deletion requires suspended actors; suspend both best-effort first (the + # busy actor is RUNNING on any failure before the drain step). + run_kubectl_ate suspend actor "${ACTOR_PARKED}" -a "${ATESPACE}" >/dev/null 2>&1 + run_kubectl_ate suspend actor "${ACTOR_BUSY}" -a "${ATESPACE}" >/dev/null 2>&1 + run_kubectl_ate delete actor "${ACTOR_PARKED}" -a "${ATESPACE}" >/dev/null 2>&1 + run_kubectl_ate delete actor "${ACTOR_BUSY}" -a "${ATESPACE}" >/dev/null 2>&1 + if [[ -n "${ORIG_REPLICAS}" ]]; then + run_kubectl patch workerpool -n "${DEMO_NS}" "${DEMO_POOL}" --type merge \ + -p "{\"spec\":{\"replicas\":${ORIG_REPLICAS}}}" >/dev/null 2>&1 + run_kubectl rollout status "deploy/${DEMO_POOL}" -n "${DEMO_NS}" --timeout=180s >/dev/null 2>&1 + fi + exit "${code}" +} +trap cleanup EXIT + +# --- Preflight ------------------------------------------------------------- + +log_step "preflight" +phase="$(run_kubectl get actortemplate -n "${DEMO_NS}" "${DEMO_POOL}" -o jsonpath='{.status.phase}' 2>/dev/null || true)" +[[ "${phase}" == "Ready" ]] || fail "ActorTemplate ${DEMO_NS}/${DEMO_POOL} is not Ready (phase: ${phase:-absent}); install the counter demo first" +# Column 4 is STATUS; the header row's "ASSIGNED ACTOR" column name must not +# trip the check. +if run_kubectl_ate get workers 2>/dev/null | awk 'NR>1 && $4=="ASSIGNED"' | grep -q .; then + fail "workers on the ${DEMO_POOL} pool are ASSIGNED; the check scales the pool to 1 and would crash running actors — suspend them first" +fi +for port in "${LOCAL_HTTP_PORT}" "${LOCAL_METRICS_PORT}"; do + if (exec 3<>"/dev/tcp/127.0.0.1/${port}") 2>/dev/null; then + exec 3>&- 3<&- + fail "local port ${port} is already in use (a stale port-forward?); free it or set LOCAL_HTTP_PORT/LOCAL_METRICS_PORT" + fi +done + +# --- Fixture: 1 worker, two actors, worker occupied -------------------------- + +log_step "pinning ${DEMO_NS}/${DEMO_POOL} to 1 worker" +ORIG_REPLICAS="$(run_kubectl get workerpool -n "${DEMO_NS}" "${DEMO_POOL}" -o jsonpath='{.spec.replicas}')" +run_kubectl patch workerpool -n "${DEMO_NS}" "${DEMO_POOL}" --type merge -p '{"spec":{"replicas":1}}' >/dev/null +run_kubectl rollout status "deploy/${DEMO_POOL}" -n "${DEMO_NS}" --timeout=180s >/dev/null + +log_step "creating actors ${ACTOR_BUSY} + ${ACTOR_PARKED} in atespace ${ATESPACE}" +run_kubectl_ate create atespace "${ATESPACE}" >/dev/null 2>&1 || true +run_kubectl_ate create actor "${ACTOR_BUSY}" -a "${ATESPACE}" --template="${DEMO_NS}/${DEMO_POOL}" >/dev/null +run_kubectl_ate create actor "${ACTOR_PARKED}" -a "${ATESPACE}" --template="${DEMO_NS}/${DEMO_POOL}" >/dev/null + +log_step "occupying the only worker with ${ACTOR_BUSY}" +for i in $(seq 1 20); do + run_kubectl_ate resume actor "${ACTOR_BUSY}" -a "${ATESPACE}" >/dev/null 2>&1 && break + [[ "$i" == 20 ]] && fail "could not resume ${ACTOR_BUSY} (worker never became available)" + sleep 3 +done + +# --- Observers --------------------------------------------------------------- + +POD="$(run_kubectl get pods -n "${ROUTER_NS}" -l app=atenet-router --no-headers | awk '$3=="Running"{print $1}')" +# wc -w pads its output on macOS; compare numerically. +(( $(wc -w <<<"${POD}") == 1 )) || fail "expected exactly 1 running router pod, got: ${POD:-none}" +log_step "router pod under test: ${POD}" + +run_kubectl logs -n "${ROUTER_NS}" "${POD}" -c atenet-router -f > "${LOG_FILE}" 2>&1 & +BG_PIDS+=($!) +run_kubectl port-forward -n "${ROUTER_NS}" svc/atenet-router "${LOCAL_HTTP_PORT}:80" >/dev/null 2>&1 & +BG_PIDS+=($!) +run_kubectl port-forward -n "${ROUTER_NS}" "${POD}" "${LOCAL_METRICS_PORT}:9090" >/dev/null 2>&1 & +BG_PIDS+=($!) +sleep 3 + +readyz() { curl -s -o /dev/null -w '%{http_code}' "localhost:${LOCAL_METRICS_PORT}/readyz"; } +healthz() { curl -s -o /dev/null -w '%{http_code}' "localhost:${LOCAL_METRICS_PORT}/healthz"; } + +# The port-forwards need a moment to come up; retry before judging. +for i in $(seq 1 20); do + [[ "$(readyz)" == "200" ]] && break + [[ "$i" == 20 ]] && fail "steady-state /readyz != 200" + sleep 0.5 +done +[[ "$(healthz)" == "200" ]] || fail "steady-state /healthz != 200" +echo "steady state: /readyz=200 /healthz=200" + +# --- The drain: park -> delete pod -> free worker ---------------------------- + +log_step "firing the request that will park (single worker is busy)" +( curl -s --max-time 30 -w '\nHTTP=%{http_code}\n' \ + -H "Host: ${ACTOR_PARKED}.${ATESPACE}.actors.resources.substrate.ate.dev" \ + "http://localhost:${LOCAL_HTTP_PORT}/" > "${CURL_OUT}" 2>&1 ) & +CURL_PID=$! +sleep 1 # inside the 5s park budget; the request is parked on the router + +log_step "deleting the router pod with the request parked on it" +DELETE_T="$(date +%s)" +run_kubectl delete pod -n "${ROUTER_NS}" "${POD}" --wait=false >/dev/null +until [[ -n "$(run_kubectl get pod -n "${ROUTER_NS}" "${POD}" -o jsonpath='{.metadata.deletionTimestamp}' 2>/dev/null)" ]]; do + sleep 0.2 +done +[[ "$(readyz)" == "503" ]] || fail "/readyz != 503 while Terminating" +[[ "$(healthz)" == "200" ]] || fail "/healthz != 200 while Terminating" +echo "Terminating: /readyz=503 /healthz=200" + +log_step "freeing the worker (suspend ${ACTOR_BUSY}) — the parked request must now complete" +run_kubectl_ate suspend actor "${ACTOR_BUSY}" -a "${ATESPACE}" >/dev/null + +wait "${CURL_PID}" || true +grep -q "HTTP=200" "${CURL_OUT}" || fail "parked request did not return 200 through the Terminating pod: $(cat "${CURL_OUT}")" +grep -q "hello from" "${CURL_OUT}" || fail "parked request body missing the counter greeting: $(cat "${CURL_OUT}")" +echo "parked request served by the Terminating pod:" +sed 's/^/ /' "${CURL_OUT}" + +# --- Termination window ------------------------------------------------------- + +log_step "waiting for the pod to terminate" +until ! run_kubectl get pod -n "${ROUTER_NS}" "${POD}" >/dev/null 2>&1; do sleep 1; done +ELAPSED=$(( $(date +%s) - DELETE_T )) +echo "pod terminated ${ELAPSED}s after deletion" +(( ELAPSED >= 10 )) || fail "terminated in ${ELAPSED}s — before the 13s drain-delay could run; the drain sequence was skipped" +(( ELAPSED <= 55 )) || fail "terminated in ${ELAPSED}s — at the grace period; SIGKILL path, the drain-complete handshake did not release Envoy" + +log_step "drain log sequence" +for marker in "Shutdown signal received; draining" "Draining dataplane" "Starting ext_proc drain" "Drain-complete marker written" "Shutdown complete"; do + grep -q "${marker}" "${LOG_FILE}" || fail "router log missing \"${marker}\" (see ${LOG_FILE})" +done +grep -E "Shutdown signal|Draining dataplane|Dataplane drain|ext_proc drain|marker written|Shutdown complete" "${LOG_FILE}" | sed 's/^/ /' + +log_step "waiting for the replacement router pod" +run_kubectl rollout status deploy/atenet-router -n "${ROUTER_NS}" --timeout=120s >/dev/null + +echo +echo "PASS: atenet-router graceful drain verified (parked request served by the" +echo "Terminating pod; readiness flipped; terminated in ${ELAPSED}s, inside the" +echo "drain window; log sequence complete; replacement pod Ready)." diff --git a/manifests/ate-install/atenet-router.yaml b/manifests/ate-install/atenet-router.yaml index 15f0b2beb9..0d64a269e2 100644 --- a/manifests/ate-install/atenet-router.yaml +++ b/manifests/ate-install/atenet-router.yaml @@ -120,6 +120,13 @@ spec: prometheus.io/port: "9090" spec: serviceAccountName: atenet-router + # Budget for the full shutdown sequence: --drain-delay (13s, readiness + # propagation) + the Envoy drain window (~15s) + the derived + # --drain-timeout (park budget + route timeout + margin, ~20s at + # defaults) + slack for the force stop and the tracer/meter flush. The + # sum must fit within terminationGracePeriodSeconds, or the kubelet + # SIGKILLs mid-drain. + terminationGracePeriodSeconds: 60 containers: - name: atenet-router image: ko://github.com/agent-substrate/substrate/cmd/atenet @@ -139,10 +146,17 @@ spec: - "--ateapi-address=dns:///api.ate-system.svc:443" - "--ateapi-ca-file=/run/servicedns-ca/trust-bundle.pem" - "--ateapi-client-cert=/run/podidentity.podcert.ate.dev/credential-bundle.pem" + # Graceful shutdown knobs. --drain-timeout is left at its derived + # default (park budget + the default route timeout + margin). The sum + # of the sequence must fit within terminationGracePeriodSeconds above. + - "--drain-delay=13s" # Envoy's end-to-end timeout on the workload route. Raise it for actors # whose turns legitimately run long — a harness relaying an LLM # completion holds the request open for the whole generation, and at the - # 10s default the client gets a 504 mid-turn. + # 10s default the client gets a 504 mid-turn. NOTE: the drain sequence + # deliberately does NOT scale with this; if you raise it and want long + # turns to survive a shutdown, raise --drain-timeout and + # terminationGracePeriodSeconds alongside it. # - "--route-timeout=5m" env: - name: POD_NAME @@ -175,6 +189,21 @@ spec: containerPort: 4040 - name: metrics containerPort: 9090 + readinessProbe: + httpGet: + path: /readyz + port: 9090 + initialDelaySeconds: 5 + periodSeconds: 2 + failureThreshold: 3 + # /healthz stays 200 while a terminating pod drains; /readyz turns + # 503, so liveness and readiness diverge correctly during shutdown. + livenessProbe: + httpGet: + path: /healthz + port: 9090 + initialDelaySeconds: 10 + periodSeconds: 10 volumeMounts: # Router's own client identity presented to ateapi. - name: "podidentity" @@ -182,6 +211,11 @@ spec: # Trust bundle used to verify ateapi's servicedns serving cert. - name: "servicedns-ca" mountPath: "/run/servicedns-ca" + # Shared with the Envoy container: the router writes the + # drain-complete marker here at the end of its shutdown sequence, + # releasing Envoy's preStop poll. + - name: "drain-signal" + mountPath: "/var/run/atenet" - name: envoy image: envoyproxy/envoy:v1.30-latest command: @@ -190,6 +224,14 @@ spec: - "/etc/envoy/envoy.yaml" - "--component-log-level" - "upstream:debug,router:debug,ext_proc:debug" + # Prevents Envoy from fast-exiting on SIGTERM before atenet-router finishes + # its drain sequence. Polls for the drain-complete marker written by the + # router on the shared emptyDir, terminating Envoy as soon as the drain + # completes (or at terminationGracePeriodSeconds if the router crashes). + lifecycle: + preStop: + exec: + command: ["sh", "-c", "while [ ! -f /var/run/atenet/drain-complete ]; do sleep 0.5; done"] ports: - name: http containerPort: 8080 @@ -211,10 +253,19 @@ spec: # --atunnel-client-identity allows. - name: "podidentity" mountPath: "/run/podidentity.podcert.ate.dev" + # The preStop hook polls this path for the router's drain-complete + # marker; read-only, the router is the only writer. + - name: "drain-signal" + mountPath: "/var/run/atenet" + readOnly: true volumes: - name: envoy-config configMap: name: atenet-router-envoy-config + # Pod-shared scratch space for the shutdown handshake between the router + # and the Envoy preStop hook. + - name: "drain-signal" + emptyDir: {} - name: "servicedns" projected: sources: