Merge pull request #402 from NVIDIA/jobs-migration/373-timeout-retry

feat(operator): treat a stage timeout as a retryable failure (#373)

Author: ayuskauskas

chart/RELEASE_NOTES.md

@@ -5,6 +5,21 @@ For the full commit-level log see CHANGELOG.md.
 
 ## Unreleased
 
+### New Features
+
+- **Package stage execution moved to `batch/v1` Jobs**, with four new operator
+  env values under `controllerManager.manager.env`:
+
+  | Value | Default | What it does |
+  | --- | --- | --- |
+  | `jobStageTimeout` | `"1h"` | Default per-attempt deadline for a package stage, when the package sets no `stageTimeout` of its own. `"0"` removes the time bound. |
+  | `jobBackoffLimit` | `"3"` | Retries after the first attempt before a stage is surfaced as `erroring` — so at most four attempts. |
+  | `jobTtlSucceeded` | `"1h"` | How long a succeeded stage Job (and its logs) is kept. Minimum `"1m"`. |
+  | `jobTtlFailed` | `"24h"` | How long a failed stage Job is kept, so failure logs outlive success logs. Minimum `"1m"`. |
+
+  The chart also gains `batch/job` RBAC for the operator; no values change is
+  required to upgrade.
+
 ### Bug Fixes
 
 - **Helm upgrade no longer fails on the immutable Deployment selector after the
@@ -32,6 +47,12 @@ For the full commit-level log see CHANGELOG.md.
 
 ### Upgrade notes
 
+- **A crash-looping package now gives up after ~70 seconds instead of retrying
+  for up to an hour.** `jobBackoffLimit` bounds retries per stage; if you rely on
+  a package self-healing through transient environment problems, raise it before
+  upgrading. See the operator release notes for the full behavior change.
+- **`jobTtlSucceeded` / `jobTtlFailed` must be at least `"1m"`.** Setting `"0"`
+  to disable retention makes the operator fail validation at startup.
 - No action is required: the selector-migration hook runs automatically on
   `helm upgrade` and only recreates the Deployment when its selector is stale.
   To perform the recreate manually instead, set `selectorMigration.enabled=false`

chart/templates/deployment.yaml

@@ -101,6 +101,8 @@ spec:
           value: {{ quote .Values.controllerManager.manager.env.jobTtlFailed }}
         - name: JOB_STAGE_TIMEOUT
           value: {{ quote .Values.controllerManager.manager.env.jobStageTimeout }}
+        - name: JOB_BACKOFF_LIMIT
+          value: {{ quote .Values.controllerManager.manager.env.jobBackoffLimit }}
         - name: RUNTIME_REQUIRED_TAINT
           value: {{ quote .Values.controllerManager.manager.env.runtimeRequiredTaint }}
         - name: PAUSE_IMAGE

chart/templates/nodewright-crd.yaml

@@ -526,10 +526,24 @@ spec:
                       type: object
                     stageTimeout:
                       description: |-
-                        StageTimeout bounds the wall-clock runtime of each of this package's stage Jobs
-                        (mapped to the Job's activeDeadlineSeconds), all retries and reboot time included.
-                        A stage that runs past it is failed and surfaced as erroring. Unset uses the
-                        operator default (JOB_STAGE_TIMEOUT); "0" disables the deadline for this package.
+                        StageTimeout bounds the wall-clock runtime of one attempt at each of this package's
+                        stages (the stage Job pod's activeDeadlineSeconds). An attempt that runs past it is
+                        killed and retried like any other failed attempt, and the package surfaces as erroring
+                        once the operator's retry budget (JOB_BACKOFF_LIMIT) is spent. Interrupt stages are the
+                        exception: their attempt must span a reboot, so it bounds the whole stage instead.
+                        Unset uses the operator default (JOB_STAGE_TIMEOUT); "0" removes the time bound for this
+                        package, leaving the retry budget as its only limit. Note what that does NOT bound: the
+                        budget is only spent by attempts that fail, so with "0" an attempt that hangs hangs
+                        forever. There is also no bound on a pod the kubelet never acknowledges — the attempt clock
+                        runs from the pod's start time, which such a pod never gets — so that case is unbounded at
+                        any stageTimeout. It surfaces as a stage stuck in_progress with a Pending pod, and is node
+                        health rather than stage health.
+
+                        The value is fixed when a stage's Job is created. Editing it does not change a Job already
+                        running, because the bound lives on that Job's pod template and a Job's template is
+                        immutable. To apply a new value to work already under way, clear that Job — `kubectl
+                        nodewright package rerun <package>`, or deleting the Job directly — and the stage restarts
+                        under the new value. Otherwise it takes effect at the package's next stage.
                       type: string
                     uninstall:
                       description: Uninstall configures explicit uninstall support

chart/values.yaml

@@ -85,9 +85,23 @@ controllerManager:
       ## Jobs by outcome, so failure logs outlive success logs.
       jobTtlSucceeded: "1h"
       jobTtlFailed: "24h"
-      ## jobStageTimeout is the default activeDeadlineSeconds for a package stage Job when the
-      ## package itself sets no stageTimeout. "0" disables the deadline.
+      ## jobStageTimeout is the default per-attempt deadline for a package stage Job when the
+      ## package itself sets no stageTimeout. "0" removes the time bound, leaving jobBackoffLimit
+      ## as the only limit — and a retry budget is only spent by attempts that FAIL, so with "0"
+      ## an attempt that hangs hangs forever. A pod the kubelet never acknowledges is unbounded at
+      ## any value, since the per-attempt clock starts from the pod's start time; that shows up as
+      ## a stage stuck in_progress with a Pending pod, and is node health rather than stage health.
+      ## Like jobBackoffLimit below, this is fixed when a stage's Job is created: changing it does
+      ## not affect Jobs already running. Clear those Jobs (`kubectl nodewright package rerun`) to
+      ## apply a new value to work already under way.
       jobStageTimeout: "1h"
+      ## jobBackoffLimit is how many retries a package stage gets after its first attempt before
+      ## 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.
       ## NOTE: If your systems nodes have this taint make sure to add the toleration to the controllerManager.tolerations