Skip to content

docs(operator): correct where a timed-out stage's logs actually live - #458

Merged
ayuskauskas merged 1 commit into
feature/package-as-jobsfrom
jobs-migration/449-release-notes-lastlogs
Aug 13, 2026
Merged

docs(operator): correct where a timed-out stage's logs actually live#458
ayuskauskas merged 1 commit into
feature/package-as-jobsfrom
jobs-migration/449-release-notes-lastlogs

Conversation

@ayuskauskas

Copy link
Copy Markdown
Collaborator

Important

Base this work on feature/package-as-jobs, not main.

The Jobs migration (#223) is integrating on that branch. main does not yet execute
packages as Jobs, so a PR opened against main will be missing the code this depends
on. feature/package-as-jobs merges to main once #305 lands.

That branch squash-merges, so if your branch is stacked on another PR in this epic,
replay only your own commits when restacking:
git rebase --onto origin/feature/package-as-jobs <last-inherited-commit>

Part of #223. Refs #449 — this corrects the claim; the underlying gap is tracked there.

The claim that was wrong

The Jobs release notes promised:

A stage killed by its own deadline additionally gets a ~16KiB log tail snapshotted into the Job's nodewright.nvidia.com/last-logs annotation

It does not. A per-attempt stageTimeout fails the pod in place rather than deleting it, so the timed-out attempt survives as an ordinary full-log archive and snapshotFailureLogs deliberately skips ("a genuine failed archive already holds full logs"). I wrote that sentence in #402 from the pre-#402 design, where a Job-level deadline could delete the running pod.

Manual validation found the annotation empty in every package-stage case, twice:

Case Build last-logs
stage timeout (stageTimeout: 20s, SLEEP_LEN: 600) 93caf2ac empty — logs read from the archive pod, showing the SIGTERM
kubelet-refused attempts 93caf2ac empty
unpullable image (stageTimeout: 60s) 93caf2ac empty
stage timeout, re-run after main merged 3e3886ea empty
unpullable image, re-run 3e3886ea empty

What it says now

That the archive holds the logs for a timed-out attempt, and that the last-logs fallback belongs to the interrupt Job's whole-stage deadline — the one case where the pod really is deleted.

Deliberately not claiming the fallback works for package stages, because per #449 it currently cannot fire for them at all: the snapshot is gated on FailureTarget, which a package Job only reaches once the final attempt has already failed, at which point either a genuine archive exists (skip) or no live pod remains to read. Fixing that is #449's job; this PR just stops the release notes promising something users will not find.

Docs only — no code, no test changes.

The Jobs release notes said a stage killed by its own deadline gets a
last-logs snapshot. It does not, and manual validation on 2026-08-12 and
again on 2026-08-13 found the annotation empty in every package-stage
case: a per-attempt deadline fails the pod in place, so the archive holds
the logs and snapshotFailureLogs deliberately skips.

The annotation is still written for an interrupt Job's whole-stage
deadline, where the Job controller deletes the pod, so the note now says
that instead of promising it for every timeout.

Refs #449, which tracks the fact that the fallback is currently
unreachable for package Jobs at all.

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>
@github-actions github-actions Bot added doc Documentation change (PR path label; doc issues use the Documentation type) component/operator Skyhook operator (controller-manager) component/ci CI workflows, GitHub Actions, and repo tooling labels Aug 13, 2026
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 3c64eb5e-cc10-4b8b-a4ce-dfafef6ee518

📥 Commits

Reviewing files that changed from the base of the PR and between 3e3886e and 61b0e07.

📒 Files selected for processing (1)
  • operator/RELEASE_NOTES.md

📝 Walkthrough

Walkthrough

Failure-log handling now retains full logs for stage-timeout failures. When interrupt-job pod deletion occurs, the Job annotation records a best-effort log tail of approximately 16 KiB. The previous stage-timeout annotation snapshot was removed.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Mergeability Score: ⚪ Minimal · up to 61b0e

This documentation-only change corrects the release-note description of where timed-out stage logs are stored without changing runtime behavior. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: lockwobr, anouarmohamed, rice-riley

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the documentation correction about where timed-out stage logs are stored.
Description check ✅ Passed The description directly explains the release-notes correction and the distinction between archive logs and the last-logs annotation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jobs-migration/449-release-notes-lastlogs

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

@ayuskauskas
ayuskauskas merged commit b9b1114 into feature/package-as-jobs Aug 13, 2026
8 checks passed
@ayuskauskas
ayuskauskas deleted the jobs-migration/449-release-notes-lastlogs branch August 13, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI workflows, GitHub Actions, and repo tooling component/operator Skyhook operator (controller-manager) doc Documentation change (PR path label; doc issues use the Documentation type)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants