feat(recipe): deliver deployment-phase floor at per-accelerator wildcards#1001
Merged
mchmarny merged 1 commit intoMay 26, 2026
Merged
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 extends recipe classification with accelerator awareness and establishes deployment-phase validation baselines for three new accelerator-specific recipe overlays. The test infrastructure gains an Accelerator field and helpers to require deployment-phase gpu-operator-version gating for accelerator-bound overlays. Three overlays (GB200, H100, RTX Pro 6000) provide a standardized deployment-phase floor including operator-version constraints. The test matrix is rewritten to cover accelerator combinations and the knownGaps allowlist is cleared. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced May 21, 2026
Closed
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 21, 2026
…ards (NVIDIA#969) Push the deployment-phase floor — both the 4 standard checks AND the gpu-operator version constraint — to per-accelerator wildcard overlays so every accelerator-bound query inherits a constraint-backed phase. Closes the gap surfaced by Codex's review: PR NVIDIA#1001 v1 added the check name at the service-root, but per-phase replace semantics meant descendants without their own deployment block (e.g., gb200-oke-training) inherited the check name without a Deployment.gpu-operator.version constraint. The gpu-operator-version check is a no-op without that constraint, so the recipe shipped with the check name but no version assertion. Changes - recipes/overlays/{h100,gb200,rtx-pro-6000}-any.yaml: new wildcard overlays (service: any, accelerator: <X>) carrying the full deployment block (4 checks + version constraint). Concrete leaves that declare their own deployment block continue to override via per-phase replace; constraint values match the wildcard floor. - recipes/overlays/{aks,eks,gke-cos,kind,lke,oke}.yaml: revert the service-root deployment block added in v1. Service-roots own the service-scoped phase (conformance); accelerator-scoped phases live on the per-accelerator wildcards. The applied-overlay order (wildcards then base chain) means a service-root deployment block would shadow the wildcard's, so v1's service-root block was the wrong location. - pkg/recipe/validation_phase_floor_test.go: assert Deployment.gpu-operator.version constraint is present whenever the deployment phase is required; exempt accelerator-unbound classifications from requiring deployment + performance phases (intermediates without an accelerator: criterion have no meaningful accelerator-specific threshold and concrete leaves carry the constraint via wildcard composition). Add isAcceleratorBound, requiresDeployment, and hasGPUOperatorVersionConstraint helpers. Update TestClassifyOverlay to cover the accelerator-unbound exempt path. Verification - TestOverlayValidationPhaseFloor passes for all 56 gateable overlays in both default and strict modes; knownGaps stays empty. - aicr recipe --service oke --accelerator gb200 --intent training (the Codex-reported regression case) now resolves with Deployment.gpu-operator.version >= v25.10.0. - Same query against RTX Pro 6000 LKE and H100 Kind also resolves with their respective version constraints. - make qualify passes. Follow-up NVIDIA#1000 will switch validation.{phase}.checks/constraints to per-field union merge so concrete leaves can declare just the incremental additions instead of repeating the wildcard's full block.
yuanchen8911
force-pushed
the
fix/969-deployment-phase-floor-service-root
branch
from
116cd6e to
9595676
Compare
yuanchen8911
changed the title
![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 `@pkg/recipe/validation_phase_floor_test.go`:
- Around line 121-136: The helper hasGPUOperatorVersionConstraint currently only
checks for a Deployment.gpu-operator.version constraint; change it to require
both the deployment check named "gpu-operator-version" and the constraint
"Deployment.gpu-operator.version" are present on the ValidationPhase (i.e.,
iterate p.Checks and p.Constraints and return true only if both are found).
Update the same helper/duplicate occurrence elsewhere in this file (the other
hasGPUOperatorVersionConstraint implementation) so tests assert both the
presence of the check and its version constraint.
🪄 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: acbde46d-9041-478c-9d9c-5dded44874d5
📒 Files selected for processing (4)
pkg/recipe/validation_phase_floor_test.gorecipes/overlays/gb200-any.yamlrecipes/overlays/h100-any.yamlrecipes/overlays/rtx-pro-6000-any.yaml
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 21, 2026
…ards (NVIDIA#969) Push the deployment-phase floor — both the 4 standard checks AND the gpu-operator version constraint — to per-accelerator wildcard overlays so every accelerator-bound query inherits a constraint-backed phase. Closes the gap surfaced by Codex's review: PR NVIDIA#1001 v1 added the check name at the service-root, but per-phase replace semantics meant descendants without their own deployment block (e.g., gb200-oke-training) inherited the check name without a Deployment.gpu-operator.version constraint. The gpu-operator-version check is a no-op without that constraint, so the recipe shipped with the check name but no version assertion. Changes - recipes/overlays/{h100,gb200,rtx-pro-6000}-any.yaml: new wildcard overlays (service: any, accelerator: <X>) carrying the full deployment block (4 checks + version constraint). Concrete leaves that declare their own deployment block continue to override via per-phase replace; constraint values match the wildcard floor. - recipes/overlays/{aks,eks,gke-cos,kind,lke,oke}.yaml: revert the service-root deployment block added in v1. Service-roots own the service-scoped phase (conformance); accelerator-scoped phases live on the per-accelerator wildcards. The applied-overlay order (wildcards then base chain) means a service-root deployment block would shadow the wildcard's, so v1's service-root block was the wrong location. - pkg/recipe/validation_phase_floor_test.go: assert Deployment.gpu-operator.version constraint is present whenever the deployment phase is required; exempt accelerator-unbound classifications from requiring deployment + performance phases (intermediates without an accelerator: criterion have no meaningful accelerator-specific threshold and concrete leaves carry the constraint via wildcard composition). Add isAcceleratorBound, requiresDeployment, and hasGPUOperatorVersionConstraint helpers. Update TestClassifyOverlay to cover the accelerator-unbound exempt path. Verification - TestOverlayValidationPhaseFloor passes for all 56 gateable overlays in both default and strict modes; knownGaps stays empty. - aicr recipe --service oke --accelerator gb200 --intent training (the Codex-reported regression case) now resolves with Deployment.gpu-operator.version >= v25.10.0. - Same query against RTX Pro 6000 LKE and H100 Kind also resolves with their respective version constraints. - make qualify passes. Follow-up NVIDIA#1000 will switch validation.{phase}.checks/constraints to per-field union merge so concrete leaves can declare just the incremental additions instead of repeating the wildcard's full block.
yuanchen8911
force-pushed
the
fix/969-deployment-phase-floor-service-root
branch
from
9595676 to
1aafaf0
Compare
mchmarny
previously approved these changes
May 26, 2026
…ards (NVIDIA#969) Push the deployment-phase floor — both the 4 standard checks AND the gpu-operator version constraint — to per-accelerator wildcard overlays so every accelerator-bound query inherits a constraint-backed phase. Closes the gap surfaced by Codex's review: PR NVIDIA#1001 v1 added the check name at the service-root, but per-phase replace semantics meant descendants without their own deployment block (e.g., gb200-oke-training) inherited the check name without a Deployment.gpu-operator.version constraint. The gpu-operator-version check is a no-op without that constraint, so the recipe shipped with the check name but no version assertion. Changes - recipes/overlays/{h100,gb200,rtx-pro-6000}-any.yaml: new wildcard overlays (service: any, accelerator: <X>) carrying the full deployment block (4 checks + version constraint). Concrete leaves that declare their own deployment block continue to override via per-phase replace; constraint values match the wildcard floor. - recipes/overlays/{aks,eks,gke-cos,kind,lke,oke}.yaml: revert the service-root deployment block added in v1. Service-roots own the service-scoped phase (conformance); accelerator-scoped phases live on the per-accelerator wildcards. The applied-overlay order (wildcards then base chain) means a service-root deployment block would shadow the wildcard's, so v1's service-root block was the wrong location. - pkg/recipe/validation_phase_floor_test.go: assert Deployment.gpu-operator.version constraint is present whenever the deployment phase is required; exempt accelerator-unbound classifications from requiring deployment + performance phases (intermediates without an accelerator: criterion have no meaningful accelerator-specific threshold and concrete leaves carry the constraint via wildcard composition). Add isAcceleratorBound, requiresDeployment, and hasGPUOperatorVersionConstraint helpers. Update TestClassifyOverlay to cover the accelerator-unbound exempt path. Verification - TestOverlayValidationPhaseFloor passes for all 56 gateable overlays in both default and strict modes; knownGaps stays empty. - aicr recipe --service oke --accelerator gb200 --intent training (the Codex-reported regression case) now resolves with Deployment.gpu-operator.version >= v25.10.0. - Same query against RTX Pro 6000 LKE and H100 Kind also resolves with their respective version constraints. - make qualify passes. Follow-up NVIDIA#1000 will switch validation.{phase}.checks/constraints to per-field union merge so concrete leaves can declare just the incremental additions instead of repeating the wildcard's full block.
yuanchen8911
force-pushed
the
fix/969-deployment-phase-floor-service-root
branch
from
4d15392 to
0a8d747
Compare
mchmarny
approved these changes
May 26, 2026
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/recipe/validation_phase_floor_test.go (1)
27-31: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUpdate floor documentation to reflect accelerator-bound requirement.
The "Per-intent floor" comment describes deployment as required for all Training/Inference overlays, but the implementation now only requires deployment for accelerator-bound classifications (see
requiresDeployment()at line 94 and the test comment at lines 284-286). Consider updating the file-level summary to clarify that the listed floors apply to accelerator-bound classifications; accelerator-unbound intermediates are exempt per the PR objectives.📝 Suggested documentation update
// Per-intent floor: -// Training (non-Kind) : deployment + conformance [performance recommended] -// Inference Dynamo / NIM (non-Kind) : deployment + conformance [performance recommended] -// Inference (plain) : deployment + conformance -// Kind (any intent) : deployment + conformance +// Training (non-Kind, accelerator-bound) : deployment + conformance [performance recommended] +// Inference Dynamo / NIM (non-Kind, accelerator-bound) : deployment + conformance [performance recommended] +// Inference (plain, accelerator-bound) : deployment + conformance +// Kind (any intent, accelerator-bound) : deployment + conformance +// +// Accelerator-unbound classifications (no accelerator criterion or accelerator=any) +// are exempt from deployment+performance because those phases contain accelerator-specific +// values (gpu-operator version pin, NCCL thresholds) that live on accelerator wildcards.🤖 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/recipe/validation_phase_floor_test.go` around lines 27 - 31, Update the file-level "Per-intent floor" comment to state that the listed deployment + conformance floors apply only to accelerator-bound classifications (i.e., those for which requiresDeployment() returns true) and that accelerator-unbound intermediates are exempt; reference the behavior implemented in requiresDeployment() and the related test cases in validation_phase_floor_test.go to ensure wording matches the actual logic.
🤖 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.
Outside diff comments:
In `@pkg/recipe/validation_phase_floor_test.go`:
- Around line 27-31: Update the file-level "Per-intent floor" comment to state
that the listed deployment + conformance floors apply only to accelerator-bound
classifications (i.e., those for which requiresDeployment() returns true) and
that accelerator-unbound intermediates are exempt; reference the behavior
implemented in requiresDeployment() and the related test cases in
validation_phase_floor_test.go to ensure wording matches the actual logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 5e90368f-39af-4d04-b787-6c8ec2b62b3e
📒 Files selected for processing (4)
pkg/recipe/validation_phase_floor_test.gorecipes/overlays/gb200-any.yamlrecipes/overlays/h100-any.yamlrecipes/overlays/rtx-pro-6000-any.yaml
10 tasks
This was referenced May 27, 2026
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 27, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 27, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
11 tasks
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 27, 2026
The criteria-wildcard overlay `gb200-any-training.yaml` carried a single `nccl-all-reduce-bw >= 720` constraint applied to every GB200 + training query regardless of service. The companion `b200-any-training.yaml` had the same shape. The pattern is misleading: each service has a different network fabric (EFA on EKS, TCPXO on GKE, RoCE on OKE) and a single cross-service threshold is rarely correct for two fabrics. NCCL bandwidth thresholds belong on the concrete service-bound leaf, anchored to a measurement on that specific fabric. The fabric-independent deployment-phase floor (4 standard health checks + `Deployment.gpu-operator.version` pin) remains on `gb200-any.yaml` — that pattern (established by NVIDIA#1001) is correct because the same gpu-operator version requirement applies across every service for the accelerator. Changes: - Delete `recipes/overlays/gb200-any-training.yaml`. - Update `recipes/overlays/gb200-any.yaml` comment block: drop the "Companion to gb200-any-training.yaml" intro, explain why the intent-scoped sibling was retired. - Doc updates with the same rationale: - `docs/integrator/recipe-development.md` — switch the criteria-wildcard example from `gb200-any-training.yaml` to `gb200-any.yaml` (deployment-phase floor); add the "per-fabric values don't belong here" caveat. - `docs/contributor/data.md` — refresh the wildcard explanation section, the resolver-tracing example, and the Mermaid flowchart to use `gb200-any` throughout; rename `-any-` naming convention to allow `-any` (deployment-floor pattern). - `docs/design/005-overlay-refactoring.md` — drop the `b200-any-training` / `gb200-any-training` lines from the overlay tree and leave a brief historical note explaining the retirement across PRs NVIDIA#1004 (b200 wildcard) and NVIDIA#1052 (gb200 wildcard). - Update the `knownGaps` header comment in `pkg/recipe/validation_phase_floor_test.go` to document the OKE GB200 training performance data gap (warn-only in non-strict mode today; closes when NVIDIA#1007 lands an OCI testbed measurement). The map stays empty: the floor test treats missing performance as a `t.Log(WARN)` in default mode, so a knownGaps entry would be stale-flagged. The `b200-any-training.yaml` deletion is in flight via NVIDIA#1004 (PR NVIDIA#1053); the two issues are independent. Acceptance (per NVIDIA#1052): 1. `recipes/overlays/gb200-any-training.yaml` removed: yes. 2. `gb200-oke-training` perf coverage: option (b) — deferred to NVIDIA#1007 for real OCI measurements; covered as a warn-only floor gap today. 3. `go test ./pkg/recipe/... -run TestOverlayValidationPhaseFloor`: passes. 4. `aicr recipe --service oke --accelerator gb200 --intent training --format yaml`: resolves to 11 components, 6 overlays (base, monitoring-hpa, gb200-any, oke, oke-training, gb200-oke-training); carries the gpu-operator v25.10.0 floor and the standard 4 deployment checks via `gb200-any.yaml`. 5. Doc references updated. 6. `make qualify` clean. Fixes NVIDIA#1052 Related NVIDIA#1004, NVIDIA#1007
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 27, 2026
The criteria-wildcard overlay `gb200-any-training.yaml` carried a single `nccl-all-reduce-bw >= 720` constraint applied to every GB200 + training query regardless of service. The companion `b200-any-training.yaml` had the same shape. The pattern is misleading: each service has a different network fabric (EFA on EKS, TCPXO on GKE, RoCE on OKE) and a single cross-service threshold is rarely correct for two fabrics. NCCL bandwidth thresholds belong on the concrete service-bound leaf, anchored to a measurement on that specific fabric. The fabric-independent deployment-phase floor (4 standard health checks + `Deployment.gpu-operator.version` pin) remains on `gb200-any.yaml` — that pattern (established by NVIDIA#1001) is correct because the same gpu-operator version requirement applies across every service for the accelerator. Changes: - Delete `recipes/overlays/gb200-any-training.yaml`. - Update `recipes/overlays/gb200-any.yaml` comment block: drop the "Companion to gb200-any-training.yaml" intro, explain why the intent-scoped sibling was retired. - Doc updates with the same rationale: - `docs/integrator/recipe-development.md` — switch the criteria-wildcard example from `gb200-any-training.yaml` to `gb200-any.yaml` (deployment-phase floor); add the "per-fabric values don't belong here" caveat. - `docs/contributor/data.md` — refresh the wildcard explanation section, the resolver-tracing example, and the Mermaid flowchart to use `gb200-any` throughout; rename `-any-` naming convention to allow `-any` (deployment-floor pattern). - `docs/design/005-overlay-refactoring.md` — drop the `b200-any-training` / `gb200-any-training` lines from the overlay tree and leave a brief historical note explaining the retirement across PRs NVIDIA#1004 (b200 wildcard) and NVIDIA#1052 (gb200 wildcard). - Update the `knownGaps` header comment in `pkg/recipe/validation_phase_floor_test.go` to document the OKE GB200 training performance data gap (warn-only in non-strict mode today; closes when NVIDIA#1007 lands an OCI testbed measurement). The map stays empty: the floor test treats missing performance as a `t.Log(WARN)` in default mode, so a knownGaps entry would be stale-flagged. The `b200-any-training.yaml` deletion is in flight via NVIDIA#1004 (PR NVIDIA#1053); the two issues are independent. Acceptance (per NVIDIA#1052): 1. `recipes/overlays/gb200-any-training.yaml` removed: yes. 2. `gb200-oke-training` perf coverage: option (b) — deferred to NVIDIA#1007 for real OCI measurements; covered as a warn-only floor gap today. 3. `go test ./pkg/recipe/... -run TestOverlayValidationPhaseFloor`: passes. 4. `aicr recipe --service oke --accelerator gb200 --intent training --format yaml`: resolves to 11 components, 6 overlays (base, monitoring-hpa, gb200-any, oke, oke-training, gb200-oke-training); carries the gpu-operator v25.10.0 floor and the standard 4 deployment checks via `gb200-any.yaml`. 5. Doc references updated. 6. `make qualify` clean. Fixes NVIDIA#1052 Related NVIDIA#1004, NVIDIA#1007
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 27, 2026
The criteria-wildcard overlay `gb200-any-training.yaml` carried a single `nccl-all-reduce-bw >= 720` constraint applied to every GB200 + training query regardless of service. The companion `b200-any-training.yaml` had the same shape. The pattern is misleading: each service has a different network fabric (EFA on EKS, TCPXO on GKE, RoCE on OKE) and a single cross-service threshold is rarely correct for two fabrics. NCCL bandwidth thresholds belong on the concrete service-bound leaf, anchored to a measurement on that specific fabric. The fabric-independent deployment-phase floor (4 standard health checks + `Deployment.gpu-operator.version` pin) remains on `gb200-any.yaml` — that pattern (established by NVIDIA#1001) is correct because the same gpu-operator version requirement applies across every service for the accelerator. Changes: - Delete `recipes/overlays/gb200-any-training.yaml`. - Update `recipes/overlays/gb200-any.yaml` comment block: drop the "Companion to gb200-any-training.yaml" intro, explain why the intent-scoped sibling was retired. - Doc updates with the same rationale: - `docs/integrator/recipe-development.md` — switch the criteria-wildcard example from `gb200-any-training.yaml` to `gb200-any.yaml` (deployment-phase floor); add the "per-fabric values don't belong here" caveat. - `docs/contributor/data.md` — refresh the wildcard explanation section, the resolver-tracing example, and the Mermaid flowchart to use `gb200-any` throughout; rename `-any-` naming convention to allow `-any` (deployment-floor pattern). - `docs/design/005-overlay-refactoring.md` — drop the `b200-any-training` / `gb200-any-training` lines from the overlay tree and leave a brief historical note explaining the retirement across PRs NVIDIA#1004 (b200 wildcard) and NVIDIA#1052 (gb200 wildcard). - Update the `knownGaps` header comment in `pkg/recipe/validation_phase_floor_test.go` to document the OKE GB200 training performance data gap (warn-only in non-strict mode today; closes when NVIDIA#1007 lands an OCI testbed measurement). The map stays empty: the floor test treats missing performance as a `t.Log(WARN)` in default mode, so a knownGaps entry would be stale-flagged. The `b200-any-training.yaml` deletion is in flight via NVIDIA#1004 (PR NVIDIA#1053); the two issues are independent. Acceptance (per NVIDIA#1052): 1. `recipes/overlays/gb200-any-training.yaml` removed: yes. 2. `gb200-oke-training` perf coverage: option (b) — deferred to NVIDIA#1007 for real OCI measurements; covered as a warn-only floor gap today. 3. `go test ./pkg/recipe/... -run TestOverlayValidationPhaseFloor`: passes. 4. `aicr recipe --service oke --accelerator gb200 --intent training --format yaml`: resolves to 11 components, 6 overlays (base, monitoring-hpa, gb200-any, oke, oke-training, gb200-oke-training); carries the gpu-operator v25.10.0 floor and the standard 4 deployment checks via `gb200-any.yaml`. 5. Doc references updated. 6. `make qualify` clean. Fixes NVIDIA#1052 Related NVIDIA#1004, NVIDIA#1007
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 28, 2026
The criteria-wildcard overlay `gb200-any-training.yaml` carried a single `nccl-all-reduce-bw >= 720` constraint applied to every GB200 + training query regardless of service. The companion `b200-any-training.yaml` had the same shape. The pattern is misleading: each service has a different network fabric (EFA on EKS, TCPXO on GKE, RoCE on OKE) and a single cross-service threshold is rarely correct for two fabrics. NCCL bandwidth thresholds belong on the concrete service-bound leaf, anchored to a measurement on that specific fabric. The fabric-independent deployment-phase floor (4 standard health checks + `Deployment.gpu-operator.version` pin) remains on `gb200-any.yaml` — that pattern (established by NVIDIA#1001) is correct because the same gpu-operator version requirement applies across every service for the accelerator. Changes: - Delete `recipes/overlays/gb200-any-training.yaml`. - Update `recipes/overlays/gb200-any.yaml` comment block: drop the "Companion to gb200-any-training.yaml" intro, explain why the intent-scoped sibling was retired. - Doc updates with the same rationale: - `docs/integrator/recipe-development.md` — switch the criteria-wildcard example from `gb200-any-training.yaml` to `gb200-any.yaml` (deployment-phase floor); add the "per-fabric values don't belong here" caveat. - `docs/contributor/data.md` — refresh the wildcard explanation section, the resolver-tracing example, and the Mermaid flowchart to use `gb200-any` throughout; rename `-any-` naming convention to allow `-any` (deployment-floor pattern). - `docs/design/005-overlay-refactoring.md` — drop the `b200-any-training` / `gb200-any-training` lines from the overlay tree and leave a brief historical note explaining the retirement across PRs NVIDIA#1004 (b200 wildcard) and NVIDIA#1052 (gb200 wildcard). - Update the `knownGaps` header comment in `pkg/recipe/validation_phase_floor_test.go` to document the OKE GB200 training performance data gap (warn-only in non-strict mode today; closes when NVIDIA#1007 lands an OCI testbed measurement). The map stays empty: the floor test treats missing performance as a `t.Log(WARN)` in default mode, so a knownGaps entry would be stale-flagged. The `b200-any-training.yaml` deletion is in flight via NVIDIA#1004 (PR NVIDIA#1053); the two issues are independent. Acceptance (per NVIDIA#1052): 1. `recipes/overlays/gb200-any-training.yaml` removed: yes. 2. `gb200-oke-training` perf coverage: option (b) — deferred to NVIDIA#1007 for real OCI measurements; covered as a warn-only floor gap today. 3. `go test ./pkg/recipe/... -run TestOverlayValidationPhaseFloor`: passes. 4. `aicr recipe --service oke --accelerator gb200 --intent training --format yaml`: resolves to 11 components, 6 overlays (base, monitoring-hpa, gb200-any, oke, oke-training, gb200-oke-training); carries the gpu-operator v25.10.0 floor and the standard 4 deployment checks via `gb200-any.yaml`. 5. Doc references updated. 6. `make qualify` clean. Fixes NVIDIA#1052 Related NVIDIA#1004, NVIDIA#1007
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 28, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 28, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 28, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 28, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 28, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
15 tasks
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
May 28, 2026
Adds the first concrete service-bound overlays for the b200 accelerator on GKE COS: b200-any.yaml (deployment-phase floor wildcard, mirrors gb200-any.yaml from NVIDIA#1001) plus four leaves (training, inference, training-kubeflow, inference-dynamo). Layout mirrors h100-gke-cos-*.yaml (GKE is COS-only by AICR convention — no -ubuntu- variant). Retires the placeholder b200-any-training.yaml wildcard per the principle established in NVIDIA#1052; H100 / RTX Pro 6000 already follow this shape, so B200 reaches parity. Anchored on the production reference cluster nvcf-dgxc-k8s-gcp-azne1-prd7, which deploys no separate NCCL plugin installer (no gke-nccl-tcpxo, no GPUDirect-RDMA DaemonSet) — high- bandwidth multi-node NCCL is provided by GPU Operator with `gdrcopy` enabled, combined with GKE A4's native multi-NIC infrastructure. Both training and inference leaves set cdi.enabled + gdrcopy.enabled on the gpu-operator overrides to mirror that deployment model. B200-vs-GB200 deltas honored: - x86 host (vs GB200 Grace ARM) → real tuning-gke.yaml, not no-op - No NVreg_GrdmaPciTopoCheckOverride flag - Single-fabric NCCL (no MNNVL / NVL72) - gpu-operator floor >= v25.10.0 (Blackwell baseline) The gke-nccl-tcpxo component is intentionally NOT added: its DaemonSets pin to nvidia-h100-mega-80gb and target the TCPX transport on a3-megagpu-8g (H100), so they would not run on A4 nodes and would misrepresent the deployment model. nccl-all-reduce-bw threshold is a placeholder (>= 100); the H100 GKE TCPXO baseline (>= 250) is not the right anchor for A4's gdrcopy + native-multi-NIC model. Tighten once an empirical measurement from the reference cluster is captured. Fixes: NVIDIA#1004 Related: NVIDIA#1001, NVIDIA#1052, NVIDIA#969, NVIDIA#436
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Push the deployment-phase floor — both the 4 standard checks AND the `Deployment.gpu-operator.version` constraint — to per-accelerator wildcard overlays (`h100-any.yaml`, `gb200-any.yaml`, `rtx-pro-6000-any.yaml`). Every accelerator-bound query now inherits a constraint-backed deployment phase. Tighten `TestOverlayValidationPhaseFloor` to require the constraint, not just phase presence; exempt accelerator-unbound classifications (whose threshold can't be meaningful without an accelerator).
Closes the gap Codex flagged on v1: declaring `gpu-operator-version` at the service-root let accelerator-bound descendants like `gb200-oke-training` inherit the check name without a `Deployment.gpu-operator.version` constraint, so the check ran as a no-op.
Scope (explicit)
In scope: deployment-phase floor for accelerator-bound recipes — every recipe a user can actually deploy (specifies `--accelerator` or has an `accelerator:` criterion).
Explicitly out of scope: accelerator-unbound intermediates (e.g., `eks`, `eks-training`, `aks-inference`). These resolve to incomplete recipes — running `aicr recipe --service eks --intent training` without `--accelerator` produces a bundle with no accelerator-specific GPU operator config (no driver version pin, no chart values per accelerator). The deployment-phase `Deployment.gpu-operator.version` constraint is accelerator-specific (H100 ≥ v24.6.0, GB200 ≥ v25.10.0, RTX Pro 6000 ≥ v25.10.0); a generic version pin at the intent layer would either be over-permissive (lowest common floor) or wrong for some accelerators. The floor test exempts these classifications via `requiresDeployment()` checking `isAcceleratorBound()`. Issue #969 has been updated to reflect this narrowing.
Motivation / Context
Fixes #969 — every accelerator-bound recipe now ships with a working version-pin assertion, not just a check name.
Related:
Type of Change
Component(s) Affected
Implementation Notes
Design choice — wildcards, not service-roots: The resolver applies overlays in order `base → wildcards → base chain`. Per-phase replace means whichever layer declares `deployment:` last wins. v1 added the block at service-root, which ran later than per-accelerator wildcards and shadowed any wildcard-level constraint. v2 inverts: service-roots own the service-scoped phase (`conformance`); per-accelerator wildcards own the accelerator-scoped phase (`deployment` with version pin). Concrete leaves with their own `deployment:` block continue to win via per-phase replace; their constraint values match the wildcard floor so the override is a no-op today.
Why accelerator-unbound classifications are exempt: An intent-level intermediate without an `accelerator:` criterion (e.g., `eks-training`) is a degenerate query — the resulting recipe has no accelerator-specific GPU-operator config either, so a deployment-phase version pin would have no meaningful value at the intent layer. Concrete leaves carry the constraint via wildcard composition. Issue #969 narrowed to make this explicit.
Floor-test tightening:
Files Changed
Testing
```bash
Default mode — all 56 subtests pass, knownGaps stays empty
go test ./pkg/recipe/... -run TestOverlayValidationPhaseFloor -v
Classification cases (10 subtests, all pass)
go test ./pkg/recipe/... -run TestClassifyOverlay -v
Codex's verification command — now resolves with the constraint
go run ./cmd/aicr recipe --service oke --accelerator gb200 --intent training --format yaml
yields:
validation.deployment.constraints:
- name: Deployment.gpu-operator.version
value: '>= v25.10.0'
Spot-checks
go run ./cmd/aicr recipe --service lke --accelerator rtx-pro-6000 --intent training --format yaml
go run ./cmd/aicr recipe --service kind --accelerator h100 --intent inference --format yaml
Full gate
make qualify # PASS
```
Strict mode (`AICR_VALIDATION_FLOOR_STRICT=1`) status at this PR head: still fails on 5 known performance-phase gaps — none of them deployment-side, none of them introduced or made worse by this PR. They are tracked in dedicated follow-up issues:
PR #1009 closes the accelerator-unbound intermediate side of strict-mode perf. Once both #1001 and #1009 merge, strict-mode failures drop to the 5 above; remaining work is the dedicated follow-ups.
Risk Assessment
Rollout notes: No migration. Resolved recipes for previously-uncovered accelerator-bound queries (e.g., all `gb200-oke-`, `h100-aks--inference`, `h100-kind-`, `rtx-pro-6000-lke-`) now ship with a real version assertion. Operators running `aicr validate --phase deployment` against these recipes will see the constraint evaluated against the deployed gpu-operator version.
Checklist