feat(ci): nightly UAT version matrix + versioned install (#1274)#1579
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a composite action to install a released Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/actions/install-aicr-release/action.yml:
- Around line 52-90: The provenance check is too broad because identity_re
allows any refs/tags/*, so a binary attested for one tag could be accepted for a
different release tag. Tighten the verification in the install flow by matching
the exact requested tag in the certificate SAN, using the release tag value
already available in the action and the cosign verify-blob-attestation call.
Update the identity_re construction and any related tag handling so the
attestation must correspond to the specific release being installed, not just
any tag in the repo.
In @.github/workflows/uat-nightly-batch.yaml:
- Around line 166-177: The run lookup in the nightly workflow is relying on a
shared displayTitle plus createdAt filtering, which can match the wrong
dispatch. Thread a unique correlation token from the controller into
uat-run.yaml and use that token to identify the target run in the gh run list/jq
resolution logic instead of matching only the UAT <reservation> @ <version>
title; update the resolver around the run_id lookup to key off the unique token.
- Around line 180-183: The dispatch loop in the uat-nightly-batch workflow
should stop advancing when a job is dispatched but its run id cannot be
resolved, because continuing breaks the intended sequential ordering. In the
block that checks for an empty run_id, change the control flow in the
dispatch/wait logic to fail fast or otherwise halt the remaining dispatches
instead of using continue, so later cells cannot supersede the unresolved one.
Use the run_id resolution check in the workflow step as the unique anchor for
the fix.
In @.github/workflows/uat-run.yaml:
- Around line 17-20: The run-name in uat-run.yaml is not unique enough for
controller correlation because the same UAT title can be reused across retries
and dispatches. Update the workflow’s run-name to include a controller-supplied
nonce input alongside reservation and version, and ensure the nightly driver
passes a unique value such as a controller run id plus cell index. Use the
existing workflow dispatch inputs and the run-name field together so each
dispatched run can be matched unambiguously.
In `@docs/contributor/uat.md`:
- Line 51: The UAT example in the contributor docs is incomplete because it
omits the repository and ref, which can make copy-pasted commands fail or run
the wrong workflow revision. Update the command in the affected markdown section
to use the fully qualified gh workflow run invocation with the same repo and ref
context as the nightly path, so readers can run it reliably outside a clone or
from any branch.
🪄 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: 01afa62a-4207-400f-a021-2078e523c553
📒 Files selected for processing (6)
.github/actions/install-aicr-release/action.yml.github/workflows/uat-aws.yaml.github/workflows/uat-gcp.yaml.github/workflows/uat-nightly-batch.yaml.github/workflows/uat-run.yamldocs/contributor/uat.md
e7bba10 to
3ff1a6e
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 117-145: The workflow still runs Setup Go unconditionally, which
keeps release cells dependent on the source toolchain even though they should
use the released binary path. Update the uat-aws workflow so the Setup Go step
is guarded the same way as Build aicr binary, using inputs.aicr_version == ''
(and skip_tests as applicable), or move Setup Go into the main-cell-only path
alongside the Build aicr binary step. Keep the release-only flow around
install-aicr-release and the existing Build and push validator images gating
consistent with this split.
In @.github/workflows/uat-gcp.yaml:
- Around line 115-143: Keep release cells independent of the source Go toolchain
by updating the workflow so the existing Setup Go step is only executed for main
cells, not when inputs.aicr_version is set. Reuse the same condition pattern
already used by Build aicr binary and Build and push validator images, and
ensure install-aicr-release remains the only path for release cells so a broken
Go pin cannot block them before the released binary is installed.
In @.github/workflows/uat-nightly-batch.yaml:
- Around line 129-136: Anchor the nightly batch deadline to a single batch-wide
timestamp instead of recomputing it per matrix leg. Update the workflow in the
UAT nightly batch job so the step that derives `deadline` uses a shared batch
start value captured once for the whole run, then reuse that value across all
matrix entries in the logic around `PREVIOUS_N` and `DEADLINE_OFFSET_HOURS`.
Make the same change in the duplicated section referenced by the comment so
every reservation leg stops dispatching at the same cutoff.
- Around line 198-208: The reservation loop in uat-nightly-batch.yaml is
swallowing real failures because the `gh run watch` call in the run-waiting
block is followed by `|| true`, so failed or timed-out `uat-run.yaml` cells
never affect the job result. Update the logic around the run-watching flow (the
`run_id`/`gh run watch` block) to preserve non-superseded failures: keep the
zero-job superseded handling, but explicitly capture the watch exit status and
propagate it unless the run was cancelled with zero jobs, so this leg fails when
a cell conclusion is failed or timed out.
In `@docs/contributor/uat.md`:
- Line 44: The release verification description is overstating the trusted
signer scope; update the `install-aicr-release` documentation to say the
attestation identity is derived from `aicr_version`, so only the exact release
tag’s `on-tag.yaml@refs/tags/*` identity is accepted. Keep the rest of the
integrity and provenance checks in place, but adjust the wording in the
release-cells section to reflect the exact tag-bound identity check instead of
implying any matching signer is trusted.
🪄 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: c995a072-830c-4231-9010-da94efb26994
📒 Files selected for processing (6)
.github/actions/install-aicr-release/action.yml.github/workflows/uat-aws.yaml.github/workflows/uat-gcp.yaml.github/workflows/uat-nightly-batch.yaml.github/workflows/uat-run.yamldocs/contributor/uat.md
3ff1a6e to
06a7590
Compare
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 145-148: The release-cell path still installs ko because Install
tools is passed install_ko: 'true' unconditionally, even though Build and push
validator images is gated off; update the install-tools invocation to disable ko
whenever the job is running a release cell (same conditions used around
aicr_version and skip_tests). Use the existing release-vs-main split in the uat
workflow and the Install tools step to ensure ko is only installed for the main
cell where it is actually consumed.
In @.github/workflows/uat-gcp.yaml:
- Around line 143-146: The workflow still installs ko unconditionally in the
“Install tools” step, so release cells can fail before UAT starts even though
the release-cell build is skipped elsewhere. Update the “Install tools” logic in
the same workflow to skip ko installation when the release-cell path is taken,
matching the existing conditional used by “Build and push validator images” and
the snapshot-agent build gate, so release cells do not download or rely on ko at
all.
In @.github/workflows/uat-nightly-batch.yaml:
- Around line 178-196: The run resolution logic in the uat-nightly-batch
workflow is over-filtering by comparing GitHub run `createdAt` against the
runner-local `since` timestamp. Remove the `createdAt` cutoff from the `gh run
list`/`jq` selection in the dispatch resolution loop, and rely on the unique
`displayTitle` match produced by `dispatch_key` in uat-run.yaml to identify the
correct run. Keep the polling/fail-fast flow intact, but stop rejecting valid
runs solely because of clock skew between the runner and GitHub.
🪄 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: 52d5abc6-6fae-4fe2-9607-7e37cfc63d94
📒 Files selected for processing (6)
.github/actions/install-aicr-release/action.yml.github/workflows/uat-aws.yaml.github/workflows/uat-gcp.yaml.github/workflows/uat-nightly-batch.yaml.github/workflows/uat-run.yamldocs/contributor/uat.md
06a7590 to
4c71fe9
Compare
There was a problem hiding this comment.
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 @.github/workflows/uat-nightly-batch.yaml:
- Around line 140-161: Validate and sanitize the `DEADLINE_OFFSET_HOURS` input
in the `uat-nightly-batch` workflow before it is used in the Bash arithmetic for
`deadline`. The issue is that `workflow_dispatch` can pass arbitrary strings and
negative values, which would make the cutoff earlier than `BATCH_START`. Add a
pre-check in the `run` block around the `deadline` calculation to reject
anything that is not a non-negative integer, and fail fast with a clear message
before computing the cutoff.
🪄 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: 25a9b21d-63e3-4a53-a7d7-033055fad192
📒 Files selected for processing (6)
.github/actions/install-aicr-release/action.yml.github/workflows/uat-aws.yaml.github/workflows/uat-gcp.yaml.github/workflows/uat-nightly-batch.yaml.github/workflows/uat-run.yamldocs/contributor/uat.md
4c71fe9 to
e6d6157
Compare
|
@njhensley this PR now has merge conflicts with |
mchmarny
left a comment
There was a problem hiding this comment.
Strong PR — the least-privilege split (drive job dropped to contents:read + actions:write, dispatched runs go top-level with their own perms) and the install-aicr-release provenance verification are well done: fails closed on a missing attestation, tag-pinned identity regexp with an injection guard and escaped dots, and the checksum grep is safe because a no-match grep trips pipefail. Release cells correctly skip image cleanup so shared tags aren't deleted. One non-blocking medium on the controller's time-box vs the 6h job ceiling, plus a nit — both inline. Nothing blocks merge; the medium is worth a look before the first nightly since that's when it'll bite.
Turn the nightly UAT batch into a controller that runs a cross-version regression per reservation: main (built from source) plus the previous N stable releases, ordered by `uat-broker schedule` and time-boxed. - uat-nightly-batch.yaml: controller loop — one matrix leg per reservation (parallel), each expanding an ordered, deadline-bounded schedule and dispatching uat-run.yaml per cell sequentially (shared reservation lease), resolving the dispatched run by run-name and watching it; drop-oldest at the deadline; controller-side superseded reconcile. - uat-run.yaml: aicr_version input (dispatch + call), threaded to the per-cloud callers; run-name "UAT <reservation> @ <version>" so the controller can discover the dispatched run. - uat-aws.yaml / uat-gcp.yaml: aicr_version workflow_call input. main cells build from source; release cells install the released binary and let it self-resolve its version's validator/agent images (no image builds). - .github/actions/install-aicr-release: shared composite action that downloads, checksum-verifies, and provenance-verifies the released binary (cosign verify-blob-attestation against the archived SLSA Build Provenance v1 bundle, pinned to NVIDIA CI's keyless signer) before use — fail-closed. - docs/contributor/uat.md: version-matrix section, tunables, roadmap update. Related NVIDIA#1274, NVIDIA#1264. Signed-off-by: Nathan Hensley <[email protected]>
e03b9ac to
f205b83
Compare
Summary
Turns the nightly UAT batch into a controller that runs a per-reservation version matrix —
main(built from source) plus the previous N stable releases — where release cells install a checksum- and provenance-verified released binary instead of rebuilding from source.Motivation / Context
Third PR in the DC1 day/night scheduler series (#1274), folding in the DC5 versioned-install slice. Builds on the reservation broker (#1559) and the broker cutover (#1569, both merged). The nightly side now exercises an older stable
aicragainst today's cluster, catching cross-version regressions the single-mainbatch never would.Fixes: N/A (further DC issues remain in the epic)
Related: #1274, #1264
Type of Change
Component(s) Affected
docs/,examples/).github/workflows/uat-*.yaml,.github/actions/install-aicr-release)Implementation Notes
uat-nightly-batch.yaml→ controller loop. One matrix leg per reservation (parallel across reservations, sequential within). Each leg expands an ordered, deadline-bounded schedule viagit tag | uat-broker schedule --previous-n, dispatchesuat-run.yamlper cell (so cells share the per-reservation lease), resolves the dispatched run by itsrun-name, andgh run watches it. Drop-oldest at the deadline; controller-side reconcile surfaces a superseded (pending-cancelled, zero-job) run. Dispatch (notworkflow_call) keepsuat-run.yamltop-level, so reusable-nesting stays at 3 levels.aicr_versionthreading. New input onuat-run.yaml(dispatch + call) and the per-cloud callers.run-name: UAT <reservation> @ <version>is the coupling the controller uses to discover the run it just dispatched (gh workflow runreturns no id).maincells build the binary + validator/agent images from the tree. Release cells (aicr_version=vX.Y.Z) download the released binary and let it self-resolve its version's validator images (pkg/validator/catalogrewrites:latest → :vX.Y.Z) and agent (ghcr.io/nvidia/aicr:vX.Y.Z) — no images are built.install-aicr-releasecomposite action (new). Shared by both cloud impls. Fails closed on two checks before use: (1) integrity — the archive matches itsaicr_checksums.txtentry; (2) provenance —cosign verify-blob-attestationvalidates the SLSA Build Provenance v1 bundle goreleaser ships inside the archive (aicr-attestation.sigstore.json), pinned to NVIDIA CI's keyless signer (on-tag.yaml@refs/tags/*, issuertoken.actions.githubusercontent.com). The attestation's subject is the binary's own digest, so authenticity binds to the exact bytes that run — not to the same-release checksums manifest. Verify needs noid-token(only signing does).uat-nightly-batch.yaml):previous_n(default2),deadline_offset_hours(default10).Testing
actionlintclean on all four consuming workflows.yamllintclean on the new action and the workflows.bash -nclean on the composite action's embedded script; confirmed the YAML-block → bash double-quote layering yields a literal-doton-tag\.yamlregex (matches a real tag ref, rejectson-tagXyaml).gh workflow run+ run-id-by-run-name discovery +gh run watch) is novel and will be validated on the first nightly / manual dispatch.Risk Assessment
Rollout notes: No default-behavior change until releases with the shipped attestation are exercised;
previous_ndefaults to2.uat-run.yamlstays top-level, so signing identities and evidence ingest are unchanged from PR-2. Easy to revert (additive workflows + one composite action).Checklist
make testwith-race) — N/A, no Go source changedactionlint+yamllint; no Go →make lintGo gate N/A)docs/contributor/uat.md)git commit -S)