From 45ee246590a022a5e7417acdfc77150b4f6709ec Mon Sep 17 00:00:00 2001 From: Maya Wang Date: Tue, 21 Jul 2026 11:58:31 -0700 Subject: [PATCH] feat: configurable timeouts + warmup for long-running / large-snapshot actors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running a heavy suspend/resume workload (a multi-process Node.js agent) end to end surfaced four control-plane assumptions that are too tight for actors with slow initialization or large gVisor snapshots. Each is made tunable (or detected at runtime); all defaults are unchanged, so this is purely additive. - atecontroller: golden-snapshot warmup is env-configurable via ATE_GOLDEN_WARMUP_SECONDS (default 20s, unchanged). A probe-less workload that keeps initializing past 20s otherwise gets checkpointed before it is ready, capturing a dead golden. - atenet: resume / route / ext_proc timeouts are env-configurable via ATE_RESUME_TIMEOUT_SECONDS / ATE_ROUTE_TIMEOUT_SECONDS / ATE_EXTPROC_TIMEOUT_SECONDS (defaults 15s / 10s / 5s, unchanged; see timeouts.go). Cold restore-on-demand of a large snapshot (tens of MiB) can take tens of seconds and the default ceilings cancel the in-flight restore, surfacing as a 504. Long LLM turns likewise exceed the 10s route timeout. - ateom-gvisor: `runsc start -allow-connected-on-save` is gated on runtime capability detection. Some GKE-Sandbox runsc builds do not define the flag and reject `runsc start` ("flag provided but not defined"), failing every actor start on those builds. ateom probes `runsc help start` once per binary path and passes the flag only when that build accepts it; a probe that cannot run assumes it is supported, preserving prior behavior. Detection replaces an env-var opt-out that was unreachable in practice — the ateom container's env is built from a fixed list in workerpool_apply.go, so no operator could set it. Invalid env values are logged and fall back to the default rather than being silently ignored. The atenet timeouts require a positive value: zero would mean no deadline at all on a route, an ext_proc round-trip, or a background resume, turning a stuck actor into a leaked goroutine instead of a 504. The golden warmup accepts zero, since it is a delay rather than a deadline and zero means "snapshot as soon as the golden actor resumes". Adds table-driven tests for each knob (unset / valid / zero / negative / garbage) and for the runsc capability probe, and documents the knobs as commented-out env entries in the ate-controller and atenet-router manifests. The atenet timeout bumps are stopgaps for the connected-socket suspend/restore problem tracked in #465 (suspend-safe actor networking), which should remove most of the need to tune them. --- .../controllers/actortemplate_controller.go | 44 +++++++- .../actortemplate_controller_test.go | 45 +++++++- cmd/atenet/internal/router/resumer.go | 7 +- cmd/atenet/internal/router/timeouts.go | 89 +++++++++++++++ cmd/atenet/internal/router/timeouts_test.go | 81 ++++++++++++++ cmd/atenet/internal/router/xds.go | 18 +++- cmd/ateom-gvisor/runsc.go | 60 ++++++++++- cmd/ateom-gvisor/runsc_test.go | 101 ++++++++++++++++++ manifests/ate-install/ate-controller.yaml | 10 ++ manifests/ate-install/atenet-router.yaml | 11 ++ 10 files changed, 452 insertions(+), 14 deletions(-) create mode 100644 cmd/atenet/internal/router/timeouts.go create mode 100644 cmd/atenet/internal/router/timeouts_test.go create mode 100644 cmd/ateom-gvisor/runsc_test.go diff --git a/cmd/atecontroller/internal/controllers/actortemplate_controller.go b/cmd/atecontroller/internal/controllers/actortemplate_controller.go index f5778fc5d..7da976b93 100644 --- a/cmd/atecontroller/internal/controllers/actortemplate_controller.go +++ b/cmd/atecontroller/internal/controllers/actortemplate_controller.go @@ -17,6 +17,8 @@ package controllers import ( "context" "fmt" + "os" + "strconv" "time" "github.com/agent-substrate/substrate/internal/resources" @@ -30,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" ) const ( @@ -41,9 +44,42 @@ const ( // a readiness probe. Templates whose containers all declare readyz skip // this wait — ResumeActor only returns once readyz reports 200, so the // workload is already initialized by the time we get here. + // + // The default can be overridden with the ATE_GOLDEN_WARMUP_SECONDS env + // var: workloads with a long, probe-less initialization (e.g. a heavy + // multi-process runtime that keeps warming up well past the 20s default) + // otherwise get checkpointed before they are ready. goldenSnapshotWarmup = 20 * time.Second + + // goldenWarmupEnv overrides goldenSnapshotWarmup with a whole number of + // seconds when set to a valid, non-negative integer. + goldenWarmupEnv = "ATE_GOLDEN_WARMUP_SECONDS" ) +// defaultGoldenWarmup returns the golden-snapshot warmup delay, honoring the +// ATE_GOLDEN_WARMUP_SECONDS env override and falling back to goldenSnapshotWarmup +// when the var is unset or invalid. +// +// Zero is accepted here, unlike the atenet timeout knobs, which require a +// positive value. This is a delay, not a deadline: 0 means "snapshot as soon as +// the golden actor resumes", which is exactly what a template with readyz on +// every container already does (see goldenSnapshotWarmupFor). A zero timeout, by +// contrast, would mean "no bound at all" and is rejected there. Negative and +// unparseable values are rejected in both places and fall back to the default. +func defaultGoldenWarmup(ctx context.Context) time.Duration { + v := os.Getenv(goldenWarmupEnv) + if v == "" { + return goldenSnapshotWarmup + } + secs, err := strconv.Atoi(v) + if err != nil || secs < 0 { + log.FromContext(ctx).Info("Ignoring invalid golden warmup override; expected a non-negative whole number of seconds", + "env", goldenWarmupEnv, "value", v, "using", goldenSnapshotWarmup) + return goldenSnapshotWarmup + } + return time.Duration(secs) * time.Second +} + type ActorTemplateReconciler struct { client.Client Scheme *runtime.Scheme @@ -136,7 +172,7 @@ func (r *ActorTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Reques } at.Status.Phase = atev1alpha1.PhaseWaitGoldenActor - at.Status.TakeGoldenSnapshotAt = metav1.NewTime(time.Now().Add(goldenSnapshotWarmupFor(at))) + at.Status.TakeGoldenSnapshotAt = metav1.NewTime(time.Now().Add(goldenSnapshotWarmupFor(ctx, at))) if err := r.Status().Update(ctx, at); err != nil { return ctrl.Result{}, err } @@ -195,14 +231,14 @@ func (r *ActorTemplateReconciler) SetupWithManager(mgr ctrl.Manager) error { // a readyz probe (so ResumeActor already blocked until the workload reported // 200), and the default warmup otherwise. A template with no containers // keeps the default — there is nothing to gate on. -func goldenSnapshotWarmupFor(at *atev1alpha1.ActorTemplate) time.Duration { +func goldenSnapshotWarmupFor(ctx context.Context, at *atev1alpha1.ActorTemplate) time.Duration { containers := at.Spec.Containers if len(containers) == 0 { - return goldenSnapshotWarmup + return defaultGoldenWarmup(ctx) } for i := range containers { if containers[i].Readyz == nil { - return goldenSnapshotWarmup + return defaultGoldenWarmup(ctx) } } return 0 diff --git a/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go b/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go index 00d736cf5..c3ab5acbf 100644 --- a/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go +++ b/cmd/atecontroller/internal/controllers/actortemplate_controller_test.go @@ -17,6 +17,7 @@ package controllers import ( "context" "testing" + "time" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc" @@ -31,7 +32,49 @@ import ( atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" ) +func TestDefaultGoldenWarmup(t *testing.T) { + tests := []struct { + name string + env string // "" means leave ATE_GOLDEN_WARMUP_SECONDS unset + want time.Duration + }{ + {name: "unset uses default", env: "", want: goldenSnapshotWarmup}, + {name: "valid override", env: "120", want: 120 * time.Second}, + {name: "zero snapshots immediately", env: "0", want: 0}, + {name: "negative falls back", env: "-5", want: goldenSnapshotWarmup}, + {name: "garbage falls back", env: "20s", want: goldenSnapshotWarmup}, + {name: "float falls back", env: "1.5", want: goldenSnapshotWarmup}, + {name: "whitespace falls back", env: " 30 ", want: goldenSnapshotWarmup}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(goldenWarmupEnv, tt.env) + if got := defaultGoldenWarmup(t.Context()); got != tt.want { + t.Errorf("defaultGoldenWarmup() = %v, want %v", got, tt.want) + } + }) + } +} + +// A template with readyz on every container skips the warmup regardless of the +// env override — the probe already proved the workload is up. +func TestGoldenSnapshotWarmupForIgnoresEnvWhenReadyzPresent(t *testing.T) { + t.Setenv(goldenWarmupEnv, "300") + at := &atev1alpha1.ActorTemplate{ + Spec: atev1alpha1.ActorTemplateSpec{Containers: []atev1alpha1.Container{ + {Name: "a", Readyz: &atev1alpha1.ContainerReadyz{HTTPGet: &atev1alpha1.HTTPGetAction{Port: 80}}}, + }}, + } + if got := goldenSnapshotWarmupFor(t.Context(), at); got != 0 { + t.Errorf("goldenSnapshotWarmupFor = %v, want 0", got) + } +} + func TestGoldenSnapshotWarmupFor(t *testing.T) { + // Keep the non-readyz cases pinned to goldenSnapshotWarmup even if the + // developer's shell exports an override. + t.Setenv(goldenWarmupEnv, "") + probe := &atev1alpha1.ContainerReadyz{ HTTPGet: &atev1alpha1.HTTPGetAction{Port: 80}, } @@ -83,7 +126,7 @@ func TestGoldenSnapshotWarmupFor(t *testing.T) { at := &atev1alpha1.ActorTemplate{ Spec: atev1alpha1.ActorTemplateSpec{Containers: tt.containers}, } - got := goldenSnapshotWarmupFor(at) + got := goldenSnapshotWarmupFor(t.Context(), at) if tt.wantZero && got != 0 { t.Errorf("goldenSnapshotWarmupFor = %v, want 0", got) } diff --git a/cmd/atenet/internal/router/resumer.go b/cmd/atenet/internal/router/resumer.go index db2231c06..cdfbfcaa6 100644 --- a/cmd/atenet/internal/router/resumer.go +++ b/cmd/atenet/internal/router/resumer.go @@ -55,7 +55,12 @@ func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorName stri // We detach the context from the first caller using a fixed background timeout. // This guarantees that if Caller 1 disconnects or times out, the underlying // resume operation continues running for Caller 2 and Caller 3 without failing. - bgCtx, bgCancel := context.WithTimeout(context.Background(), 15*time.Second) + // + // The ceiling must cover a cold restore of a large snapshot: a ~60 MiB + // gVisor restore can exceed the default 15s, and a shorter timeout cancels + // the in-flight restore, surfacing as a 504. Configurable via + // ATE_RESUME_TIMEOUT_SECONDS (see timeouts.go). + bgCtx, bgCancel := context.WithTimeout(context.Background(), resumeTimeout()) defer bgCancel() backoff := wait.Backoff{ diff --git a/cmd/atenet/internal/router/timeouts.go b/cmd/atenet/internal/router/timeouts.go new file mode 100644 index 000000000..cb72d10b4 --- /dev/null +++ b/cmd/atenet/internal/router/timeouts.go @@ -0,0 +1,89 @@ +// 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 ( + "log/slog" + "os" + "strconv" + "sync" + "time" +) + +// Resume / forwarding timeouts and their environment overrides. +// +// Resume-on-demand of a suspended actor performs a cold gVisor restore, which +// for a large snapshot (tens of MiB) can take tens of seconds. The default +// route / ext_proc / background-resume timeouts are sized for steady-state +// traffic and can cancel an in-flight restore, surfacing as a 504. These knobs +// let an operator running heavy actors raise the ceilings without a code change. +// +// Defaults are unchanged from prior behavior, so this is purely additive. +// +// Longer term, suspend-safe actor networking (agent-substrate/substrate#465) +// should remove most of the need to tune these. +const ( + resumeTimeoutEnv = "ATE_RESUME_TIMEOUT_SECONDS" + routeTimeoutEnv = "ATE_ROUTE_TIMEOUT_SECONDS" + extProcTimeoutEnv = "ATE_EXTPROC_TIMEOUT_SECONDS" + + defaultResumeTimeout = 15 * time.Second + defaultRouteTimeout = 10 * time.Second + defaultExtProcTimeout = 5 * time.Second +) + +// timeoutFromEnv returns the duration from a whole-seconds env var, falling back +// to def when the var is unset or not a positive integer. +// +// A timeout of zero is not accepted: for these three knobs it would mean "no +// deadline at all" on a route, an ext_proc round-trip, or a background resume, +// which turns a stuck actor into a leaked goroutine or a hung connection rather +// than a 504. Unparseable and non-positive values are rejected with a warning +// and the default is used, so a typo degrades to prior behavior instead of +// silently removing a bound. +func timeoutFromEnv(key string, def time.Duration) time.Duration { + v := os.Getenv(key) + if v == "" { + return def + } + secs, err := strconv.Atoi(v) + if err != nil || secs <= 0 { + slog.Warn("Ignoring invalid timeout override; expected a positive whole number of seconds", + slog.String("env", key), + slog.String("value", v), + slog.Duration("using", def)) + return def + } + return time.Duration(secs) * time.Second +} + +// The env vars are read once per process: they cannot change under a running +// container, xds.go re-reads the route / ext_proc timeouts on every xDS rebuild, +// and memoizing keeps an invalid-value warning to a single line. + +// resumeTimeout bounds a detached, background resume operation. +var resumeTimeout = sync.OnceValue(func() time.Duration { + return timeoutFromEnv(resumeTimeoutEnv, defaultResumeTimeout) +}) + +// routeTimeout bounds a forwarded upstream request (e.g. a long LLM turn). +var routeTimeout = sync.OnceValue(func() time.Duration { + return timeoutFromEnv(routeTimeoutEnv, defaultRouteTimeout) +}) + +// extProcTimeout bounds the ext_proc round-trip, which can drive a cold restore. +var extProcTimeout = sync.OnceValue(func() time.Duration { + return timeoutFromEnv(extProcTimeoutEnv, defaultExtProcTimeout) +}) diff --git a/cmd/atenet/internal/router/timeouts_test.go b/cmd/atenet/internal/router/timeouts_test.go new file mode 100644 index 000000000..ca21011e5 --- /dev/null +++ b/cmd/atenet/internal/router/timeouts_test.go @@ -0,0 +1,81 @@ +// 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 ( + "testing" + "time" +) + +// The exported accessors memoize with sync.OnceValue, so the parsing behavior is +// tested through timeoutFromEnv rather than through resumeTimeout et al. +func TestTimeoutFromEnv(t *testing.T) { + const key = "ATE_TEST_TIMEOUT_SECONDS" + const def = 15 * time.Second + + tests := []struct { + name string + env string // "" means leave the var unset + want time.Duration + }{ + {name: "unset uses default", env: "", want: def}, + {name: "valid override", env: "300", want: 300 * time.Second}, + {name: "one second", env: "1", want: time.Second}, + {name: "zero rejected", env: "0", want: def}, + {name: "negative rejected", env: "-1", want: def}, + {name: "garbage rejected", env: "300s", want: def}, + {name: "float rejected", env: "1.5", want: def}, + {name: "whitespace rejected", env: " 300 ", want: def}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(key, tt.env) + if got := timeoutFromEnv(key, def); got != tt.want { + t.Errorf("timeoutFromEnv(%q=%q) = %v, want %v", key, tt.env, got, tt.want) + } + }) + } +} + +// Guards against a knob being wired to the wrong env var or default. +func TestTimeoutDefaults(t *testing.T) { + tests := []struct { + name string + key string + def time.Duration + }{ + {name: "resume", key: resumeTimeoutEnv, def: defaultResumeTimeout}, + {name: "route", key: routeTimeoutEnv, def: defaultRouteTimeout}, + {name: "extproc", key: extProcTimeoutEnv, def: defaultExtProcTimeout}, + } + seen := map[string]bool{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if seen[tt.key] { + t.Errorf("env var %q is used by more than one timeout", tt.key) + } + seen[tt.key] = true + + t.Setenv(tt.key, "") + if got := timeoutFromEnv(tt.key, tt.def); got != tt.def { + t.Errorf("timeoutFromEnv(%q) = %v, want %v", tt.key, got, tt.def) + } + t.Setenv(tt.key, "300") + if got, want := timeoutFromEnv(tt.key, tt.def), 300*time.Second; got != want { + t.Errorf("timeoutFromEnv(%q=300) = %v, want %v", tt.key, got, want) + } + }) + } +} diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 5869805a8..82d5ca510 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -400,7 +400,11 @@ func (x *XdsServer) buildRoutes() *routev3.RouteConfiguration { ClusterSpecifier: &routev3.RouteAction_Cluster{ Cluster: "dynamic_forward_proxy_cluster", }, - Timeout: durationpb.New(10 * time.Second), + // Long-lived agent turns (e.g. an LLM streaming a + // response) can far exceed the default 10s route + // timeout and get cut off mid-turn. Configurable via + // ATE_ROUTE_TIMEOUT_SECONDS (see timeouts.go). + Timeout: durationpb.New(routeTimeout()), }, }, }, @@ -418,13 +422,19 @@ func (x *XdsServer) buildHcm(statPrefix string) *anypb.Any { ClusterName: ClusterName, }, }, - Timeout: durationpb.New(5 * time.Second), + // The ext_proc round-trip can drive a cold restore-on-demand of a + // suspended actor (a gVisor restore of a large snapshot), which may + // take tens of seconds; the default 5s timeout aborts the restore. + // Configurable via ATE_EXTPROC_TIMEOUT_SECONDS (see timeouts.go). + Timeout: durationpb.New(extProcTimeout()), }, MutationRules: &mutationrulesv3.HeaderMutationRules{ AllowAllRouting: &wrapperspb.BoolValue{Value: true}, }, - // Explicitly configure the message timeout to avoid the 200ms default - MessageTimeout: durationpb.New(5 * time.Second), + // Explicitly configure the message timeout to avoid the 200ms default. + // Sized to accommodate a cold restore-on-demand (see the GrpcService + // timeout above) rather than just the steady-state header exchange. + MessageTimeout: durationpb.New(extProcTimeout()), ProcessingMode: &extprocv3filter.ProcessingMode{ RequestHeaderMode: extprocv3filter.ProcessingMode_SEND, ResponseHeaderMode: extprocv3filter.ProcessingMode_SKIP, diff --git a/cmd/ateom-gvisor/runsc.go b/cmd/ateom-gvisor/runsc.go index 003bd861a..8955d6c66 100644 --- a/cmd/ateom-gvisor/runsc.go +++ b/cmd/ateom-gvisor/runsc.go @@ -17,6 +17,7 @@ package main import ( + "bytes" "context" "encoding/json" "fmt" @@ -25,6 +26,7 @@ import ( "os" "os/exec" "path/filepath" + "sync" specs "github.com/opencontainers/runtime-spec/specs-go" @@ -113,15 +115,60 @@ func (r *runsc) cmdCreate(ctx context.Context, out io.Writer, containerName stri return nil } +// allowConnectedOnSaveFlag lets runsc checkpoint a sandbox that still has open +// connected sockets. Not every build defines it: some releases reject it on +// `runsc start` with "flag provided but not defined" and handle connected +// sockets via SaveRestoreNetstack instead. Passing it unconditionally makes +// `runsc start` fail outright on those builds, so it is probed for below. +const allowConnectedOnSaveFlag = "allow-connected-on-save" + +// runscFlagSupport memoizes capability probes keyed by runsc binary path. +// Probing shells out, and cmdStart runs on every container start. +var runscFlagSupport sync.Map // map[string]bool + +// supportsAllowConnectedOnSave reports whether the runsc binary at path defines +// -allow-connected-on-save, probing it once per path. Detecting the capability +// keeps ateom working across runsc builds without operator configuration. +func supportsAllowConnectedOnSave(ctx context.Context, path string) bool { + if v, ok := runscFlagSupport.Load(path); ok { + return v.(bool) + } + supported := probeAllowConnectedOnSave(ctx, path) + runscFlagSupport.Store(path, supported) + return supported +} + +// probeAllowConnectedOnSave shells out to `runsc help start` and looks for the +// flag in the usage text. A probe that cannot run assumes the flag is present, +// preserving the previous unconditional behavior. +func probeAllowConnectedOnSave(ctx context.Context, path string) bool { + cmd := exec.CommandContext(ctx, path, "help", "start") + // Usage goes to stdout on some builds and stderr on others; capture both. + var usage bytes.Buffer + cmd.Stdout = &usage + cmd.Stderr = &usage + + if err := cmd.Run(); err != nil { + slog.WarnContext(ctx, "Could not probe runsc for -"+allowConnectedOnSaveFlag+"; assuming it is supported", + slog.String("runsc", path), + slog.Any("error", err)) + return true + } + + supported := bytes.Contains(usage.Bytes(), []byte(allowConnectedOnSaveFlag)) + slog.InfoContext(ctx, "Probed runsc for -"+allowConnectedOnSaveFlag, + slog.String("runsc", path), + slog.Bool("supported", supported)) + return supported +} + func (r *runsc) cmdStart(ctx context.Context, out io.Writer, containerName string) error { reapLock.RLock() defer reapLock.RUnlock() slog.InfoContext(ctx, "About to run runsc start", slog.String("container", containerName)) - cmd := exec.CommandContext( - ctx, - r.path, + args := []string{ "-log-format", "json", "--alsologtostderr", // "-debug", @@ -129,11 +176,16 @@ func (r *runsc) cmdStart(ctx context.Context, out io.Writer, containerName strin // "-debug-to-user-log", // "-log-packets", // "-strace", - "-allow-connected-on-save", + } + if supportsAllowConnectedOnSave(ctx, r.path) { + args = append(args, "-"+allowConnectedOnSaveFlag) + } + args = append(args, "-root", ateompath.RunSCStateDir(r.actorUID), "start", containerName, // Name of the container ) + cmd := exec.CommandContext(ctx, r.path, args...) cmd.Stdout = out cmd.Stderr = out diff --git a/cmd/ateom-gvisor/runsc_test.go b/cmd/ateom-gvisor/runsc_test.go new file mode 100644 index 000000000..863ac6c18 --- /dev/null +++ b/cmd/ateom-gvisor/runsc_test.go @@ -0,0 +1,101 @@ +//go:build linux + +// 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 main + +import ( + "os" + "path/filepath" + "testing" +) + +// fakeRunsc writes an executable stub at a unique path and returns it. Each stub +// gets its own path so the supportsAllowConnectedOnSave memo cannot leak between +// test cases. +func fakeRunsc(t *testing.T, script string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "runsc") + if err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o755); err != nil { + t.Fatalf("writing fake runsc: %v", err) + } + return path +} + +func TestProbeAllowConnectedOnSave(t *testing.T) { + tests := []struct { + name string + script string + want bool + }{ + { + name: "flag in stdout usage", + script: "echo ' -allow-connected-on-save allow checkpoint with connected sockets'\n", + want: true, + }, + { + name: "flag in stderr usage", + script: "echo ' -allow-connected-on-save' >&2\n", + want: true, + }, + { + name: "flag absent from usage", + script: "echo ' -detach detach from the container'\n", + want: false, + }, + { + // A build that cannot be probed is assumed to support the flag, + // preserving the behavior from before capability detection. + name: "probe failure assumes supported", + script: "echo 'unknown command' >&2\nexit 1\n", + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := probeAllowConnectedOnSave(t.Context(), fakeRunsc(t, tt.script)) + if got != tt.want { + t.Errorf("probeAllowConnectedOnSave = %v, want %v", got, tt.want) + } + }) + } +} + +func TestProbeAllowConnectedOnSaveMissingBinary(t *testing.T) { + missing := filepath.Join(t.TempDir(), "does-not-exist") + if !probeAllowConnectedOnSave(t.Context(), missing) { + t.Error("probeAllowConnectedOnSave on a missing binary = false, want true (fail open)") + } +} + +// The probe shells out, and cmdStart runs on every container start, so the +// result must be cached per binary path. +func TestSupportsAllowConnectedOnSaveMemoizes(t *testing.T) { + path := fakeRunsc(t, "echo ' -"+allowConnectedOnSaveFlag+"'\n") + t.Cleanup(func() { runscFlagSupport.Delete(path) }) + + if !supportsAllowConnectedOnSave(t.Context(), path) { + t.Fatal("first call = false, want true") + } + + // Replacing the stub with one that reports no such flag must not change the + // answer: a cached result means the binary is never re-probed. + if err := os.WriteFile(path, []byte("#!/bin/sh\necho ' -detach'\n"), 0o755); err != nil { + t.Fatalf("rewriting fake runsc: %v", err) + } + if !supportsAllowConnectedOnSave(t.Context(), path) { + t.Error("second call = false, want the memoized true") + } +} diff --git a/manifests/ate-install/ate-controller.yaml b/manifests/ate-install/ate-controller.yaml index a6955bb81..b7d275658 100644 --- a/manifests/ate-install/ate-controller.yaml +++ b/manifests/ate-install/ate-controller.yaml @@ -91,6 +91,16 @@ spec: args: - --ateapi-ca-file=/run/servicedns-ca/trust-bundle.pem - --ateapi-client-cert=/run/podidentity.podcert.ate.dev/credential-bundle.pem + # Delay between resuming the golden actor and checkpointing it, for + # templates whose containers do not all declare a readyz probe. Raise it + # for workloads with a long, probe-less startup that would otherwise be + # snapshotted mid-initialization. Whole seconds, non-negative (0 means + # snapshot immediately); anything else is ignored with a warning. + # Prefer adding readyz to the ActorTemplate over tuning this — templates + # with readyz on every container skip the wait entirely. + # env: + # - name: ATE_GOLDEN_WARMUP_SECONDS # default 20 + # value: "120" ports: - name: metrics containerPort: 8080 diff --git a/manifests/ate-install/atenet-router.yaml b/manifests/ate-install/atenet-router.yaml index 8fe146561..6411a30a5 100644 --- a/manifests/ate-install/atenet-router.yaml +++ b/manifests/ate-install/atenet-router.yaml @@ -157,6 +157,17 @@ spec: value: k8s.namespace.name=$(POD_NAMESPACE),k8s.pod.name=$(POD_NAME),k8s.pod.uid=$(POD_UID),service.instance.id=$(POD_UID) - name: OTEL_EXPORTER_OTLP_ENDPOINT value: http://opentelemetry-collector.gke-managed-otel.svc.cluster.local:4317 + # Resume-on-demand of a suspended actor performs a cold gVisor restore, + # which for a large snapshot (tens of MiB) can take tens of seconds and + # blow through the defaults below, surfacing as a 504. Uncomment and + # raise these when running heavy actors. Values are whole seconds and + # must be positive; anything else is ignored with a warning. + # - name: ATE_RESUME_TIMEOUT_SECONDS # background resume; default 15 + # value: "300" + # - name: ATE_ROUTE_TIMEOUT_SECONDS # forwarded upstream request; default 10 + # value: "300" + # - name: ATE_EXTPROC_TIMEOUT_SECONDS # ext_proc round-trip, drives the restore; default 5 + # value: "300" ports: - name: xds containerPort: 18000