docs: design doc for package execution as Jobs

[ayuskauskas]

First PR of the #223 series (closes #299). Targets feature/package-as-jobs, not main — the feature branch PRs to main once the whole series lands.

Adds docs/designs/2026-07-10-package-execution-as-jobs.md: one batch/v1 Job per (skyhook, package, version, stage, node) replaces operator-managed raw Pods. Docs-only change; no code.

Key decisions specified in the doc:

• Pod shape: initContainer chain unchanged; the forever-running pause main container becomes an exit-0 container so the pod can reach Succeeded and the Job can complete. New not-ready/unreachable NoExecute tolerations (unbounded, deliberately) so reboot-class interrupts don't get the pod evicted at the 300s default.
• Retry substrate — Never + failed-attempt archive: package/stage Jobs use restartPolicy: Never (each attempt a fresh pod; backoffLimit: MaxInt32 counts Failed pods; podFailurePolicy Ignore-on-DisruptionTarget keeps disruptions silent and uncounted). The operator prunes to one archived Failed pod per Job — full kubectl logs on the last real failure through retries, deadline park, and pause. Interrupt Jobs keep OnFailure (a reboot interrupt kills its own pod by design). Erroring evidence is triple-guarded (no DisruptionTarget, real verdict, DeletionTimestamp unset) so suspension/sweeps/manual deletes stay silent.
• Stage deadline (the one deliberate enhancement + the one CRD change, additive): new per-package stageTimeout field (metav1.Duration, gracefulShutdown pattern) → Job activeDeadlineSeconds; operator default JOB_STAGE_TIMEOUT (1h, chart controllerManager.manager.env.jobStageTimeout), "0" disables. A hung/crash-looping/unpullable stage now surfaces as erroring after the deadline and parks — the Failed Job blocks recreation until rerun/reset/config-update or JOB_TTL_FAILED expiry (deliberate slow-retry cadence). On FailureTarget the operator snapshots the stuck container's last ~16KiB into the skyhook.nvidia.com/last-logs Job annotation before the pod is deleted, so the parked tombstone stays debuggable; full logs remain on the host (SKYHOOK_LOG_DIR).
• Completion: a persisted skyhook.nvidia.com/state-recorded Job annotation replaces delete-on-success as the processed-once marker, with per-transition postcondition guards (incl. the interrupt/ProgressSkipped case) for the two-object crash window. Child pods are selected by batch.kubernetes.io/controller-uid; a labels-based fallback covers GC'd pods; node-NotFound completions are marked processed instead of error-looping.
• Outcome-based TTL: created with ttlSecondsAfterFinished unset; set at completion — JOB_TTL_SUCCEEDED (1h) / JOB_TTL_FAILED (24h), chart values controllerManager.manager.env.jobTtlSucceeded/jobTtlFailed.
• Finished-Job lifecycle: four rules (validation scoped to unfinished Jobs; a rerun predicate with the deadline-park exception; AlreadyExists handling that never discards unrecorded completions and honors the park; an orphaned-node sweep) — plus HasRunningPackages/JobExists counting only unfinished Jobs.
• Upgrade: no zz.migration shim; four legacy accommodations for one minor, keyed on the batch.kubernetes.io/job-name label (completion path, existence gating, validation sweep, HandleConfigUpdates direct pod deletion).
• RBAC: batch/jobs verbs + pods/log get (log-tail snapshot), mirrored into the chart.
• Rejected alternatives documented: podFailurePolicy for ImagePullBackOff (cannot work — precise fix stays Surface ImagePullBackOff/ErrImagePull on package pods as state: erroring #306, though the deadline now bounds it), suspend as pause, separate JobReconciler, JobSet, static TTL, backoffLimit: 0 variants, Never-mode parking, random name suffixes.

Review history: two independent adversarial review passes plus CodeRabbit. c5df70fe addressed pass 1 (3 blockers/5 majors); 4f7e02a6 addressed CodeRabbit (incl. the backoffLimit/OnFailure accounting correction) and pass 2 (4 majors); 2653abcd adds the stage-deadline/park/log-snapshot enhancement from maintainer review discussion. Dispositions in the comments below.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a design document describing migration of per-stage package execution from operator-managed Pods to one Kubernetes Job per skyhook, package, stage, and node. It specifies Job naming, labels, annotations, retry and completion behavior, deadline parking, suspension, outcome-based TTLs, idempotency, RBAC changes, legacy-pod compatibility during upgrades, and rejected alternatives.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• Jobs migration 5/7: switch package execution to Jobs (wiring, RBAC, chart) #303: Covers the Job-based package execution design, including deadline parking, pause suspension, RBAC, and rerun behavior.
• Jobs migration 3/7: Job builders for package and interrupt stages #301: Covers deterministic Job construction, metadata, TTL behavior, and stageTimeout mapping.
• Jobs migration 4/7: dal Job accessors and Job event handling #302: Covers Job completion handling, state-recorded annotations, TTLs, deadline failures, parking, and log snapshots.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The document covers the requested Job migration design, process-once marker, TTL handling, upgrade path, and related review refinements.
Out of Scope Changes check	✅ Passed	No unrelated code changes are present; the added documentation stays within the package-as-Jobs migration scope.
Title check	✅ Passed	The title clearly summarizes the main change: a design doc for executing packages as Jobs.
Description check	✅ Passed	The description is directly related and accurately summarizes the documentation-only Job migration design.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jobs-migration/299-design-doc

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/designs/2026-07-10-package-execution-as-jobs.md`:
- Line 71: Update the quoted phrase in the explanation around the `pause`
container to use the hyphenated wording “in-flight” instead of “in flight.”
- Around line 48-49: The job metadata design currently uses the raw node name as
the skyhook.nvidia.com/node label value. Replace it with a bounded, stable hash
or truncated identifier that satisfies Kubernetes label limits, preserve the
full node name in an annotation, and update all related label/query examples and
references consistently.
- Around line 79-82: Make Job completion handling crash-safe and idempotent:
revise JobReconcile and the completion path around
HandleCompletePod/UpdateNodeState so processing is keyed by the Job UID or uses
a durable claim that is written before the non-idempotent state transition, with
recovery for claims left by crashes. Ensure duplicate or re-served
Complete/Failed events cannot repeat transitions or resurrect uninstall state,
while preserving the existing outcome TTL and special-case behavior.
- Around line 57-58: Update the Job/Pod lifecycle design to disambiguate Pods by
the current Job UID via controller owner references rather than relying only on
the job-name label. When multiple Pods exist, select the terminal Pod for the
correct attempt, and before recreating a deterministically reused Job name,
delete the old Job and wait until its owned Pods have fully disappeared,
including terminating Pods, before creation.
- Around line 57-58: Update the documented job configuration to account for
Kubernetes versions before v1.34: either raise the minimum supported version to
v1.34 or specify that podReplacementPolicy is conditionally omitted when the API
server lacks JobPodReplacementPolicy support.
- Around line 85-90: Correct the “Retry semantics” section to state that
restartPolicy: OnFailure retries contribute to Job backoff accounting and can
cause the Job to become Failed even when backoffLimit is 0. Update the described
recovery path accordingly, including how the operator handles a failed
crash-looping Job and whether it deletes and recreates it when the stage should
still run.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: e58dd2d4-bda5-4e4b-a67f-3c60cad508c9

📥 Commits

Reviewing files that changed from the base of the PR and between 6a3d146 and 4e1246f.

📒 Files selected for processing (1)

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

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/designs/2026-07-10-package-execution-as-jobs.md`:
- Around line 77-88: Update the documented NotReady and Unreachable tolerations
to use a bounded, configurable tolerationSeconds reboot grace period instead of
infinite toleration. Add an explicit recovery path for permanently unavailable
nodes so the active Job can terminate and be replaced or otherwise converge
despite backoffLimit: 0, and revise the surrounding comments to describe this
behavior.
- Around line 139-141: Resolve the contradiction between the rerun predicate and
AlreadyExists handling: in ApplyPackage and Interrupt, after GETting an existing
Job, preserve any Complete Job lacking the state-recorded marker and requeue its
completion processing instead of deleting it. Continue deleting only Failed Jobs
or Complete Jobs already marked state-recorded, while leaving unfinished
matching Jobs untouched.
- Around line 120-129: Clarify the failure-path behavior in the TTL
documentation: specify where JobReconcile assigns JOB_TTL_FAILED for Failed Jobs
that do not write state-recorded, and whether this occurs before any retention
or deletion action. Explicitly describe the ordering with immediate disruption
reruns, including whether rerun deletion can happen before the TTL update and
that TTL is only a backstop when the Job remains present.
- Line 105: Define how the GC fallback supplies the required containerName for
UpdateNodeState and HandleCompletePod: persist the completion container name in
Job metadata when creating or updating the Job, then read it alongside stage,
node, interrupt, and restart data when the child Pod is gone; alternatively
implement and document a deterministic stage-to-container mapping that preserves
uninstall-interrupt handling.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 2f826982-3514-43a5-ae84-ab498ca3c49f

📥 Commits

Reviewing files that changed from the base of the PR and between 4e1246f and c5df70f.

📒 Files selected for processing (1)

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

[ayuskauskas]

Adversarial design review — summary for reviewers

An independent high-effort review pass ran against the initial design (4e1246fa), tracing every claim through the operator code. Verdict was needs rework; all findings are addressed in c5df70fe. Recorded here so human review can focus on judgment calls rather than re-deriving these.

Blockers (all fixed)

1. Rerun paths never converged. Deterministic Job names + retained finished Jobs meant Failed-Job recovery, REAPPLY_ON_REBOOT, config-update reruns, and CLI reset/rerun all hit AlreadyExists against a name-squatting finished Job — and "AlreadyExists is benign" wedged the state machine (worst case: Upsert(in_progress) against a Job that will never run). → Fixed: precise AlreadyExists handling (GET; foreground-delete finished/mismatched + requeue; never record in_progress) plus a rerun predicate as the only finished-Job cleanup besides TTL.
2. Retention was self-destructing. Keeping ValidateRunningPackages' node-state stage-mismatch check "verbatim" over all Jobs would delete every retained Job the moment the node progressed to the next stage — minutes of retention, not JOB_TTL_SUCCEEDED. → Fixed: existing checks scoped to unfinished Jobs only.
3. Upgrade window created duplicate executors. The dual-path covered completion but not existence-gating: with a legacy raw pod mid-stage, JobExists (Jobs-only) missed it and ApplyPackage would create a second privileged chroot executor against the same host copyDir; Interrupt could cordon/drain under a running legacy stage. → Fixed: JobExists/HasRunningPackages/validation OR-in legacy pods (name label present, batch.kubernetes.io/job-name absent) for the one-minor window.

Majors (all fixed)

4. Slow reboots (>300s NotReady) would evict the pod via DefaultTolerationSeconds and fail the Job (backoffLimit: 0) — a regression vs today's explicit interrupt-pod recreate. → not-ready/unreachable Exists NoExecute tolerations on the template.
5. Writing erroring on disruption-failed Jobs would feed DeploymentPolicy batch-failure counting for what today is an invisible self-healing event. → Disruption failures are now silent re-executions; erroring stays sourced from crash-loop/exit-code evidence via the Pod watch, as today.
6. HandleConfigUpdates was missing from the blast radius — it deletes erroring package pods directly, which under Jobs fails the parent Job instead of forcing recreate-with-new-config. → Deletes the Job; uses JobExists.
7. The marker's "written before any event can be re-served" claimed impossible two-object atomicity, and re-processing can regress node state (Upsert is not stage-monotonic). → Claim corrected (same window exists today); stage-progress re-processing guard added; state-first/marker-second order kept (inverting loses completions).
8. Failed Jobs usually have no child pod left to read (that's what failed them), and terminated-pod GC (--terminated-pod-gc-threshold, default 12500) can reap retained pods before TTL; the in-flight erroring path also needs the package annotation on the child pod. → Job-label fallback, pod-template annotation, retention caveat documented.

Minors

9. Interrupt Jobs keep their own name formula (skyhook, stage, interruptType, node — the doc had implied the package formula) and the interrupt: "True" label. 10. The stage-metrics map in ValidateRunningPackages is dead code (skyhook_package_stage_count derives from node state) — deleted, not ported. 11. podReplacementPolicy: Failed rationale corrected (with backoffLimit: 0 no replacement is ever created; the field only affects failure timing). 12. Jobs informer namespace-scoped via cache.Options.ByObject. 13. Tuple includes version; node label falls back to generateSafeName(63, node) for long node names; generation label wording tightened.

Verified correct (no action)

Baseline behavior section (pause container, init-status completion detection + delete-on-success, crash-loop-forever retries, silent vanished-pod recreate, labels/annotations, cluster_state_v2.go reads no pods); ResourceID() too long for a label; no CLI change needed (package logs is label-based, rerun/reset are annotation-only); retained Succeeded pods don't block drain or interrupt gating; single-queue serialization holds with job--- routing; K8s facts (TTL mutable + applies to Failed, podFailurePolicy can't catch ImagePullBackOff, suspend restarts from scratch, initContainers are the sequencing primitive) — all valid at min-supported K8s 1.31.

[ayuskauskas]

Review round 2 — CodeRabbit triage + independent re-review (4f7e02a6)

CodeRabbit findings — dispositions

Accepted (fixed in 4f7e02a6):

• backoffLimit: 0 + OnFailure was broken — the Job controller counts in-place container restarts (init containers included) toward backoffLimit and terminates the pod at the limit, so the first step retry would have killed the package. This invalidated the previous draft's retry section; backoffLimit is now math.MaxInt32 and the whole Failed-recovery machinery collapsed into "backstop only" (pod loss self-heals via the Job controller's replacement pod). Best catch of the round.
• AlreadyExists could discard an unrecorded completion — rule 3 now requeues Complete-but-unmarked Jobs instead of deleting them.
• Child pods selected by batch.kubernetes.io/controller-uid (job-name label alone can match a terminating pod from a previous same-named Job instance).
• containerName fallback for GC'd pods defined (it is only ever compared against InterruptContainerName, so the Job's interrupt label determines it — verified in pod_controller.go).
• Failed-path marker/TTL write was ambiguous — now explicit (the Update carries JOB_TTL_FAILED; Failed alone already counts as processed).
• Completion crash-guard upgraded from stage-ordering to per-transition postconditions.
• "in-flight" hyphenation.

Rejected (with reasoning):

• Always hash the node label: kept name-when-it-fits with a generateSafeName(63, node) fallback (deterministic, hash-suffixed) — always-hashing would gut the human-queryable label for virtually all clusters; spec.template.spec.nodeName remains the authoritative full name.
• Bounded tolerationSeconds on the not-ready/unreachable tolerations: eviction of a nodeName-pinned pod only produces churn against a down node; NotReady-forever mirrors today's raw-pod behavior, and node removal is handled by the orphaned-node sweep.
• Version-gate podReplacementPolicy: beta + on-by-default across the entire supported range (1.31+). The doc's earlier "correctness unaffected if the gate is off" overclaim is fixed instead: gate-off degrades Job-controller-replacement non-overlap to best-effort; operator-initiated recreates remain safe via foreground deletion.

Independent re-review (fresh pass, high effort)

Verdict: ship with edits — all applied in 4f7e02a6. It re-verified the new backoff model from first principles (incl. init-container restart accounting, nodeName-pinned replacement creation, the PodGC↔Job-controller loop motivating the orphan sweep, and math.MaxInt32 having no status-churn pathology) and agreed with all three CodeRabbit rejections. Its own findings, all fixed:

1. HasRunningPackages needed an explicit Jobs-based definition — left pod-based it has no phase filter, so retained Succeeded pods would have stalled every interrupt (node cordoned + drained) for the success-TTL window.
2. Fourth legacy accommodation: HandleConfigUpdates keeps direct deletion of legacy erroring pods during the upgrade window, else a mid-window config update deadlocks on a crash-looping legacy pod nothing can replace.
3. Complete-but-unrecorded Jobs of a deleted node would error-loop the reconciler and leak forever — node-NotFound now marks the Job processed, and the orphan sweep covers Jobs of deleted nodes regardless of status.
4. The postcondition guard was missing HandleCompletePod's fifth transition (interrupt container → ProgressSkipped cross-package promotion) — now enumerated.
5. Documented retention carve-outs: successful no-interrupt uninstall Jobs get no retention window (their completion is entry removal), and starting an uninstall cycle clears the package's retained stage Jobs.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/designs/2026-07-10-package-execution-as-jobs.md`:
- Around line 60-66: Clarify the backoffLimit explanations in the affected
design sections to distinguish failed Pods from active Pods: state that failed
Pods are counted separately, while container restarts are summed only for Pods
currently in Pending or Running phases when restartPolicy is OnFailure, not
across every Pod belonging to the Job.
- Around line 67-76: Update the package execution design to avoid relying on the
Job controller’s TerminatingOrFailed fallback when podReplacementPolicy=Failed
is unavailable. Gate this configuration by cluster capability and use an
operator-managed wait-then-recreate flow that waits for the prior node-bound Pod
to terminate before creating its replacement, preserving exclusive hostPath
access.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 385b1551-56c4-4e14-afac-e01c27010b1c

📥 Commits

Reviewing files that changed from the base of the PR and between c5df70f and 4f7e02a.

📒 Files selected for processing (1)

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

[ayuskauskas]

Addition (2653abcd): per-package stage deadline, parked failures, log-tail snapshot

From maintainer review discussion — one deliberate enhancement over behavior parity, and the migration's single (additive) CRD change:

• Package.stageTimeout → Job activeDeadlineSeconds (default JOB_STAGE_TIMEOUT=1h via controllerManager.manager.env.jobStageTimeout; "0" disables). Bounds the wall-clock of each stage including in-place retries and reboot time.
• On expiry: erroring (hung stages get surfaced for the first time — today they sit in_progress invisibly forever) and the Failed Job parks, absorbing recreate attempts so the churn actually stops. Unparked by rerun/reset/config-update, or JOB_TTL_FAILED expiry (a deliberate ~1h-on/24h-off slow-retry cadence until someone intervenes).
• Because deadline expiry deletes the pod (and its API-readable logs), JobReconcile reacts to the pre-terminal FailureTarget condition by snapshotting the stuck container's last ~16KiB into a skyhook.nvidia.com/last-logs annotation on the Job — the parked tombstone stays debuggable via kubectl get job -o yaml for the full failed-TTL window. Best-effort by design; full logs remain in the host-side SKYHOOK_LOG_DIR and any log aggregation.
• Side effect worth noting: the deadline also bounds the ImagePullBackOff hang (Surface ImagePullBackOff/ErrImagePull on package pods as state: erroring #306) — an unpullable image now surfaces as erroring within stageTimeout instead of never. Surface ImagePullBackOff/ErrImagePull on package pods as state: erroring #306 remains open for the fast, precise surfacing.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/designs/2026-07-10-package-execution-as-jobs.md`:
- Around line 67-71: Update the package execution job examples and stageTimeout
behavior text to convert nonzero stageTimeout durations to whole integer seconds
for activeDeadlineSeconds, and omit activeDeadlineSeconds entirely when
stageTimeout is explicitly 0 or unset. Ensure both documentation locations
consistently describe and demonstrate this mapping.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 9f8a2159-d71f-437b-8580-a4e50f9fdeec

📥 Commits

Reviewing files that changed from the base of the PR and between 4f7e02a and 2653abc.

📒 Files selected for processing (1)

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

[ayuskauskas]

Addition (03ffc7e9): pause cascades to Job suspension

Second enhancement from maintainer review discussion. skyhook.nvidia.com/pause (the CLI's Emergency Stop) currently only blocks new stage scheduling — it cannot stop a running stage. Now UpdatePauseStatus sets spec.suspend: true on the Skyhook's unfinished Jobs (and clears it on resume); the annotation remains the user-facing primitive.

• Mid-step SIGTERM is the recovery shape packages already must tolerate (reboot/eviction mid-stage); the agent's flag files skip completed steps on resume. The old "Rejected: suspend as pause" section is replaced — the no-checkpointing cost is accepted deliberately, because an emergency stop that can't stop anything isn't one.
• Coherence win with the new stage deadline: suspension clears the Job's startTime, so activeDeadlineSeconds stops ticking while paused and resets fresh on resume — otherwise a "paused" in-flight stage could hit its deadline and park as erroring while the user believed everything was frozen.
• Suspended Jobs remain unfinished for JobExists/validation; interrupts that already fired a reboot converge via the SKYHOOK_RESOURCE_ID flag; legacy raw pods keep today's let-finish semantics during the upgrade window, so pause's stop strength is operator-version-dependent (CLI docs note required — added to Jobs migration 5/7: switch package execution to Jobs (wiring, RBAC, chart) #303/Jobs migration 7/7: release notes and upgrade verification #305 scope).
• disable is deliberately unchanged.

[ayuskauskas]

Rework (1b28b81d): Never + failed-attempt archive — and review pass 3

From maintainer review discussion: since the stage deadline already catches hangs, restartPolicy: OnFailure's remaining advantage (in-place crash-loop parity) no longer justified its costs (no failure archive possible, restart-sum backoff trap, no podFailurePolicy). Package Jobs now use Never with a keep-one failed-attempt archive (full kubectl logs on the last real failure, surviving deadline park and pause), Ignore-on-DisruptionTarget for silent disruptions, and interrupt Jobs keeping OnFailure (a reboot interrupt kills its own pod by design). The deadline snapshot now records the Waiting reason+message for never-started containers, so an unpullable image's parked tombstone names the pull error.

Independent review pass 3 (fresh high-effort agent) verified the substrate from first principles — pruner accounting safety under job-tracking finalizers, DisruptionTarget coverage at the 1.31 floor, nodeName-pinned replacement creation, suspension semantics, the interrupt carve-out — and found one blocker + two majors in the evidence perimeter, all fixed in 1b28b81d:

1. [blocker] The erroring guards missed deletion-driven kills: pause suspension, rule-deletions, and manual kubectl delete pod produce Failed pods with real-looking verdicts and no DisruptionTarget — the design's own pause would have flipped nodes erroring. → third guard: DeletionTimestamp unset.
2. [major] Resume had no owner (UpdatePauseStatus only runs while paused, and paused Skyhooks skip validation) → un-suspension is now an explicit post-validation step for non-paused Skyhooks, ordered so stale-spec suspended Jobs are invalidated before suspend clears.
3. [major] REAPPLY_ON_REBOOT reset could be undone by a pre-reboot Complete Job whose recording was still queued → new rule 5: reboot-reset sweeps the node's Jobs regardless of status.

Plus: pruner precision (archive excludes DisruptionTarget casualties; never force-delete), HandleConfigUpdates scoped to unfinished/parked Jobs (a literal port would gut retention on every config update), unreachable-node deadline surfacing via stale FailureTarget, stageTimeout: "0" = field omitted, attempts figure now job.status.failed (user-visible improvement, documented), honest hard-crash/admission-rejection deltas — including credit for fixing a latent wedge where an admission-rejected raw pod satisfies PodExists forever. Two e2e asserts added to the test plan for the claims the reviewer couldn't verify statically (disruption-eviction promptness; FailureTarget-before-Failed on a 1.31 kind node).

[lockwobr]

Overall this is a strong, thorough design and I could follow the substance. My one piece of feedback is about readability: the detail isn't the problem, the layering is. Almost every sentence does three jobs at once (states the decision, justifies it, and pre-empts a reviewer objection), so there's no altitude a casual reader can cruise at. It reads a bit like the two adversarial review passes got inlined into the prose instead of layered underneath it.

Some concrete ways to cut it down without losing the rigor:

1. Add a ~10-line TL;DR up top, before "Problems"

A reader is dropped straight into full detail with no map. A short decision table would let someone get the shape in 60 seconds and choose where to drill in:

Decision	Choice	Why (1 line)
Retry substrate	Never + unlimited backoff	durable per-attempt log archive
Completion marker	state-recorded annotation	pods now linger, can't delete-mark
Stage bound	activeDeadlineSeconds + park	hung stages surface as erroring
Pause	cascades to spec.suspend	pause becomes a real stop
TTL	outcome-based, set at completion	keep failure logs longer

Everything after becomes optional depth rather than required reading.

2. Cut implementation identifiers out of the narrative

This is the biggest single lever. The prose is dense with things that belong in code review, not a design doc:

• exact constants: 2147483647, ~16KiB, 10s doubling, capped at 6 min, 300s
• Go symbol names: fudgeInterruptWithPriority, HandleCompletePod, UpdateNodeState, NextStage/IsPackageComplete
• k8s field literals: batch.kubernetes.io/controller-uid, DefaultTolerationSeconds

A design doc can say "effectively unlimited backoff" and "a small byte cap (annotations share a per-object metadata budget)"; the exact number is a PR detail that will drift. Referencing behavior by role instead of by symbol would shrink the Retry, Completion, and Finished-Job sections noticeably and stop the reader context-switching between "design" and "codebase" every sentence.

3. Trim the parentheticals and inline (see X) chains

The pattern "claim (justification) (edge case) (see other section)" repeats throughout. Two fixes: move the "why" out of multi-line YAML comments into one prose sentence after the block (the backoffLimit comment runs ~8 lines), and drop most forward/back cross-refs; a reader following linearly doesn't need to be told to jump.

4. Demote the defensive material to a clearly-marked appendix

The crash-window honesty, the OutOfPods admission edge, the ContainerStatusUnknown-vs-terminated{137} distinction, the "two honest deltas around hard node crashes" are all valuable (and clearly the product of review), but they drown the main flow. Collecting them under one "Edge cases and correctness arguments" heading at the end lets the main sections describe the happy path plus the one or two load-bearing invariants, and a reviewer who wants the proof-of-safety finds it in one place.

5. Collapse "Rejected alternatives" to a table plus short entries

Seven rejections, each a full paragraph. Most reduce to two sentences (what it was, the one fact that killed it):

podFailurePolicy for ImagePullBackOff — can't fire: an unpullable image never starts a container, so no exit-code/condition rule matches. Fast surfacing stays #306.

The Never-vs-OnFailure reasoning is the one worth keeping in full, since that's the central decision.

---

Net: not cutting the analysis, just moving it below the fold so the doc has a readable top layer. Rough guess is this goes from ~309 lines to ~200 of body plus an appendix, with the first ~40 lines carrying most of what a typical reader needs. Happy to draft the restructured version if useful.

[ayuskauskas]

Restructured in 495f7805 — kept all the analysis, just moved it below the fold as you suggested. Mapping to your five points:

1. TL;DR up top — added a decision table (execution unit, retry substrate, disruptions, completion marker, stage bound, pause, TTL, finished-Job cleanup, upgrade) plus a two-line invariant summary, before Problems.
2. Cut implementation identifiers — the narrative now describes behavior by role. Exact constants (2147483647, ~16KiB, 10s/6min, 300s) are gone; Go symbols (fudgeInterruptWithPriority, HandleCompletePod, UpdateNodeState, NextStage/IsPackageComplete) and file refs live in the "Current behavior (baseline)" section and a new References block. I kept the k8s API surface the design actually sets or reacts to (restartPolicy, podFailurePolicy, activeDeadlineSeconds, DisruptionTarget/FailureTarget/DeadlineExceeded, spec.suspend) since those are the design, not incidental.
3. Trimmed parentheticals / (see X) chains — the multi-line YAML comments are now one prose paragraph after each block; most forward/back cross-refs dropped (a couple of pointers into the appendix kept).
4. Defensive material → appendix — new "Edge cases and correctness arguments" section collects the crash-window postconditions, the hard-crash/OutOfPods deltas, the full erroring guards, pruner accounting, and the replacement-policy-gate-off caveat. Main sections now carry the happy path plus the load-bearing invariant.
5. Rejected alternatives → table — seven rejections are a table of one-liners now; only the central Never-vs-OnFailure decision stays in full.

Also fixed the meta-lint failure — the Goals list had a duplicate 4. (MD029); it's sequential 1–5 now, and markdownlint-cli2 is clean locally.