docs(operator): separate the retry budget from the parking decision

CodeRabbit review on #402. The comments conflated two decisions the code
deliberately keeps apart: backoffLimit counts Failed pods, but exhausting
it only takes the Job terminal — the stage parks as erroring solely when a
retained attempt genuinely failed.

A Failed pod falls into one of three classes: an ignored disruption spends
nothing; an attempt the kubelet refused to admit spends an attempt without
being the package's failure; a genuine step failure or per-attempt timeout
spends an attempt and is. Only the third parks. Say so in the options
docstring, manager.yaml, values.yaml, and the design doc's backoffLimit
paragraph, which still claimed only genuine failures could count at all.

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>

Author: ayuskauskas

chart/values.yaml

@@ -90,10 +90,11 @@ controllerManager:
       ## as the only limit.
       jobStageTimeout: "1h"
       ## jobBackoffLimit is how many retries a package stage gets after its first attempt before
-      ## its Job goes terminal and the stage parks as erroring, awaiting a rerun/reset/config
-      ## change — so a stage runs at most jobBackoffLimit+1 times. It bounds every failure class,
-      ## not just timeouts, so raising it buys a crash-looping package more time to self-heal.
-      ## "0" gives a stage a single attempt with no retry.
+      ## its Job goes terminal — so a stage runs at most jobBackoffLimit+1 times. If a retained
+      ## attempt genuinely failed the stage then parks as erroring, awaiting a rerun/reset/config
+      ## change; otherwise it simply re-runs. It bounds every failure class, not just timeouts,
+      ## so raising it buys a crash-looping package more time to self-heal. "0" gives a stage a
+      ## single attempt with no retry.
       jobBackoffLimit: "3"
       ## runtimeRequiredTaint: This feature assumes nodes are added to the cluster with `--register-with-taints` kubelet flag.
       ## This taint is assume to be all new nodes, and skyhook pods will tolerate this taint, and remove it one the nodes packages are complete.

docs/designs/2026-07-10-package-execution-as-jobs.md

@@ -110,7 +110,7 @@ Three things about this shape are non-obvious:
 - **`restartPolicy: Never` on package Jobs** makes each attempt a fresh pod, so a failure survives as a full-log archive pod instead of being destroyed by an in-place restart. Interrupt Jobs are the exception — they keep `OnFailure`, because a reboot interrupt kills its own pod by design; under `Never` every successful reboot would mint a spurious failed attempt, whereas in-place restart after the node returns is the proven recovery shape (the agent skips the already-done interrupt via its resource-id flag file).
 - **The unbounded not-ready/unreachable tolerations** stop the taint manager from evicting a node-pinned pod when a reboot-class interrupt keeps the node NotReady past the default eviction timeout. These pods are node-bound host agents — running them anywhere else is meaningless, so eviction is never useful. A node that stays NotReady forever holds the Job Active exactly as today's raw pod would; a *removed* node is handled by the orphaned-node sweep.
 
-`backoffLimit` is finite (`JOB_BACKOFF_LIMIT`, default 3): under `restartPolicy: Never` it counts Failed pods, and the `Ignore`-on-`DisruptionTarget` rule means only *genuine* step failures count, not disruptions. The retry loop lives inside the Job controller — attempt fails → the operator archives the Failed pod → the Job controller paces the next attempt under its own backoff — so the operator never drives recreation, and there is no operator-side retry counter to persist. `podReplacementPolicy: Failed` guarantees the replacement lands only after the previous pod has fully terminated, so two executors never overlap on the shared hostPath mounts.
+`backoffLimit` is finite (`JOB_BACKOFF_LIMIT`, default 3): under `restartPolicy: Never` it counts Failed pods, and the `Ignore`-on-`DisruptionTarget` rule keeps disruptions out of that count. Counting and *parking* are then two different questions, and a Failed pod falls into one of three classes: an ignored disruption spends nothing; an attempt the kubelet refused to admit spends an attempt but is not the package's failure; a genuine step failure or per-attempt timeout spends an attempt and is. Exhausting the budget therefore takes the Job terminal, but only the third class parks the stage — see Retry and the failed-attempt archive. The retry loop lives inside the Job controller — attempt fails → the operator archives the Failed pod → the Job controller paces the next attempt under its own backoff — so the operator never drives recreation, and there is no operator-side retry counter to persist. `podReplacementPolicy: Failed` guarantees the replacement lands only after the previous pod has fully terminated, so two executors never overlap on the shared hostPath mounts.
 
 Interrupt Jobs are the exception and keep an unbounded limit. Under `OnFailure` the limit counts container *restarts* rather than failed pods, so a finite budget would be spent by the in-place restart that *is* the reboot recovery.
 

operator/config/manager/manager.yaml

@@ -112,7 +112,8 @@ spec:
           # package-stage Jobs by outcome; JobStageTimeout is the default per-attempt
           # deadline when a package sets no stageTimeout (0 removes the time bound);
           # JobBackoffLimit is how many retries a package stage gets after its first attempt
-          # before it parks, so a stage runs at most JobBackoffLimit+1 times.
+          # before its Job goes terminal, so a stage runs at most JobBackoffLimit+1 times; it
+          # parks as erroring only if a retained attempt genuinely failed.
           - name: JOB_TTL_SUCCEEDED
             value: "1h"
           - name: JOB_TTL_FAILED

operator/internal/controller/skyhook_controller.go

@@ -149,8 +149,11 @@ type JobOperatorOptions struct {
 	// outcome so failure logs outlive success logs; JobStageTimeout is the default
 	// per-attempt deadline for a package stage Job when the package sets no stageTimeout
 	// (0 removes the time bound); JobBackoffLimit is how many *retries* a package stage gets
-	// after its first attempt before the Job goes terminal and the stage parks, so the stage
-	// runs at most JobBackoffLimit+1 times (0 means a single attempt, no retry).
+	// after its first attempt before its Job goes terminal, so the stage runs at most
+	// JobBackoffLimit+1 times (0 means a single attempt, no retry). Exhausting the budget is
+	// not by itself a park: the stage parks as erroring only if a retained attempt genuinely
+	// failed, since attempts the kubelet refused to admit spend the budget without ever
+	// running the package.
 	JobTTLSucceeded time.Duration `env:"JOB_TTL_SUCCEEDED, default=1h"`
 	JobTTLFailed    time.Duration `env:"JOB_TTL_FAILED, default=24h"`
 	JobStageTimeout time.Duration `env:"JOB_STAGE_TIMEOUT, default=1h"`