feat: readyz: make the overall wait timeout configurable per template

[Maya Wang (mayawang)]

Rescoped again. This PR previously proposed --golden-snapshot-warmup, a
tunable wall-clock delay before the golden checkpoint. Per discussion, that
direction is dropped: the answer for a workload that cannot report readiness
is a readiness endpoint — a small sidecar where the workload itself cannot be
changed — not a longer timer. What survives is the piece that discussion
agreed on, and which the previous revision already flagged as a follow-up:
making the readyz deadline itself configurable.

The warmup work is not in this branch. It is kept locally in case a workload
genuinely cannot be given a readiness signal before GA, and would come back as
its own PR if so.

Summary

readyz.Wait polls until the container returns 200 or a hardcoded 30s
elapses. A workload that legitimately takes longer to bind its HTTP server
cannot be accommodated without raising the ceiling for every actor in the
cluster, and losing that race fails the actor start.

How long a workload takes to become ready is a property of that workload, so
this makes the deadline a per-template setting rather than a package constant.

Adds optional timeoutSeconds to ContainerReadyz. Unset keeps today's 30s,
so no existing template changes behavior.

Changes

The value rides on the existing probe, so it follows the chain the probe already
takes and no call site needs to know about it:

ContainerReadyz.timeoutSeconds → toAteletReadyz → ateletpb.Readyz →
toAteomReadyz → ateompb.Readyz → readyz.Wait

• pkg/api/v1alpha1/actortemplate_types.go — TimeoutSeconds *int32,
  +optional, Minimum=1, Maximum=3600.
• internal/proto/ateletpb/atelet.proto, internal/proto/ateompb/ateom.proto —
  int32 timeout_seconds = 2 on both Readyz messages.
• cmd/ateapi/internal/controlapi/workload_spec.go, cmd/atelet/main.go — pass
  it through the two conversions.
• internal/readyz/readyz.go — OverallTimeout becomes
  DefaultOverallTimeout (still 30s) and Wait resolves its deadline through a
  new overallTimeout(probe) helper.
• Regenerated: both .pb.go, zz_generated.deepcopy.go, and the
  actortemplates CRD.

None of the four readyz.WaitAll call sites change.

On the zero value. Unlike a warmup delay — where zero is a real request
meaning "checkpoint immediately" — a zero readiness deadline could never be met,
so it is never something a template author means. A non-positive value on the
wire is therefore read as "unset" and falls back to the default, and the CRD
field is a pointer with Minimum=1 so the API rejects 0 outright rather than
silently substituting 30s behind the author's back.

On bounding, which was the open question left on the previous revision:
bounded at 3600. A template asking to wait longer than an hour for readiness
is expressing a broken workload, not a slow one, and the bound keeps a typo from
pinning a worker for a day.

Verification

• go build ./..., go vet ./..., gofmt, go test ./... — all pass.

• internal/readyz/readyz_test.go — overallTimeout resolves unset and
  negative to the default and honors an explicit value; Wait against a port
  nothing binds gives up at the probe's 1s deadline rather than the 30s default.

• workload_spec_test.go, cmd/atelet/main_test.go — the timeout crosses both
  conversions, and a probe without one stays zero on the wire.

• actortemplate_validation_test.go — the bounds are enforced by a real API
  server. This suite runs under envtest against the generated CRD directory, so
  it exercises the regenerated actortemplates CRD rather than the Go markers:
  300 is accepted, unset is accepted, and 0, -1 and 3601 are all
  rejected by apiserver schema validation.

• On a real cluster, via CI. internal/e2e/fixtures/probe now declares a
  readyz probe with timeoutSeconds: 60, pointed at the /healthz the probe
  binary already serves on :80. The kind e2e that runs on every PR therefore
  exercises the value crossing ateapi → atelet → ateom on real binaries, across
  the auth matrix, on both the run and restore paths. This is also the readyz
  path's first e2e coverage — no fixture declared a probe before.

Wire compatibility degrades safely in both skew directions: timeout_seconds is
a new field 2 on a Readyz message that has only ever had field 1, so an old
ateom ignores it and an old ateapi leaves it zero, which reads as the 30s
default.

No GKE run. What that would add over the above is a workload whose readiness
genuinely exceeds 30s, and that is the readiness-sidecar work rather than this
PR.

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

• Tests pass
• Appropriate changes to documentation are included in the PR

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[Dmitry Berkovich (dberkov)]

Maya Wang (@mayawang) - I have recently added readyz to the container template -

substrate/pkg/api/v1alpha1/actortemplate_types.go

Line 139 in 0f6144e

type ContainerReadyz struct {

. Today it is just action and actually timeout defined as a contant here -

substrate/internal/readyz/readyz.go

Line 42 in 0f6144e

OverallTimeout = 30 * time.Second

.

Have you considered to extend the readyz with a timeout via actorTemplate and push it up to the atelet?

[Maya Wang (mayawang)]

Maya Wang (Maya Wang (@mayawang)) - I have recently added readyz to the container template -

substrate/pkg/api/v1alpha1/actortemplate_types.go

Line 139 in 0f6144e

type ContainerReadyz struct {

. Today it is just action and actually timeout defined as a contant here -

substrate/internal/readyz/readyz.go

Line 42 in 0f6144e

OverallTimeout = 30 * time.Second

.
Have you considered to extend the readyz with a timeout via actorTemplate and push it up to the atelet?

Thanks Dmitry Berkovich (@dberkov) — agreed, readyz is the better mechanism. One note on how it fits with this PR: goldenSnapshotWarmupFor() here already returns 0 when every container declares readyz, so the timer is skipped entirely and ATE_GOLDEN_WARMUP_SECONDS is only the fallback for probe-less templates.

Strong +1 on making the timeout configurable — and it's live for us, not hypothetical. The Hermes actor we're onboarding declares readyz (httpGet /health) and takes ~20s to golden-warm against the 30s ceiling. It fits today, but that number grows as we add tools/MCP to the image, and there's no knob to turn: once a probe is declared, ATE_GOLDEN_WARMUP_SECONDS no longer applies.

The failure mode is harsher than the timer's, too. A WaitAll error propagates out of RunWorkload/RestoreWorkload (cmd/ateom-gvisor/main.go:230,430), so a too-short probe hard-fails create and every restore, rather than just capturing an early golden. (The probe-less case that motivated the env fallback here is a different workload — a multi-process Node.js agent that needs ~30s to initialize and reports healthy too early to gate on.)

Shape I'd propose: optional TimeoutSeconds on ContainerReadyz, default 30 so behavior is unchanged, plumbed ateletpb.Readyz → ateompb.Readyz into readyz.Wait.

I'd keep it as a separate PR rather than folding it in — this one is internal-only, and adding a v1alpha1 field pulls in codegen and API review. The two don't overlap, so they can land in either order. Happy to take it since I have the workload to validate against, unless you'd rather own it as your API — either way I'd want your input on the field bounds.

[Maya Wang (mayawang)]

Dmitry Berkovich (@dberkov) — this has been reshaped since your review, so rather than have you re-read
from memory, here's what actually changed. Would appreciate another pass when you have
time.

Two of the four knobs are gone, not rebased. Request parking landed on main and covers that ground properly: the resume timeout is now failFastResumeBudget plus --parked-request-budget, and the ext_proc timeout derives from the park budget in SetExtProcMessageTimeout. resumer.go isn't touched at all anymore, so there's no overlap with that work.

The two survivors moved from env vars to flags, matching the convention parking established. One correction to my comment abve: ATE_GOLDEN_WARMUP_SECONDS no longer exists. It's --golden-snapshot-warmup on atecontroller (cmd/atecontroller/main.go:57, default 20s). The semantics we discussed are unchanged — goldenSnapshotWarmupFor() still returns 0 the moment every container declares readyz, so it remains the probe-less fallback and never competes with a declared probe. The other knob is --route-timeout on atenet-router.

The runsc opt-out became an automatic capability probe, and that turned up a real bug worth flagging since it's the one change with no e2e coverage. The probe originally shelled runsc help start and grepped the usage — which can never match: -allow-connected-on-save is a top-level flag, and the per-subcommand usage lists only -h/-help even on builds that define it. It would have reported "unsupported" on every build and silently stopped passing the flag where it does work. It's
runsc flags now, checked against five real runsc builds (two that reject the flag, three that accept it — the probe's verdict matches ground truth on all five), with a test pinning the argv because the stubs answer regardless of what they're passed.

Rebased onto current main, so this is now post-atunnel. Relevant to the route timeout: it attaches to the actor_original_dst route that replaced the dynamic_forward_proxy path, and the test pins that cluster name so that if actor traffic ever moves to a different route this fails loudly instead of leaving the timeout governing a route nothing uses. Caveat on the evidence — my /config_dump reading of 10s → 300s was taken before atunnel landed, so it measured the old path. The route identity is pinned by test rather than re-measured; I'll re-read it once we have an atunnel-era worker to point at.

TimeoutSeconds on ContainerReadyz is still queued as its own PR, unchanged from what we landed on — and I'd still like your view on the field bounds before I write it.

[Dmitry Berkovich (dberkov)]

2 minor comments, otherwise LGTM

[Maya Wang (mayawang)]

2 minor comments, otherwise LGTM

Thanks — all addressed, PTAL.

One consequence worth flagging: with omitempty on a non-pointer, a Go client that sets 0 doesn't serialize the field, so it's read as unset and defaults to 30 rather than being rejected. A manifest that spells out 0 still hits Minimum=1. I dropped the envtest case for explicit zero since it can't be expressed through the typed client; the -1 case still covers the bound.