ci(uat): enroll azure-h100 inference in nightly batch#1776
Conversation
njhensley
added
the
theme/ci-dx
|
🌿 Preview your docs: https://nvidia-preview-ci-azure-nightly-inference.docs.buildwithfern.com/aicr |
📝 WalkthroughWalkthroughNightly UAT scheduling now supports per-intent minimum release-version gates. Reservation models and registry validation enforce valid gates, schedule cells carry eligible intents, and the nightly workflow skips fully gated release cells. Azure H100 now runs training and inference nightly, with inference gated at Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/contributor/uat.md`:
- Around line 61-67: Update the heading at the section around the nightly
reservation table to replace the stale “both clouds” wording with “all three
clouds” or “all nightly-enrolled reservations,” matching the AWS, GCP, and Azure
coverage described by the table.
🪄 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: 58efd560-d5b1-42c9-9555-10da2d082446
📒 Files selected for processing (3)
docs/contributor/uat.mdinfra/uat/reservations.yamlpkg/uatbroker/registry_test.go
mchmarny
left a comment
mchmarny
left a comment
There was a problem hiding this comment.
The enrollment mechanism is straightforward, but its acceptance prerequisites are not met. The main inference run is not green end-to-end, and the newly scheduled v0.17.0 inference cell predates the required tuning fix. Both block merge despite the required CI gate passing.
mchmarny
left a comment
mchmarny
left a comment
There was a problem hiding this comment.
Inline details for the blocking request-changes review above.
mchmarny
left a comment
mchmarny
left a comment
There was a problem hiding this comment.
Inline evidence for the blocking review.
| nightly-intents: [training] | ||
| # joined after a green manual intent=inference dispatch, matching the | ||
| # AWS/GCP rows which run both. | ||
| nightly-intents: [training, inference] |
There was a problem hiding this comment.
blocker: The prerequisite stated above is not met. Run 29125390442 was a training run, and the latest manual Azure inference run (29448415427) finished in failure at evidence verification. This enrollment therefore adds a predictably red nightly main cell. Keep inference unenrolled until a full provision → validate → verify → teardown run succeeds, and link that run here.
| ### Nightly intent cadence (both intents, both clouds) | ||
|
|
||
| The single nightly cron (`uat-nightly-batch.yaml`, `0 4 * * *`) runs **both intents on every nightly-enrolled reservation**, so training *and* inference are exercised nightly on AWS and GCP, and training on Azure (`azure-h100` enrolls `[training]` only until a manual inference run goes green — see the table below). (Note: an inference cell currently provisions and validates the inference platform; the `phase_serve` serving-CUJ step itself remains disabled in both cloud workflows pending #1644, so nightly runs do not yet execute the serving request path.) The set of intents per reservation is data — the `nightly-intents` list in `infra/uat/reservations.yaml` (absent defaults to `[training]`; an explicit empty list `[]` opts the reservation out of the nightly batch entirely — bring-up mode, manual dispatch only): | ||
| The single nightly cron (`uat-nightly-batch.yaml`, `0 4 * * *`) runs **both intents on every nightly-enrolled reservation**, so training *and* inference are exercised nightly on AWS, GCP, and Azure (see the table below). (Note: an inference cell currently provisions and validates the inference platform; the `phase_serve` serving-CUJ step itself remains disabled in both cloud workflows pending #1644, so nightly runs do not yet execute the serving request path.) The set of intents per reservation is data — the `nightly-intents` list in `infra/uat/reservations.yaml` (absent defaults to `[training]`; an explicit empty list `[]` opts the reservation out of the nightly batch entirely — bring-up mode, manual dispatch only): |
There was a problem hiding this comment.
blocker: The nightly controller defaults to previous_n=1, so this also enables the v0.17.0 inference cell. That release pins nvidia-tuned 0.3.1; the Azure performance fix arrived in 0.3.2 after v0.17.0. Exclude unsupported release cells for this reservation/intent or demonstrate a green v0.17.0 inference run before enrolling it.
63e198e to
1d7e8a5
Compare
njhensley
added
theme/recipes
|
Pushed a revision that addresses the review. @mchmarny's blocker B (v0.17.0 predates the fix) — resolved by mechanism. Rather than a one-off exclusion, added a data-driven nightly-intent-min-versions:
inference: v0.18.0 # nvidia-tuned 0.3.2 (#1767) is on main, not yet in a release tag
CodeRabbit — heading fix applied ("both clouds" → "all three clouds"). Still open — blocker A (green end-to-end acceptance run). The perf constraint is now green on |
There was a problem hiding this comment.
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 `@pkg/uatbroker/registry_test.go`:
- Around line 638-640: Update the committed-registry test entry for azure-h100
in the registry assertion to require the v0.18.0 minimum for IntentInference,
while preserving the existing IntentTraining requirement and inference intent.
In `@tools/uat-broker/README.md`:
- Around line 75-84: Add a brief note to the `schedule --previous-n 2` README
example indicating that entries for other reservations are omitted for brevity,
while preserving the shown `azure-h100` output and command.
🪄 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: faad136f-c731-4121-bd83-1c2880062f7d
📒 Files selected for processing (12)
.github/workflows/uat-nightly-batch.yamldocs/contributor/uat.mdinfra/uat/reservations.yamlpkg/uatbroker/doc.gopkg/uatbroker/model.gopkg/uatbroker/registry.gopkg/uatbroker/registry_test.gopkg/uatbroker/schedule.gopkg/uatbroker/schedule_test.gotools/uat-broker/README.mdtools/uat-broker/main.gotools/uat-broker/main_test.go
| ```sh | ||
| git tag -l 'v*' | uat-broker schedule --previous-n 2 | ||
| # { | ||
| # "aws-h100": [ | ||
| # { "reservation": "aws-h100", "aicr_version": "", "is_main": true }, | ||
| # { "reservation": "aws-h100", "aicr_version": "v2.0.0", "is_main": false }, | ||
| # { "reservation": "aws-h100", "aicr_version": "v1.10.0", "is_main": false } | ||
| # ], | ||
| # "gcp-h100": [ ... ] | ||
| # "azure-h100": [ | ||
| # { "reservation": "azure-h100", "aicr_version": "", "is_main": true, "intents": ["training","inference"] }, | ||
| # { "reservation": "azure-h100", "aicr_version": "v2.0.0", "is_main": false, "intents": ["training","inference"] }, | ||
| # { "reservation": "azure-h100", "aicr_version": "v0.17.0", "is_main": false, "intents": ["training"] } | ||
| # ] | ||
| # } | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Clarify the truncated multi-reservation example.
The example runs schedule --previous-n 2 with no --reservations filter, so the real output would include every registry row (aws-h100, gcp-h100, azure-h100) as top-level keys — but only azure-h100 is shown. Add a short note that other reservations' entries are omitted for brevity, so readers don't mistake this for the full output shape.
📝 Proposed clarification
```sh
git tag -l 'v*' | uat-broker schedule --previous-n 2
-# {
+# { # (other reservations' entries omitted here for brevity)
# "azure-h100": [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```sh | |
| git tag -l 'v*' | uat-broker schedule --previous-n 2 | |
| # { | |
| # "aws-h100": [ | |
| # { "reservation": "aws-h100", "aicr_version": "", "is_main": true }, | |
| # { "reservation": "aws-h100", "aicr_version": "v2.0.0", "is_main": false }, | |
| # { "reservation": "aws-h100", "aicr_version": "v1.10.0", "is_main": false } | |
| # ], | |
| # "gcp-h100": [ ... ] | |
| # "azure-h100": [ | |
| # { "reservation": "azure-h100", "aicr_version": "", "is_main": true, "intents": ["training","inference"] }, | |
| # { "reservation": "azure-h100", "aicr_version": "v2.0.0", "is_main": false, "intents": ["training","inference"] }, | |
| # { "reservation": "azure-h100", "aicr_version": "v0.17.0", "is_main": false, "intents": ["training"] } | |
| # ] | |
| # } | |
| ``` |
🤖 Prompt for 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.
In `@tools/uat-broker/README.md` around lines 75 - 84, Add a brief note to the
`schedule --previous-n 2` README example indicating that entries for other
reservations are omitted for brevity, while preserving the shown `azure-h100`
output and command.
|
Update on blocker A (green end-to-end acceptance run): it is not an issue with this PR — it's a separate The Azure inference acceptance runs (and every other This PR touches nothing in the evidence path, so its correctness is independent. Leaving it open pending #1793 — once |
|
@njhensley this PR now has merge conflicts with |
Add nightly-intent-min-versions to the UAT reservation registry: a per-intent minimum AICR release that gates RELEASE cells of the nightly version matrix. The tip-of-main cell always runs every listed intent; a release cell drops any intent whose minimum version is newer than the tag (semver). This generalizes the informal 'a released version that predates a platform is a premature cell' guidance into checked config, so a release that predates an intent's support is skipped instead of running a permanently-red cell. The gate is resolved in the schedule — uat-broker attaches each cell's eligible intents — so the nightly controller dispatches a cell's own intents with no per-version semver logic in workflow bash. Validate rejects a gate on an unrun/unknown intent or a non-semver value. Enroll azure-h100 for [training, inference] with inference gated to v0.18.0: the AKS ND H100 v5 perf fix (nvidia-tuned 0.3.2, NVIDIA#1767) is on main but not yet in a release tag (v0.17.0 pins 0.3.1). Today only main runs Azure inference nightly (green); older releases are skipped and the gated release enrolls automatically once it ships. Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
497fcb8 to
6fff9f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/uatbroker/registry_test.go (1)
639-641: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPin the Azure inference minimum in the committed-registry guard.
Requiring the inference intent without asserting
NightlyIntentMinVersions[IntentInference] == "v0.18.0"allows the release safety gate to be removed unnoticed.Proposed assertion
got := res.NightlyIntentsOrDefault() if !slices.Equal(got, want) { t.Errorf("committed registry nightly-intents[%q] = %v, want %v", name, got, want) } + if name == "azure-h100" && + res.NightlyIntentMinVersions[IntentInference] != "v0.18.0" { + t.Errorf("azure inference minimum = %q, want v0.18.0", + res.NightlyIntentMinVersions[IntentInference]) + }🤖 Prompt for 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. In `@pkg/uatbroker/registry_test.go` around lines 639 - 641, Update the committed-registry guard for the "azure-h100" entry to assert that NightlyIntentMinVersions[IntentInference] remains exactly "v0.18.0", while preserving its existing IntentTraining and IntentInference requirements.
🤖 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 `@infra/uat/reservations.yaml`:
- Around line 108-110: Keep Azure inference unenrolled in
infra/uat/reservations.yaml until the complete provision, validation,
evidence-verification, and teardown acceptance flow succeeds; remove or replace
the unsupported green inference claim there. Also update docs/contributor/uat.md
to remove the claim that main-only inference coverage is green until that
acceptance run passes.
---
Duplicate comments:
In `@pkg/uatbroker/registry_test.go`:
- Around line 639-641: Update the committed-registry guard for the "azure-h100"
entry to assert that NightlyIntentMinVersions[IntentInference] remains exactly
"v0.18.0", while preserving its existing IntentTraining and IntentInference
requirements.
🪄 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: 9bda8457-7985-484d-af02-c6ab873e9757
📒 Files selected for processing (12)
.github/workflows/uat-nightly-batch.yamldocs/contributor/uat.mdinfra/uat/reservations.yamlpkg/uatbroker/doc.gopkg/uatbroker/model.gopkg/uatbroker/registry.gopkg/uatbroker/registry_test.gopkg/uatbroker/schedule.gopkg/uatbroker/schedule_test.gotools/uat-broker/README.mdtools/uat-broker/main.gotools/uat-broker/main_test.go
| # joined after a green manual intent=inference dispatch, matching the | ||
| # AWS/GCP rows which run both. | ||
| nightly-intents: [training, inference] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Azure inference still lacks a clean end-to-end acceptance run.
The v0.18.0 floor fixes unsupported release cells, but it does not gate main; the PR objectives report that the latest inference run failed evidence verification.
infra/uat/reservations.yaml#L108-L110: keep inference unenrolled until provision → validation → evidence verification → teardown succeeds, and replace the unsupported “green” claim.docs/contributor/uat.md#L73-L82: remove the claim that main-only inference coverage is green until that acceptance run passes.
📍 Affects 2 files
infra/uat/reservations.yaml#L108-L110(this comment)docs/contributor/uat.md#L73-L82
🤖 Prompt for 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.
In `@infra/uat/reservations.yaml` around lines 108 - 110, Keep Azure inference
unenrolled in infra/uat/reservations.yaml until the complete provision,
validation, evidence-verification, and teardown acceptance flow succeeds; remove
or replace the unsupported green inference claim there. Also update
docs/contributor/uat.md to remove the claim that main-only inference coverage is
green until that acceptance run passes.
2 participants
Summary
Enroll
azure-h100fortraining+inferencein the nightly UAT batch, and add a data-drivennightly-intent-min-versionsgate so an intent that only became supported at a given release is not run on the releases that predate it (which would produce a permanently-red cell rather than a regression signal).Motivation / Context
Azure inference was held out of the nightly batch pending an AKS ND H100 v5
inference-perfdeficit. That deficit is fixed onmainby nvidia-tuned 0.3.2 (#1767), but the fix is not in any release tag yet —v0.17.0and older pin 0.3.1. Withprevious_n=1the nightly batch runsmain+ the newest release, so naively enrolling inference would run a predictably-redv0.17.0Azure inference cell every night. As more versions/platforms land, this "release predates support" case recurs — hence a formal minimum-version expression rather than a one-off.Related: #1644 (serve CUJ), #1767 (perf fix)
Type of Change
Component(s) Affected
docs/,examples/)infra/uat,pkg/uatbroker,tools/uat-broker), nightly controllerImplementation Notes
nightly-intent-min-versions— a per-intent map on the reservation row:mainis never gated (built from source, carries the newest fixes → always runs every listed intent). Only release cells are gated: a tag below an intent's minimum is dropped for that intent (semver;tag >= minruns).uat-broker scheduleattaches each cell's eligibleintents— so the nightly controller dispatches a cell's own intents with no semver logic in workflow bash.Validatefails closed on a gate for an unrun/unknown intent or a non-semver value.mainonly (green), then the release enrolls automatically.Verified against the real registry (no
v0.18.0tag exists today): azuremainruns training+inference;v0.17.0/v0.16.0run training only;aws-h100/gcp-h100unaffected.Testing
New tests:
EligibleNightlyIntents(main/at/above/below/ungated/unparseable),ExpandSchedulegate flow-through, per-cell intents via the tool, and registry validation (unrun/unknown intent, non-semver, valid).Risk Assessment
Rollout notes: Removing the
nightly-intent-min-versionsblock restores unconditional per-intent runs. No migration.Checklist
make testwith-race)make lint)git commit -S)