feat(ci): collect UAT cluster debug bundle on failure - #1764
Conversation
njhensley
added
the
theme/ci-dx
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds a shared best-effort Kubernetes cluster debug collector and integrates it into AWS, Azure, and GCP UAT runners through a new Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/uat-aws.yaml:
- Around line 627-644: Increase the job timeout budget or revise its budget
calculation to include the new 8-minute “Collect cluster debug bundle” step
while preserving sufficient teardown headroom. Apply the same adjustment to
`.github/workflows/uat-aws.yaml` lines 627-644,
`.github/workflows/uat-azure.yaml` lines 701-716, and
`.github/workflows/uat-gcp.yaml` lines 543-558; update each job’s
`timeout-minutes` declaration and associated budget comment consistently.
In `@tests/uat/lib/collect-debug.sh`:
- Around line 112-122: Update the _cd_section function so each "$@" command is
executed through the repository’s established timeout mechanism, with a bounded
duration that prevents hung kubectl calls from blocking later sections. Preserve
the existing output capture, non-zero suppression, and continuation behavior
while ensuring timeout failures are handled like other command failures.
- Around line 22-35: Update the secrecy claim in the collector documentation
near the kubectl diagnostics to state only that Kubernetes Secret objects are
not fetched; do not claim that pod descriptions or current/previous logs are
free of credentials or PII. Keep the existing best-effort collection behavior
unchanged.
🪄 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: 9ff8ad7c-a6f1-4b56-bb5f-971e1c20ab26
📒 Files selected for processing (7)
.github/workflows/uat-aws.yaml.github/workflows/uat-azure.yaml.github/workflows/uat-gcp.yamltests/uat/aws/runtests/uat/azure/runtests/uat/gcp/runtests/uat/lib/collect-debug.sh
c5561ee to
2e388d1
Compare
|
Thanks @coderabbitai — all three findings were valid and are addressed in the amended commit:
Validated: shellcheck + |
|
Tip For best results, initiate chat on the files or code changes.
Nice work getting shellcheck/yamllint/actionlint clean and re-validating the smoke test on both paths. This all looks good to me. ヽ(・∀・)ﾉ |
mchmarny
left a comment
mchmarny
left a comment
There was a problem hiding this comment.
This closes a real gap — the two recent UAT-AWS failures took raw-CI-log archaeology to root-cause, and the three-layer design here (gate time-series during install, inline Skyhook snapshot at failure, full bundle before teardown) captures exactly the non-monotonic signal that made those hard. The shell discipline is good throughout: guarded kubectl, bounded step, unchanged pass-path semantics in the gate loop, and the errexit-suspension trap in the relogin helper is explicitly handled.
Two comments inline: one medium (the azure debug phase skips az_federated_relogin, so an expired kubelogin token yields a silently empty bundle in exactly the long-failure case this targets) and one low (describe-pods env exposure to public artifacts). Neither changes the design; the azure one is worth fixing before merge since it hollows out the feature on one of the three clouds. Branch is behind main — rebase before merge.
The UAT `Upload failure debug` artifact captured only runner-local files
(report.json, recipe, snapshot); it carried zero live cluster state, so a
deployment-phase failure had to be reconstructed from raw CI step logs.
This adds a cloud-agnostic collector so a failed run's artifact explains
WHY, not just WHAT failed.
- tests/uat/lib/collect-debug.sh: shared `collect_cluster_debug` (full
bundle) + `capture_skyhook_snapshot` (fast, inline-on-failure). Captures
a MANIFEST (failing checks parsed from report.json), node conditions +
reboot fingerprint (bootID/kernel/Ready-transition) + taints, cluster
events (reboots, pod evictions), operator CRs (Skyhook full YAML first),
and per-namespace describe/events + operator/check-Job logs incl.
--previous. Best-effort throughout; never fails the run.
- New `debug` phase wired into tests/uat/{aws,gcp,azure}/run.
- phase_install now persists the readiness-gate status.status time-series
(each attempt, timestamped) into the bundle, so complete->in_progress
tuning flips are captured as they happen — not lost to a single
teardown-time snapshot.
- phase_conformance captures the validate exit code and, on failure,
snapshots the Skyhook CR + node reboot fingerprint inline (seconds after
the failing check, while status is most likely still in_progress).
- A failure()-gated `Collect cluster debug bundle` step (8m bound, before
teardown) added to uat-{aws,gcp,azure}.yaml, with cluster-debug/** added
to the upload artifact.
Motivated by two UAT-AWS runs (29393800033, 29402956254) that failed at
expected-resources on a non-monotonic skyhook re-tuning race; the current
bundle lacked the Skyhook status history, node events, and reboot markers
needed to root-cause it conclusively.
Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
2e388d1 to
0dc2f85
Compare
|
Thanks @mchmarny — both are real and now fixed in the amended commit. 1. Dead credential at collect time (
2. Re-validated: shellcheck / bash-syntax / yamllint / actionlint clean, and the no-cluster smoke test still produces the bundle (incl. the no- |
The three UAT runners (tests/uat/{aws,gcp,azure}/run) were near-identical:
aws and gcp differed only in three comment lines, and azure was the same
body plus a federated-session refresh. Every change to a phase had to be
applied three times (as the debug-bundle work in NVIDIA#1764 showed).
Extract the cloud-agnostic body — the prep/install/conformance/train/serve/
verify phases, inject_push_target/serve_debug helpers, env defaults, and the
phase dispatcher — into tests/uat/lib/phases.sh. Each runner is now a thin
shim that sources the lib and calls `uat_main "$@"`.
Cloud-specific behavior is injected through one hook: cloud_refresh_credentials
(default no-op) plus CLOUD_REFRESH_INTERVAL_SECONDS. The Azure runner overrides
the hook with az_federated_relogin (moved verbatim) and lowers the interval;
the readiness gate calls the hook periodically and the debug phase forces it
before collecting. AWS/GCP keep the no-op default (their sessions outlast a
phase and the workflows refresh before teardown).
Net: ~2400 lines across three files → ~790 shared + ~30 per shim. Behavior is
preserved: every extracted phase function is byte-identical to the previous
aws/run (verified), except phase_install's new hook call and two comments
genericized for the multi-cloud context. az_federated_relogin is moved verbatim.
No workflow or collect-debug.sh changes.
Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
The three UAT runners (tests/uat/{aws,gcp,azure}/run) were near-identical:
aws and gcp differed only in three comment lines, and azure was the same
body plus a federated-session refresh. Every change to a phase had to be
applied three times (as the debug-bundle work in NVIDIA#1764 showed).
Extract the cloud-agnostic body — the prep/install/conformance/train/serve/
verify phases, inject_push_target/serve_debug helpers, env defaults, and the
phase dispatcher — into tests/uat/lib/phases.sh. Each runner is now a thin
shim that sources the lib and calls `uat_main "$@"`.
Cloud-specific behavior is injected through one hook: cloud_refresh_credentials
(default no-op) plus CLOUD_REFRESH_INTERVAL_SECONDS. The Azure runner overrides
the hook with az_federated_relogin (moved verbatim) and lowers the interval;
the readiness gate calls the hook periodically and the debug phase forces it
before collecting. AWS/GCP keep the no-op default (their sessions outlast a
phase and the workflows refresh before teardown).
Net: ~2400 lines across three files → ~790 shared + ~30 per shim. Behavior is
preserved: every extracted phase function is byte-identical to the previous
aws/run (verified), except phase_install's new hook call and two comments
genericized for the multi-cloud context. az_federated_relogin is moved verbatim.
No workflow or collect-debug.sh changes.
Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
Signed-off-by: Nathan Hensley <nhensley@nvidia.com> Co-authored-by: Mark Chmarny <mchmarny@users.noreply.github.com>
2 participants
Summary
On UAT failure, snapshot live cluster state into a
cluster-debug/bundle (nodes, taints/reboot fingerprint, events, operator CRs incl. Skyhook, operator/check-Job logs) before teardown — so a failed run's artifact explains why it failed, not just what check failed.Motivation / Context
The existing
Upload failure debugartifact captured only runner-local files (report.json,recipe.yaml,snapshot.yaml,dry-run.json) — zero live cluster state. Two recent UAT-AWS runs (29393800033,29402956254) failed at theexpected-resourcesdeployment check on a non-monotonic skyhook re-tuning race (Skyhook /tuning status.statusgoescomplete → in_progressafter the readiness gate certifies it, and the tuning reboot can evict the check Job's pod). Root-causing that required reconstructing the timeline from raw CI step logs, because the bundle contained none of the Skyhook status history, node events, or reboot markers needed. This closes that gap.Fixes: N/A
Related: N/A
Type of Change
Component(s) Affected
tests/uat/**) + UAT workflows (.github/workflows/uat-*.yaml)Implementation Notes
tests/uat/lib/collect-debug.sh(new shared, cloud-agnostic lib):collect_cluster_debug— full bundle: aMANIFEST.yaml(failing checks parsed straight fromreport.json), node conditions + reboot fingerprint (bootID / kernel / ReadylastTransitionTime) + taints, cluster events (reboots, pod evictions), operator CRs (Skyhook full YAML first), and per-namespace describe/events + operator/check-Job logs incl.--previous.capture_skyhook_snapshot— fast, focused snapshot invoked inline the moment a validate failure is detected, seconds after the failing check (whilestatus.statusis most likely stillin_progress), vs. the teardown-time full bundle taken minutes later.debugphase wired intotests/uat/{aws,gcp,azure}/run(dispatch + usage/header docs).phase_installnow persists the readiness-gatestatus.statustime-series (each attempt, timestamped) into the bundle — socomplete → in_progresstuning flips are captured as they happen, not lost to a single snapshot.phase_conformancecaptures the validate exit code and, on failure, callscapture_skyhook_snapshotbefore propagating (was:set -eaborted immediately).failure()-gatedCollect cluster debug bundlestep (8-minute bound, before teardown) added touat-{aws,gcp,azure}.yaml, withcluster-debug/**added to the upload artifact.kubectlcall is guarded, the collector never fails the run, and the step is bounded so it can't eat the teardown budget or leak a GPU node. Secrets are not dumped.Follow-up (out of scope, noted for reviewers):
aws/runandgcp/runare functionally identical (differ only in 3 comment lines) andazure/runis the same core plus anaz_federated_reloginhook — strong candidate for a furthertests/uat/lib/phases.shconsolidation in a dedicated PR.Testing
Also smoke-tested the collector against the real
report.jsonfrom run29393800033with no cluster reachable: it no-ops gracefully and theMANIFEST.yamlcorrectly surfaces both failing checks and points at the new time-series artifacts (readinessGateSeries,skyhookAtFailure).Risk Assessment
failure()before teardown; no change to the pass path or toaicritself. Thephase_conformancechange preserves the prior failure semantics (still exits non-zero on a validate failure).Rollout notes: No migration. The new artifact fields appear on the next failed UAT run.
Checklist
make testwith-race) — N/A, no Go changesmake lint) — shellcheck + yamllint + actionlint cleanaicrsurface)serve_debug()/phase_prepsnapshot-debug idioms)git commit -S)