Skip to content

feat(bundler): bundle-time GPU driver-ownership coherence check - #1819

Merged
mchmarny merged 2 commits into
NVIDIA:mainfrom
yuanchen8911:issue-1757-driver-coherence
Jul 23, 2026
Merged

mchmarny merged 2 commits into
NVIDIA:mainfrom
yuanchen8911:issue-1757-driver-coherence

Conversation

@yuanchen8911

@yuanchen8911 yuanchen8911 commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR supersedes #1756's interim AKS-only warn-text scoping with the fully provider/OS-aware remedy.

Adds the blocking bundle-generation validation CheckDriverOwnershipCoherence (registered on gpu-operator at severity: error). It evaluates the final effective values — recipe merge plus all --set/--set-json/--set-file overrides, resolved under canonical component names and registry aliases in the bundler's own application order — and blocks incoherent GPU driver-ownership profiles before a bundle is produced.

Motivation / Context

PR #1756 ships the AKS default flip to the Azure-managed GPU driver profile without a bundle-time guard; this is the release-blocking coherence check agreed in its review. It resurrects the reference implementation from #1756's history (c412d598) and fixes the three review findings the issue lists: honor the complete override tuple (not --set gpuoperator:driver.enabled=true alone), resolve overrides under canonical names and registry aliases, and gate legacy pre-flip recipes via a metadata-independent effective-values lockstep rule.

Fixes: #1757
Related: #1756

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Build/CI/tooling

Component(s) Affected

  • CLI (cmd/aicr, pkg/cli)
  • API server (cmd/aicrd, pkg/server)
  • Recipe engine / data (pkg/recipe)
  • Bundlers (pkg/bundler, pkg/component/*)
  • Collectors / snapshotter (pkg/collector, pkg/snapshotter)
  • Validator (pkg/validator)
  • Core libraries (pkg/errors, pkg/k8s)
  • Docs/examples (docs/, examples/)
  • Other: ____________

Implementation Notes

Rule 1 — driverless cluster (gated on recorded snapshot state). Snapshot-driven resolution (applyGPUDriverAutoOverride) now records the observed NVIDIA kernel driver state in metadata.gpuDriverState (preinstalled/absent; empty = unknown, gate disarmed — older recipes and GPU-less snapshots never trip it). When the snapshot observed no driver on the sampled GPU node, an effective config with driver.enabled=false (or toolkit.enabled explicitly false) is blocked: deploying it would leave GPU nodes driverless. The gate clears when the effective values have driver.enabled=true and toolkit.enabled not explicitly false; a partial flip is still caught because Rule 2 independently enforces the DRA driver-root leg. operator.runtimeClass is deliberately not validated — the toolkit derives its containerd handler name from it, so any consistent value works (the documented tuple sets nvidia by convention).

Rule 2 — DRA driver-root lockstep (metadata-independent). With driver.enabled=true, nvidia-dra-driver-gpu.nvidiaDriverRoot must equal gpu-operator hostPaths.driverInstallDir (default /run/nvidia/driver) or CDI spec generation fails (issue #1087's TestDriverRootLockstep invariant, enforced on effective values at bundle time). With driver.enabled=false, the root must not be the operator container root /run/nvidia/driver — nothing populates it in that mode. This is the signature of a legacy pre-#1756 recipe (stale valuesFile resolution + old baked DRA override) and catches those recipes even though they carry no recorded state.

Override resolution. componentOverrideKeys/mergeOverridesAcrossKeys are reimplemented locally in pkg/bundler/validations (importing pkg/bundler would be a cycle), mirroring the bundler exactly: exact name first, then registry valueOverrideKeys aliases; scalar --set applied before typed --set-json/--set-file; the enabled toggle stripped. A recipe-side resolution failure fails closed on both paths: the gate blocks it, and extractComponentValues now also returns a blocking ErrCodeInternal (rather than warning and rendering the component from an empty map), so neither can emit a bundle whose driver ownership could not be verified. Override-apply failures are blocking too: ApplyMapOverrides applies scalar --set paths in Go's randomized map-iteration order, so an overlapping pair (e.g. gpuoperator:a.b=1 alongside gpuoperator:a=2) can apply cleanly child-first during extraction yet fail parent-first when the gate reapplies them — a silent skip there would disarm the gate for exactly the override sets whose effective values it cannot reconstruct.

Provider-aware warning. The resolution-time mismatch warning now derives its remedy from criteria.service and criteria.os: AKS → recreate pools without --gpu-driver none or the four-flag override tuple; GKE+cos → COS-only wording (the operator cannot install the driver on COS; use the GKE-managed driver install gpu-driver-version); GKE+ubuntu → the GPU-Operator-managed override set extended with --set gpuoperator:hostPaths.driverInstallDir=/run/nvidia/driver (the only GKE node image where the pinned operator v26.3.2 supports driver management; GKE Google-installer profiles pin both driver roots to /home/kubernetes/bin/nvidia, so the remedy must move both roots or the DRA lockstep rule would block the bundle it recommends); any other GKE OS (unknown, any, or one GKE does not offer) → the combined wording, so an off-catalog service: gke recipe never gets an unsupported operator-managed recommendation; other → generic reprovision wording plus the tuple. The remedy helper is deliberately duplicated between pkg/client/v1 and pkg/bundler/validations (documented in both; a shared package for two small helpers was rejected).

Type promotion. RecipeResult's anonymous metadata struct is promoted to the named recipe.RecipeResultMetadata to carry the new field; DeepCopy copies it, and the OpenAPI RecipeResponse schema documents gpuDriverState.

Two synthetic bundler test fixtures (bundler_dra_annotation_parity_test.go) gained an explicit nvidiaDriverRoot override — they previously modeled an incoherent recipe (operator-managed driver, no DRA root) that the new check correctly blocks.

Cross-review fix set (applied after the multi-reviewer pass): fail closed with a blocking error when a component's effective values cannot be resolved (see Override resolution above); resolve --set enabled toggles with the bundler's exact semantics (canonical-name-wins merge across registry aliases, strconv.ParseBool so enabled=0 counts); path.Clean the driver-root comparisons so trailing-slash spellings compare equal in both Rule 2 directions; reject an explicit hostPaths.driverInstallDir of / regardless of DRA presence or equality (the #1106 regression — runc refuses a bind-mount destination of /); project metadata.gpuDriverState in the hydrated query output so aicr query/SelectFromRecipe matches the recipe YAML and OpenAPI schema; scope the GKE remedy wording to COS node images; fail closed when override reapplication fails inside the gate (see Override resolution above); reject a recorded metadata.gpuDriverState outside the two documented constants — nothing validates the field at the load/adopt boundaries, so a typo'd spelling (Absent) in a loaded or hand-edited recipe would otherwise silently degrade to the deliberate empty=unknown disarm state and clear Rule 1. A second cross-review round added: OS-aware wording for Rule 2's legacy-recipe alternative clause (GKE+COS must not be told to use the operator-managed tuple the operator cannot deploy there — it gets a DRA-root retarget at the GKE-managed install path instead); resolver error codes preserved through the fail-closed wrap (an ErrCodeInternal/ErrCodeTimeout resolution failure is no longer reclassified as ErrCodeInvalidRequest, so SDK/server consumers keep retryability and HTTP-status fidelity); and a surfaced bundler warning when gpu-operator is excluded from the bundle (--set gpuoperator:enabled=false, recipe-level disable, or the bundlers filter) while the recorded driver state is absent — exclusion is deliberate ("satisfied externally") and subset bundles are first-class, so it warns rather than blocks, but nothing in such a bundle installs a driver and a silent skip would hide that.

Guessed-value remedies are suppressed. When the declared driverInstallDir is rejected, when the DRA root is invalid, or when either value is dynamic (bundle-time-deferred), the lockstep switch is skipped — the rejection already blocks the bundle, and a second remedy computed from a guessed default or a stale static value would mislead. The legacy-recipe message says "commonly the signature of" (user-created states can produce the same values) and its override tuple is GKE-aware.

Invalid and relative driver-root declarations fail closed. Present-but-invalid declarations are rejected instead of treated as absent: a null/empty/non-string DRA nvidiaDriverRoot, and a null/non-map hostPaths or non-string driverInstallDir (Helm null-coalescing deletes chart defaults; the ClusterPolicy CRD types the field as string). Declared roots that clean to a relative path are rejected too (path.IsAbs after path.Clean): host-path mounts require absolute paths, and a relative spelling of the operator root (run/nvidia/driver) compares unequal to /run/nvidia/driver, so with driver.enabled=false no Rule 2 branch would fire and the broken mount would bundle.

Testing

unset GITLAB_TOKEN; export GOFLAGS=-mod=vendor
gofmt -l pkg/ api/            # clean
go build ./...                # ok
go test -race ./pkg/recipe/... ./pkg/client/... ./pkg/bundler/... ./pkg/validator/... ./pkg/server/...   # all ok
golangci-lint run -c .golangci.yaml ./pkg/recipe/... ./pkg/client/... ./pkg/bundler/... ./pkg/validator/...  # 0 issues
yamllint recipes/registry.yaml api/aicr/v1/server.yaml   # clean
go test -coverprofile=cover.out ./pkg/bundler/validations/...   # CheckDriverOwnershipCoherence 98.2%, package 88.7%

New coverage: a 32-case table for CheckDriverOwnershipCoherence (including the partial-flip regression test for review finding 1, canonical-name + alias + --set-json override resolution, the legacy-recipe signature, unresolvable-values fail-closed errors, canonical-beats-alias and enabled=0 toggle semantics, trailing-slash roots in both directions, and driverInstallDir=/ with and without a DRA ref), a hydrated-query projection test for metadata.gpuDriverState (recorded vs omitted), Metadata.GPUDriverState recording assertions for preinstalled/absent/unknown/not-observed, and DeepCopy coverage for the new field. Invalid/relative-root cases: nvidiaDriverRoot null/empty/bool, hostPaths null/non-map, driverInstallDir bool/null, relative DRA root with driver off (the Rule 2 bypass), relative driverInstallDir, a matching relative pair (both rejected), and a ..-spelling legacy-root regression; fail-closed override-reapplication errors on all three legs (scalar --set, typed --set-json, and the DRA component); unrecognized recorded-state rejection (typo'd Absent blocked, reported alongside a Rule 2 finding without masking it, and still firing when values resolution itself fails); remedy-wording branches for GKE+COS (COS-only), GKE+ubuntu (operator-managed), GKE with unknown OS (combined), and GKE+rhel (no such node image → combined, no unsupported recommendation); Rule 2 legacy-remedy OS branches (GKE+COS → no operator-managed tuple, GKE+ubuntu → five-flag tuple, unknown GKE OS → both paths); a resolver-code preservation test (TestEffectiveComponentValues_PreservesResolverCode); a 7-case excluded-driver-installer warning table (TestFilterEnabledComponents_ExcludedDriverInstallerWarning); GKE Google-installer-profile lockstep regressions (remedy carries the fifth hostPaths.driverInstallDir flag; the five-flag tuple applied to that profile passes both rules).

This branch is rebased onto current main; make qualify runs on the rebased result (#1756 has merged). go build, go test, and golangci-lint on the affected packages are all clean.

Known follow-ups (non-blocking): gpu-operator-ocp is registered outside the gate (OCP ships DRA disabled; no supported config trips it today) — coverage note for a follow-up. Per-OS install capability (e.g. GKE-COS with a deliberate driver.enabled=true) is out of the coherence check's scope by design; documented in component-catalog.md with gpu-operator-health as the deploy-time backstop.

Scope & follow-up work

This PR is deliberately scoped to the bundle-time coherence gate for #1757. A multi-reviewer cross-review produced a broader set of hardening changes; the small, gate-adjacent ones are kept here and the larger, orthogonal ones are split out to keep the PR focused and reviewable. Kept in this PR:

  • Fail-closed value reads — extractComponentValues returns a blocking error instead of warning and rendering from an empty map.
  • Structured error-code preservation — component-validation failures preserve an already-coded error (ErrCodeTimeout/ErrCodeInternal) at the Make call site instead of flattening every failure to ErrCodeInvalidRequest (which returned a non-retryable HTTP 400 and could expose the internal cause on a 4xx).

The gate runs on both bundle paths: DefaultBundler.Make (the CLI and the server's MakeBundle) and the public Client.BundleComponents SDK path. That SDK preflight is baseline #1757 coverage — an external Go caller of BundleComponents would otherwise produce component bundles the CLI/server reject — so it is intentionally retained; only the exact-value reuse (follow-up A) is deferred.

Deferred to their own follow-up PRs (each independently reasonable, none required for #1757). Tracked in #1873 (A, B — gate-integrity hardening) and #1874 (C, D — recipe/SDK cleanups):

  • A — Values TOCTOU closure (exact-value reuse). The coherence gate already runs on both bundle paths — DefaultBundler.Make (CLI and the server's MakeBundle) and the public Client.BundleComponents SDK path — but each currently self-resolves component values independently of the read used to emit the bundle. Follow-up: resolve values once and validate the exact maps that are emitted on both paths, so a mutable provider cannot change values between check and emit. Reachable via --data/LayeredDataProvider (external files are re-read per call). Acceptance criterion: validation examines exactly the values emitted, proven by a FilesystemSource mutation test.
  • B — Argo CD Helm install-time driver-ownership guards. Reject install-time overrides of the guarded ownership leaves in the generated argocd-helm chart so a --set at install cannot bypass the bundle-time verdict.
  • C — Generic duplicate ComponentRef.Name validation in PrepareAndValidate.
  • D — SDK disabled-component filtering in facadeResultFromInternal (omit disabled components from the facade's deployable set).

Two carry-over open questions from the cross-review, applicable to the gate as written (not introduced here):

  • The gate's value-reconstruction helpers are duplicated in pkg/bundler/validations and pkg/bundler (import-cycle-driven), kept in sync by comment only — a golden test or shared package would prevent silent drift.
  • Rule 2 hard-blocks any effective config with driver.enabled=false and a DRA nvidiaDriverRoot that cleans to /run/nvidia/driver. OSS overlays (OKE, GKE-COS, AKS) are confirmed coherent; internal overlays should be confirmed not to ship that combination.

Risk Assessment

  • Low — Isolated change, well-tested, easy to revert
  • Medium — Touches multiple components or has broader impact
  • High — Breaking change, affects critical paths, or complex rollout

Rollout notes: The check is fail-closed on two previously-undetected misconfigurations. Recipes generated by this AICR version are coherent by construction; legacy pre-#1756 AKS recipes are blocked with an actionable message (regenerate, or supply the four-flag override set). No migration steps beyond those documented in docs/integrator/aks-gpu-setup.md.

Checklist

  • Tests pass locally (make test with -race)
  • Linter passes (make lint)
  • I did not skip/disable tests to make CI green
  • I added/updated tests for new functionality
  • I updated docs if user-facing behavior changed
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S) — GPG signing info
@github-actions

Copy link
Copy Markdown
Contributor
@github-actions

github-actions Bot commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Recipe evidence check

Registry change: scoped to recipes that reference a changed component
entry in recipes/registry.yaml (not every leaf).

Protected recipes

Recipes with committed evidence (recipes/evidence/<slug>/<source>/<digest>.yaml) that this PR affects: 5

Recipe Source Pointer Verify Digest match
gb200-eks-ubuntu-training 7c4c0edc8c765a95a0f3afdb3bbb8e91 sha256-93fac974407a873d5b6a52a72bafcaa18b019190545a23d03031680d6aabd2bc ❌ invalid — registry-forbidden (HTTP 401): registry not accessible (make the fork's aicr-evidence package public, or provide registry credentials) ⚠️ skipped (no signed digest)
h100-aks-ubuntu-inference-dynamo 5bf9e82f0e90a11528ac85f4bcb866c8 sha256-b7d3b1c672568329cae994ed4c831af5e569b23209fb81e789d2e2288b44100d ✅ passed ⚠️ stale (b0081437bf6d… vs current 5c4b086ea917…)
h100-aks-ubuntu-inference-dynamo 5bf9e82f0e90a11528ac85f4bcb866c8 sha256-edc042d2e32d58bde9bb0e7cfdaa14568a13c144fdf0869958a4d582f3fc8cfc ✅ passed ⚠️ stale (ea8757f630ce… vs current 5c4b086ea917…)
h100-aks-ubuntu-inference-dynamo 5bf9e82f0e90a11528ac85f4bcb866c8 sha256-f8d2a0188274d179f37dfe39a257aeaa3fbb97273162586853e0986bfa5d3c05 ✅ passed ⚠️ stale (8e88ca57dea5… vs current 5c4b086ea917…)
h100-aks-ubuntu-training-kubeflow 5bf9e82f0e90a11528ac85f4bcb866c8 sha256-7bfed65fb09c14c6e6cbe87a68e0810a7d24178e0e83d1691c020556c92dbbd8 ✅ passed ⚠️ stale (7726976735b7… vs current a39626d57004…)
h100-aks-ubuntu-training-kubeflow 5bf9e82f0e90a11528ac85f4bcb866c8 sha256-7e7c4680bab4c44bb68fab53fc85a7f8d8065ca6b796458a2bc7cb4f4a49bfa9 ✅ passed ⚠️ stale (748b0a7f5852… vs current a39626d57004…)
h100-aks-ubuntu-training-kubeflow 5bf9e82f0e90a11528ac85f4bcb866c8 sha256-dc1670c23bbe6711a6ffd86a49160b06d992c8ff84e8f3303facc54dd7aecb61 ✅ passed ⚠️ stale (fac7033fea5c… vs current a39626d57004…)
h100-gke-cos-training 7c4c0edc8c765a95a0f3afdb3bbb8e91 sha256-be4680f26ad9ebeb57145f1953f18311ca00e81a4edb37773e0ec1060c6bd261 ❌ invalid — registry-forbidden (HTTP 401): registry not accessible (make the fork's aicr-evidence package public, or provide registry credentials) ⚠️ skipped (no signed digest)
h100-gke-cos-training 7c4c0edc8c765a95a0f3afdb3bbb8e91 sha256-f2573e7f2496cc895e6a780604645f7c24ed4d7e0edf4c4845c0d341a3a6326e ❌ invalid — registry-forbidden (HTTP 401): registry not accessible (make the fork's aicr-evidence package public, or provide registry credentials) ⚠️ skipped (no signed digest)
rtx-pro-6000-eks-ubuntu-inference-dynamo 5bf9e82f0e90a11528ac85f4bcb866c8 sha256-3ec33498d3df68b688ae96280634c1a4403b7502a49016be54aecc70b0d2549e ✅ passed ⚠️ stale (348eada47742… vs current 6e8070a9ff3e…)
Other affected recipes without evidence yet: 62

These recipes are affected by this PR but carry no committed evidence pointer, so there is
nothing to verify. This is expected — evidence is hardware-gated and added over time.

  • a100-aks-training
  • a100-aks-ubuntu-training-kubeflow
  • a100-aks-ubuntu-training
  • a100-eks-training
  • a100-eks-ubuntu-training-kubeflow
  • a100-eks-ubuntu-training
  • a100-gke-cos-training-kubeflow
  • a100-gke-cos-training
  • a100-oke-training
  • a100-oke-ubuntu-training-kubeflow
  • a100-oke-ubuntu-training
  • b200-gke-cos-inference-dynamo
  • b200-gke-cos-inference
  • b200-gke-cos-training-kubeflow
  • b200-gke-cos-training
  • gb200-eks-inference
  • gb200-eks-training
  • gb200-eks-ubuntu-inference-dynamo
  • gb200-eks-ubuntu-inference
  • gb200-eks-ubuntu-training-kubeflow
  • gb200-eks-ubuntu-training-slurm
  • gb200-oke-inference
  • gb200-oke-training
  • gb200-oke-ubuntu-inference-dynamo
  • gb200-oke-ubuntu-inference
  • gb200-oke-ubuntu-training-kubeflow
  • gb200-oke-ubuntu-training
  • h100-aks-inference
  • h100-aks-training
  • h100-aks-ubuntu-inference
  • h100-aks-ubuntu-training-slurm
  • h100-aks-ubuntu-training
  • h100-bcm-training
  • h100-bcm-ubuntu-training
  • h100-eks-inference
  • h100-eks-training
  • h100-eks-ubuntu-inference-dynamo
  • h100-eks-ubuntu-inference-nim
  • h100-eks-ubuntu-inference
  • h100-eks-ubuntu-training-kubeflow
  • h100-eks-ubuntu-training-slurm
  • h100-eks-ubuntu-training
  • h100-gke-cos-inference-dynamo
  • h100-gke-cos-inference
  • h100-gke-cos-training-kubeflow
  • h100-gke-cos-training-slurm
  • h100-kind-inference-dynamo
  • h100-kind-inference
  • h100-kind-training-kubeflow
  • h100-kind-training-slurm
  • h100-kind-training
  • h200-eks-inference
  • h200-eks-training
  • l40s-oke-inference
  • l40s-oke-training
  • rtx-pro-6000-eks-inference
  • rtx-pro-6000-eks-ubuntu-inference-nim
  • rtx-pro-6000-eks-ubuntu-inference
  • rtx-pro-6000-lke-inference
  • rtx-pro-6000-lke-training
  • rtx-pro-6000-lke-ubuntu-inference
  • rtx-pro-6000-lke-ubuntu-training

How to refresh evidence

Run on a cluster matching the recipe's criteria:

aicr snapshot -o snapshot.yaml
aicr validate \
  -r recipes/overlays/<slug>.yaml \
  -s snapshot.yaml \
  --emit-attestation ./out \
  --push ghcr.io/<your-fork>/aicr-evidence
# Copy to the per-source path printed in the emit 'copyTo' hint:
#   recipes/evidence/<slug>/<source>/<bundle-digest>.yaml

This gate is warning-only and never blocks merge. See ADR-007 for the trust model.

@yuanchen8911 yuanchen8911 changed the title feat(bundler): bundle-time GPU driver-ownership coherence validation Jul 20, 2026
@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change records snapshot-observed NVIDIA driver state in typed recipe metadata and exposes it through the recipe response schema. Snapshot resolution handles preinstalled and absent driver states with provider-specific warnings and conditional overrides. Bundle generation registers CheckDriverOwnershipCoherence, which evaluates effective GPU Operator and DRA values for ownership and driver-root consistency. AKS recipes, values, examples, tests, and documentation reflect the driver-only profile and migration behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested labels: theme/recipes

Suggested reviewers: mchmarny

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: a bundle-time GPU driver-ownership coherence check.
Description check ✅ Passed The description is directly related and accurately covers the validation, metadata, docs, and test updates in the PR.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/client/v1/gpu_driver_state.go`:
- Around line 59-74: The driverAbsentRemedy logic must distinguish GKE-COS from
other GKE profiles instead of branching on CriteriaServiceGKE alone. Pass the
resolved OS or full criteria into driverAbsentRemedy, return the COS-specific
wording only for GKE with COS, and use the generic GPU-Operator-managed remedy
for other GKE profiles; apply the same condition to the duplicated bundler
helper.
🪄 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: c7e1ca99-6ff1-4e0e-a26e-b6a1dd73ecd2

📥 Commits

Reviewing files that changed from the base of the PR and between 544491c and 281f0bc.

📒 Files selected for processing (24)
  • api/aicr/v1/server.yaml
  • docs/contributor/validator.md
  • docs/integrator/aks-gpu-setup.md
  • docs/integrator/recipe-development.md
  • docs/user/cli-reference.md
  • docs/user/component-catalog.md
  • examples/recipes/aks-training.yaml
  • pkg/bundler/bundler_dra_annotation_parity_test.go
  • pkg/bundler/deployer/helm/helm_test.go
  • pkg/bundler/validations/checks.go
  • pkg/bundler/validations/checks_test.go
  • pkg/client/v1/aicr.go
  • pkg/client/v1/aicr_test.go
  • pkg/client/v1/gpu_driver_state.go
  • pkg/client/v1/gpu_driver_state_test.go
  • pkg/recipe/builder_test.go
  • pkg/recipe/driver_root_lockstep_test.go
  • pkg/recipe/metadata.go
  • pkg/validator/v1/conversion_test.go
  • recipes/components/gpu-operator/values-aks-training.yaml
  • recipes/components/gpu-operator/values-aks.yaml
  • recipes/overlays/aks.yaml
  • recipes/registry.yaml
  • tests/uat/azure/cluster-config.yaml
Comment thread pkg/client/v1/gpu_driver_state.go Outdated
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from 281f0bc to cfccc06 Compare July 20, 2026 19:38

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@docs/contributor/validator.md`:
- Line 671: Update the CheckDriverOwnershipCoherence description in the
validator documentation to explicitly include --set-file alongside --set and
--set-json as supported override sources, while preserving the existing
validation behavior and wording.

In `@docs/integrator/recipe-development.md`:
- Line 374: Clarify the snapshot-driven override paragraph to state that
explicit CLI --set flags retain higher precedence only during bundle generation,
not when running aicr recipe or ResolveRecipeFromSnapshot. Avoid implying that
--set is supported by the recipe-resolution commands, while preserving the
existing override precedence behavior.
🪄 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: 94b1e057-5098-4071-a8df-eaebfcaa27eb

📥 Commits

Reviewing files that changed from the base of the PR and between 281f0bc and cfccc06.

📒 Files selected for processing (29)
  • api/aicr/v1/server.yaml
  • docs/contributor/validator.md
  • docs/integrator/aks-gpu-setup.md
  • docs/integrator/recipe-development.md
  • docs/user/cli-reference.md
  • docs/user/component-catalog.md
  • examples/recipes/aks-training.yaml
  • pkg/bundler/bundler_dra_annotation_parity_test.go
  • pkg/bundler/deployer/helm/helm_test.go
  • pkg/bundler/validations/checks.go
  • pkg/bundler/validations/checks_test.go
  • pkg/client/v1/aicr.go
  • pkg/client/v1/aicr_test.go
  • pkg/client/v1/gpu_driver_state.go
  • pkg/client/v1/gpu_driver_state_test.go
  • pkg/recipe/builder_test.go
  • pkg/recipe/driver_root_lockstep_test.go
  • pkg/recipe/metadata.go
  • pkg/recipe/nodewright_tuning_gate_test.go
  • pkg/validator/v1/conversion_test.go
  • recipes/components/gpu-operator/values-aks-training.yaml
  • recipes/components/gpu-operator/values-aks.yaml
  • recipes/components/nodewright-customizations/manifests/tuning.yaml
  • recipes/overlays/a100-aks-training.yaml
  • recipes/overlays/aks.yaml
  • recipes/overlays/h100-aks-inference.yaml
  • recipes/overlays/h100-aks-training.yaml
  • recipes/registry.yaml
  • tests/uat/azure/cluster-config.yaml
Comment thread docs/contributor/validator.md Outdated
Comment thread docs/integrator/recipe-development.md Outdated
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from cfccc06 to 3d9f22a Compare July 20, 2026 20:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/user/component-catalog.md`:
- Line 79: Align the preinstalled-profile guarantee with
hasPreinstalledDriverProfile in pkg/client/v1/gpu_driver_state.go: require the
complete coordinated profile, including toolkit, gdrcopy, and driver-root
settings, rather than only driver.enabled=false; alternatively, narrow the
documentation claim to match the existing marker check. Ensure incomplete
overlays cannot receive preinstalled-driver behavior as a valid profile.
🪄 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: 6e17f1b9-85ff-478d-a3c0-99e6bc6b2d12

📥 Commits

Reviewing files that changed from the base of the PR and between cfccc06 and 3d9f22a.

📒 Files selected for processing (18)
  • api/aicr/v1/server.yaml
  • docs/contributor/validator.md
  • docs/integrator/aks-gpu-setup.md
  • docs/user/component-catalog.md
  • pkg/bundler/bundler_dra_annotation_parity_test.go
  • pkg/bundler/deployer/helm/helm_test.go
  • pkg/bundler/validations/checks.go
  • pkg/bundler/validations/checks_test.go
  • pkg/client/v1/aicr.go
  • pkg/client/v1/aicr_test.go
  • pkg/client/v1/gpu_driver_state.go
  • pkg/client/v1/gpu_driver_state_test.go
  • pkg/recipe/builder_test.go
  • pkg/recipe/metadata.go
  • pkg/recipe/query.go
  • pkg/recipe/query_test.go
  • pkg/validator/v1/conversion_test.go
  • recipes/registry.yaml
Comment thread docs/user/component-catalog.md
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from 3d9f22a to d00ee61 Compare July 20, 2026 21:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/client/v1/gpu_driver_state.go`:
- Around line 404-417: Clear r.Metadata.GPUDriverState before the switch that
classifies state in the GPU driver observation flow, so gpuDriverUnknown and
gpuDriverNotObserved reset any prior value while preinstalled and absent states
still assign their corresponding values.
🪄 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: 0dd8a5c7-1718-4fae-a59d-1df417dba626

📥 Commits

Reviewing files that changed from the base of the PR and between 3d9f22a and d00ee61.

📒 Files selected for processing (34)
  • api/aicr/v1/server.yaml
  • docs/contributor/validator.md
  • docs/integrator/aks-gpu-setup.md
  • docs/integrator/recipe-development.md
  • docs/user/cli-reference.md
  • docs/user/component-catalog.md
  • examples/recipes/aks-training.yaml
  • pkg/bundler/bundler.go
  • pkg/bundler/bundler_dra_annotation_parity_test.go
  • pkg/bundler/deployer/helm/helm_test.go
  • pkg/bundler/validations/checks.go
  • pkg/bundler/validations/checks_test.go
  • pkg/bundler/validations/registry.go
  • pkg/bundler/validations/registry_test.go
  • pkg/client/v1/aicr.go
  • pkg/client/v1/aicr_test.go
  • pkg/client/v1/gpu_driver_state.go
  • pkg/client/v1/gpu_driver_state_test.go
  • pkg/recipe/builder_test.go
  • pkg/recipe/driver_root_lockstep_test.go
  • pkg/recipe/metadata.go
  • pkg/recipe/nodewright_tuning_gate_test.go
  • pkg/recipe/query.go
  • pkg/recipe/query_test.go
  • pkg/validator/v1/conversion_test.go
  • recipes/components/gpu-operator/values-aks-training.yaml
  • recipes/components/gpu-operator/values-aks.yaml
  • recipes/components/nodewright-customizations/manifests/tuning.yaml
  • recipes/overlays/a100-aks-training.yaml
  • recipes/overlays/aks.yaml
  • recipes/overlays/h100-aks-inference.yaml
  • recipes/overlays/h100-aks-training.yaml
  • recipes/registry.yaml
  • tests/uat/azure/cluster-config.yaml
Comment thread pkg/client/v1/gpu_driver_state.go
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from d00ee61 to fe630f4 Compare July 20, 2026 21:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/driver_root_lockstep_test.go`:
- Around line 160-169: The root-path normalization currently occurs after the
earlier root-rejection guard, allowing equivalent spellings such as /./ or // to
bypass it. Normalize driverInstallDir before that guard, then reuse the
normalized value in the subsequent draRoot and opInstallDir comparisons while
preserving the empty-string unset behavior.
🪄 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: af1e2df3-23cc-4e3e-880b-15847acd23c7

📥 Commits

Reviewing files that changed from the base of the PR and between d00ee61 and fe630f4.

📒 Files selected for processing (34)
  • api/aicr/v1/server.yaml
  • docs/contributor/validator.md
  • docs/integrator/aks-gpu-setup.md
  • docs/integrator/recipe-development.md
  • docs/user/cli-reference.md
  • docs/user/component-catalog.md
  • examples/recipes/aks-training.yaml
  • pkg/bundler/bundler.go
  • pkg/bundler/bundler_dra_annotation_parity_test.go
  • pkg/bundler/deployer/helm/helm_test.go
  • pkg/bundler/validations/checks.go
  • pkg/bundler/validations/checks_test.go
  • pkg/bundler/validations/registry.go
  • pkg/bundler/validations/registry_test.go
  • pkg/client/v1/aicr.go
  • pkg/client/v1/aicr_test.go
  • pkg/client/v1/gpu_driver_state.go
  • pkg/client/v1/gpu_driver_state_test.go
  • pkg/recipe/builder_test.go
  • pkg/recipe/driver_root_lockstep_test.go
  • pkg/recipe/metadata.go
  • pkg/recipe/nodewright_tuning_gate_test.go
  • pkg/recipe/query.go
  • pkg/recipe/query_test.go
  • pkg/validator/v1/conversion_test.go
  • recipes/components/gpu-operator/values-aks-training.yaml
  • recipes/components/gpu-operator/values-aks.yaml
  • recipes/components/nodewright-customizations/manifests/tuning.yaml
  • recipes/overlays/a100-aks-training.yaml
  • recipes/overlays/aks.yaml
  • recipes/overlays/h100-aks-inference.yaml
  • recipes/overlays/h100-aks-training.yaml
  • recipes/registry.yaml
  • tests/uat/azure/cluster-config.yaml
Comment thread pkg/recipe/driver_root_lockstep_test.go
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from fe630f4 to 5db05f6 Compare July 20, 2026 21:52
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch 2 times, most recently from 0a3a813 to e2cc501 Compare July 20, 2026 22:46
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch 4 times, most recently from f3d065f to 6e7c5d8 Compare July 20, 2026 23:55
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from fe6508d to eee7076 Compare July 22, 2026 02:52
@github-actions

Copy link
Copy Markdown
Contributor

@yuanchen8911 this PR now has merge conflicts with main. Please rebase to resolve them.

@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from eee7076 to cce10a6 Compare July 22, 2026 13:59
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch 5 times, most recently from d3998af to db644c3 Compare July 22, 2026 23:34
@yuanchen8911 yuanchen8911 changed the title WIP: feat(bundler): bundle-time GPU driver-ownership coherence check Jul 23, 2026
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from db644c3 to 6c31f06 Compare July 23, 2026 00:39
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch 3 times, most recently from 2ed82f3 to f7826f2 Compare July 23, 2026 02:00
@yuanchen8911
yuanchen8911 marked this pull request as ready for review July 23, 2026 02:03
@yuanchen8911
yuanchen8911 requested review from a team as code owners July 23, 2026 02:03
@yuanchen8911
yuanchen8911 requested a review from mchmarny July 23, 2026 02:34
Adds the blocking bundle-generation validation CheckDriverOwnershipCoherence
(registered on gpu-operator at severity: error). It evaluates the final
effective values — recipe merge plus all --set/--set-json/--set-file overrides,
resolved under canonical component names and registry aliases in the bundler's
own application order — and blocks incoherent GPU driver-ownership profiles
before a bundle is produced.

Rule 1 (driverless cluster, gated on recorded snapshot state): when the
snapshot observed no NVIDIA kernel driver (metadata.gpuDriverState=absent), an
effective config with driver.enabled=false (or toolkit.enabled explicitly
false) is blocked — deploying it would leave GPU nodes driverless. Empty state
disarms the gate, so older recipes and GPU-less snapshots never trip it.

Rule 2 (DRA driver-root lockstep, metadata-independent): with
driver.enabled=true, nvidia-dra-driver-gpu.nvidiaDriverRoot must equal
gpu-operator hostPaths.driverInstallDir or CDI spec generation fails. With a
preinstalled driver, the DRA root must avoid the unpopulated operator container
root and may intentionally differ from hostPaths.driverInstallDir. This also
catches legacy pre-NVIDIA#1756 recipes that carry no recorded state.

Two adjacent hardening fixes on the bundler value path the gate depends on:
extractComponentValues now fails closed on a value-resolution error instead of
logging a warning and rendering the component from an empty map; and component
validation failures preserve an already-coded error (ErrCodeTimeout /
ErrCodeInternal) instead of flattening every failure to ErrCodeInvalidRequest.

Fixes: NVIDIA#1757
Related: NVIDIA#1756

Signed-off-by: Yuan Chen <yuanchen97@gmail.com>
@yuanchen8911
yuanchen8911 force-pushed the issue-1757-driver-coherence branch from f7826f2 to 77b66ee Compare July 23, 2026 02:34
@mchmarny
mchmarny merged commit 26afcca into NVIDIA:main Jul 23, 2026
11 of 14 checks passed
@yuanchen8911
yuanchen8911 deleted the issue-1757-driver-coherence branch August 19, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

2 participants