fix(validator)!: give the Job deadline headroom over the check budget

docs/contributor/validator.md

@@ -782,10 +782,15 @@ enumerates any RDMA-candidate node (with `nodesValidated=0` — nothing is
 certified mid-poll) and again at the terminal outcome.
 `parseExtraSentinels` keeps the last valid sentinel, so the terminal emit
 wins on a clean exit; the floor exists only so a cordoned-node narrowing
-still reaches the signed bundle if the Job's `activeDeadlineSeconds`
-SIGKILLs the process at the no-margin poll budget before the terminal emit
-runs (#1952). The gate stays fail-closed: "could not observe the fabric"
-reports `0` validated and never reads as ready.
+still reaches the signed bundle on the rarer path where the process is
+SIGKILLed before the terminal emit runs (#1952). The broader no-margin kill
+race this once described (#2473) is now bounded generally by
+`ValidatorJobDeadlineHeadroom`: the Job's `activeDeadlineSeconds` adds that
+headroom on top of the catalog `timeout:`, so an exhausted poll has margin to
+unwind and reach the terminal emit before the Job's SIGKILL, and this floor
+remains defense-in-depth for a probe call that blocks past its own
+poll-budget cancellation. The gate stays fail-closed: "could not observe the
+fabric" reports `0` validated and never reads as ready.
 
 Unlike `check-nvidia-smi`, the RDMA gate never *skips* — it either
 certifies the cohort or fails closed — so it mints no `skipReason`
@@ -1400,14 +1405,18 @@ but a readiness test copied from a validator check (`assert: 5m`) is
 now capped at the gate's 2m per evaluation rather than overrunning it.
 
 The `expected-resources` catalog timeout (8m in
-`recipes/validators/catalog.yaml`) is the **outer** envelope. It must
-exceed the longest in-tree `assert` value plus headroom for
-pre-chainsaw work, chainsaw teardown, and log flush
-(`defaults.JobEnvelopeMargin`). If assert runs too close to that
-catalog deadline, the Job can SIGKILL the pod before chainsaw reports
-the failing step — operators see truncated output instead of a useful
-failure. Raise the catalog `timeout` in tandem when you need a longer
-assert budget (`TestExpectedResourcesCatalogEnvelope` guards this).
+`recipes/validators/catalog.yaml`) is the **outer** envelope for the
+check's own budget (`AICR_CHECK_TIMEOUT`). It must exceed the longest
+in-tree `assert` value plus headroom for pre-chainsaw work, chainsaw
+teardown, and log flush (`defaults.JobEnvelopeMargin`). If assert runs
+too close to that catalog deadline, the check's own context can expire
+before chainsaw reports the failing step — operators see truncated
+output instead of a useful failure. The Job's `activeDeadlineSeconds`
+(this catalog timeout plus `defaults.ValidatorJobDeadlineHeadroom`) is
+a separate, later backstop and no longer the deadline this margin
+protects against. Raise the catalog `timeout` in tandem when you need
+a longer assert budget (`TestExpectedResourcesCatalogEnvelope` guards
+this).
 
 ## Constraint evaluation algorithm
 

docs/design/002-validatorv2-adr.md

@@ -110,7 +110,9 @@ Every validator container:
 
 Each validator runs as a K8s Job with:
 - `backoffLimit: 0` — no retries
-- `activeDeadlineSeconds` — from catalog `timeout`
+- `activeDeadlineSeconds` — catalog `timeout` plus `defaults.ValidatorJobDeadlineHeadroom`,
+  so the check's own budget expires first unless container startup outruns that
+  headroom (see Timeout and Termination below)
 - `terminationGracePeriodSeconds: 30` — time between SIGTERM and SIGKILL
 - `ttlSecondsAfterFinished: 3600` — 1 hour retention for debugging
 - `restartPolicy: Never`
@@ -143,17 +145,64 @@ Created once per run via Server-Side Apply, cleaned up at end.
 
 ### Timeout and Termination
 
-Three independent timeout layers protect against hangs:
-
-1. **Job `activeDeadlineSeconds`** (from catalog): K8s sends SIGTERM, then SIGKILL
-   after `terminationGracePeriodSeconds` (30s). Pod logs and termination message
-   remain available after termination.
-
-2. **Orchestrator wait timeout** (catalog timeout + 30s buffer): If the Job hasn't
-   reached a terminal state, the orchestrator captures whatever logs/status are
-   available and moves to the next validator.
-
-3. **Parent context timeout** (CLI flag): Cancels the entire phase if exceeded.
+Four timeout layers protect against hangs. The first three are ordered so the
+check's own budget is the tightest — the ordering that closes issue #2473 (see
+below). The three clocks do not all start at the same moment, and two of those
+offsets have put clock 3 ahead of an inner clock; only the first is still live.
+
+Container startup is the live one. Clock 1 begins when the validator
+container's first instruction runs, while clock 3 begins at the Job's start
+time, so scheduling plus image pull eats into the headroom, and a startup
+slower than `defaults.ValidatorJobDeadlineHeadroom` lets clock 3 fire first.
+Kubernetes then marks the Job `Failed/DeadlineExceeded` and deletes the still-active
+pod, which is the pre-#2473 behaviour: the phase still fails closed, but the
+check's own diagnosis is lost. No offset applied to clock 2 removes this path,
+because the check's context cannot start before its container does. The
+headroom is sized as `defaults.K8sPodReadyTimeout` plus margin to cover
+ordinary startup; it is a budget, not a guarantee.
+
+A slow create/apply response delays clock 2's start alone. Past
+`defaults.ValidatorWaitBuffer` of delay the rebased wait would go negative and
+`defaults.ValidatorMinCompletionWait` floors it, and a floor measured from that
+late a start used to run past clock 3 — the same lost-verdict outcome, reached
+by the other route. `v1.OrchestratorWaitFor` now bounds every result, floor
+included, at `defaults.ValidatorPreDeadlineMargin` short of clock 3 computed
+from the same start time, so a late response costs the wait its length rather
+than its ordering. Once clock 3 has already fired, the wait is only there to
+read the terminal Job the controller left behind.
+
+1. **Check budget** (catalog `timeout`, published as `AICR_CHECK_TIMEOUT`): the
+   validator's own parent context. A well-behaved check cancels and exits on
+   this deadline before Kubernetes ever intervenes.
+
+2. **Orchestrator wait timeout** (catalog timeout + `defaults.ValidatorWaitBuffer`,
+   2m30s): if the Job hasn't reached a terminal state by then, the orchestrator
+   captures whatever logs/status are available and moves to the next validator.
+   The window is measured from the Job's **observed start time** — `status.startTime`
+   when the apiserver has stamped it, otherwise `creationTimestamp` — not from the
+   moment the create/apply response reached the CLI, so it shares an origin with
+   clock 3 below. Anchored anywhere else, the real margin between clocks 2 and 3
+   collapses to `defaults.JobEnvelopeMargin` (60s), because clock 3 has already
+   been running for however long the apply response took: a response slower than
+   that lets clock 3 fire first and delete the still-active pod. `v1.OrchestratorWaitFor`
+   carries the derivation; it caps the result at the un-rebased budget (apiserver
+   clock skew can place the start time in the CLI's future), floors it at
+   `defaults.ValidatorMinCompletionWait`, and holds every result
+   `defaults.ValidatorPreDeadlineMargin` short of clock 3 below, measured from
+   that same start time.
+
+3. **Job `activeDeadlineSeconds`** (catalog timeout + `defaults.ValidatorJobDeadlineHeadroom`,
+   3m30s): K8s sends SIGTERM, then SIGKILL after `terminationGracePeriodSeconds` (30s),
+   as a backstop for a check that never exits on its own. Because the headroom keeps
+   this clock behind clocks 1 and 2, a self-terminated pod is no longer *active* by
+   the time this deadline could fire, and the Job controller's `deleteActivePods`
+   only deletes active pods — so the pod survives, `Failed`, with its logs and
+   termination message intact for extraction. Before this ordering existed, all
+   three clocks shared one catalog value and the Job's deadline — timed from Job
+   creation, ahead of the check's own deadline, timed from container start — fired
+   first and deleted the still-running pod, which is exactly what #2473 reported.
+
+4. **Parent context timeout** (CLI flag): Cancels the entire phase if exceeded.
 
 On any timeout, the orchestrator always attempts log capture with a fresh
 `context.Background()` context before cleanup, ensuring partial results are never
@@ -194,7 +243,7 @@ ValidateAll(ctx, recipe, snapshot)
 │   ├── For each validator (sequentially):
 │   │   ├── Deploy Job
 │   │   ├── Stream stderr (background, for live progress)
-│   │   ├── WaitForCompletion(timeout + 30s buffer)
+│   │   ├── WaitForCompletion() — until jobStart + timeout + ValidatorWaitBuffer
 │   │   ├── ExtractResult() — exit code, termination msg, stdout
 │   │   ├── Add to CTRF report
 │   │   └── CleanupJob() unless --no-cleanup

docs/integrator/validator-extension.md

@@ -244,7 +244,7 @@ ENTRYPOINT ["/check.sh"]
 
 - Must run as non-root (validator Jobs use `runAsNonRoot: true`)
 - Must handle the mounted data paths (`/data/snapshot/`, `/data/validation/`)
-- Should respect timeout — the Job has `activeDeadlineSeconds` set from the catalog entry
+- **Must** self-terminate within the catalog `timeout` (published as `AICR_CHECK_TIMEOUT`) rather than rely on the Job — the Job's `activeDeadlineSeconds` is set to the catalog entry plus a fixed headroom, as a backstop for a check that never exits on its own. Self-termination is what preserves the logs: a pod that exited on its own is no longer *active*, so the Job controller's `deleteActivePods` leaves it in place as `Failed` for the orchestrator to read, whereas a pod still running when the backstop fires is deleted along with its verdict
 - Should write meaningful evidence to stdout for the CTRF report
 - Must use explicit image tags (not `:latest`) for reproducibility in external catalogs
 

docs/user/validation.md

@@ -951,8 +951,8 @@ error codes (see [`pkg/errors/exitcode.go`](https://github.com/NVIDIA/aicr/blob/
 
 > **Important:** two subtleties to be aware of when gating a pipeline on exit code:
 >
-> 1. Both `failed` and `other` are blocking. A phase whose status is `other` — the check produced no usable verdict, e.g. a crash, an OOM, or a Job that failed for a non-deadline reason with its pod already gone — drives the same non-zero exit as `failed` — an inconclusive check fails closed rather than passing silently. `--fail-on-error=false` suppresses **both** result-driven exits (the run reports the outcomes in the CTRF report and exits 0); it does not distinguish `failed` from `other`. A validator Job killed on `activeDeadlineSeconds` normally reports `failed` rather than `other`: its terminal `Failed/DeadlineExceeded` condition is a verdict, so the result names the deadline instead of the missing pod. One narrow exception remains — the CLI waits `activeDeadlineSeconds` plus a 30s buffer for the Job to become terminal, and that buffer equals the validator pod's termination grace period, so a validator that consumes its full grace can exhaust the wait before the condition is stamped. Such a run falls back to the CLI-wait path and still reports `other`.
-> 2. Exit 5 is narrower than it sounds. A timeout **inside** a check's own logic (DynamoGraphDeployment not ready, inference endpoint never healthy, AIPerf Job pod-wait deadline) surfaces as a failed phase, not as a structured `ErrCodeTimeout`, so the CLI exits **8**. So does a per-validator wait deadline: `runPhase` hands that error to the timeout handler and records the outcome as a check result rather than propagating it, which is the same path the full-grace exception in note 1 takes. The rule, rather than a list of sources: exit 5 applies whenever an `ErrCodeTimeout` reaches the top-level CLI. A timeout that is converted into a check result instead — whether raised inside a check or by a per-validator wait — surfaces as exit 8.
+> 1. Both `failed` and `other` are blocking. A phase whose status is `other` — the check produced no usable verdict, e.g. a crash, an OOM, or a Job that failed for a non-deadline reason with its pod already gone — drives the same non-zero exit as `failed` — an inconclusive check fails closed rather than passing silently. `--fail-on-error=false` suppresses **both** result-driven exits (the run reports the outcomes in the CTRF report and exits 0); it does not distinguish `failed` from `other`. A validator Job killed on `activeDeadlineSeconds` normally reports `failed` rather than `other`: its terminal `Failed/DeadlineExceeded` condition is a verdict, so the result names the deadline instead of the missing pod. One narrow exception remains — the CLI waits the catalog timeout plus `ValidatorWaitBuffer` (2m30s) for the Job to become terminal — measured from the Job's observed start time (`status.startTime` when the apiserver has stamped it, otherwise `creationTimestamp`, which is never later and so only ever ends the wait earlier), so it shares an origin with the Job's clock — a window that is deliberately shorter than the Job's own `activeDeadlineSeconds` (catalog timeout plus `ValidatorJobDeadlineHeadroom`, 3m30s), so a validator whose pod is still running when the CLI's wait expires exhausts the wait before the deadline condition is ever stamped. Such a run falls back to the CLI-wait path and still reports `other`.
+> 2. Exit 5 is narrower than it sounds. A timeout **inside** a check's own logic (DynamoGraphDeployment not ready, inference endpoint never healthy, AIPerf Job pod-wait deadline) surfaces as a failed phase, not as a structured `ErrCodeTimeout`, so the CLI exits **8**. So does a per-validator wait deadline: `runPhase` hands that error to the timeout handler and records the outcome as a check result rather than propagating it, which is the same path the wait-exhaustion exception in note 1 takes. The rule, rather than a list of sources: exit 5 applies whenever an `ErrCodeTimeout` reaches the top-level CLI. A timeout that is converted into a check result instead — whether raised inside a check or by a per-validator wait — surfaces as exit 8.
 
 Scripts that gate on validation outcome should treat **any non-zero code** as
 failure rather than branching on specific values, and should additionally
@@ -1064,7 +1064,7 @@ strategy.
 
 ### Benchmark Job stuck or timed out
 
-Each performance check has a Job-level `activeDeadlineSeconds` set by the catalog's `timeout:`. For `inference-perf`, the full pipeline (workload ready → endpoint health → benchmark) can take up to 30 min on cold-start clusters. If it still times out:
+Each performance check has a Job-level `activeDeadlineSeconds` set by the catalog's `timeout:` plus `ValidatorJobDeadlineHeadroom` (3m30s) on top. For `inference-perf`, the full pipeline (workload ready → endpoint health → benchmark) can take up to 30 min on cold-start clusters. If it still times out:
 
 ```bash
 # validator orchestrator Job + AIPerf benchmark Job both live in aicr-validation.

pkg/client/v1/aicr.go

@@ -2118,7 +2118,8 @@ func (c *Client) ValidateState(
 	// Apply a facade-level deadline only as opted into by WithValidationTimeout,
 	// mirroring MakeBundle. The default (cfg.timeout == nil) keeps the
 	// ValidationOperationTimeout cap (75m), which sits ABOVE the largest
-	// per-check Job timeout (the 65m inference-perf catalog timeout; the
+	// rendered per-check Job deadline (the 65m inference-perf catalog timeout
+	// plus the 3m30s ValidatorJobDeadlineHeadroom, i.e. 68m30s; the
 	// CheckExecutionTimeout fallback is 55m), so a single hung check fires its
 	// own per-check timeout first and surfaces as a structured check failure
 	// rather than a wrapping deadline-exceeded that loses the per-check signal —