diff --git a/docs/user/component-catalog.md b/docs/user/component-catalog.md index 78d355404..588afbbe8 100644 --- a/docs/user/component-catalog.md +++ b/docs/user/component-catalog.md @@ -53,7 +53,7 @@ The source of truth is [`recipes/registry.yaml`](https://github.com/NVIDIA/aicr/ | **cert-manager-ocp-olm** | OLM installer for cert-manager on OpenShift. Creates the OperatorGroup and Subscription resources that install the certified cert-manager Operator via the Operator Lifecycle Manager. Paired with `cert-manager-ocp`. OCP-specific. | [cert-manager (Certified)](https://catalog.redhat.com/software/container-stacks/detail/5ec3f5a5eebc3d6acb0ee71c) | | **cert-manager-ocp** | cert-manager CertManager CR for OpenShift. The operand Deployments (controller, cainjector, webhook) land in a hardcoded `cert-manager` namespace regardless of the operator's own namespace. Deployed after `cert-manager-ocp-olm`. OCP-specific. | [cert-manager](https://github.com/cert-manager/cert-manager) | | **prometheus-adapter-ocp** | Prometheus Adapter for OpenShift. Reuses the same upstream chart as `prometheus-adapter`, pointed at OCP's built-in Thanos Querier instead of kube-prometheus-stack (which stays disabled on OCP). No certified OCP operator exists for this component. OCP-specific. | [prometheus-adapter](https://github.com/kubernetes-sigs/prometheus-adapter) | -| **nvidia-dra-driver-gpu-ocp** | NVIDIA DRA GPU driver for OpenShift. Reuses the same upstream chart as `nvidia-dra-driver-gpu`, with an added SCC RoleBinding granting the kubelet-plugin DaemonSet the host device access OCP's default restricted-v2 SCC forbids. No certified OCP operator exists for this component. OCP-specific. Known limitation: some GPU-driver rollout protections and remedy hints do not yet cover the OCP aliases (`gpu-operator-ocp`, `nvidia-dra-driver-gpu-ocp`) — the deployer's stale-NVML migration wait/restart, driver-version annotation injection, and the driver-absent remedy's `gpuoperator:`/`dradriver:` override keys; tracked in [#2136](https://github.com/NVIDIA/aicr/issues/2136). | [NVIDIA DRA Driver](https://github.com/kubernetes-sigs/dra-driver-nvidia-gpu) | +| **nvidia-dra-driver-gpu-ocp** | NVIDIA DRA GPU driver for OpenShift. Reuses the same upstream chart as `nvidia-dra-driver-gpu`, with an added SCC RoleBinding granting the kubelet-plugin DaemonSet the host device access OCP's default restricted-v2 SCC forbids. No certified OCP operator exists for this component. OCP-specific. Known limitation: the driver-version annotation injected onto the DRA pod templates falls back to the `gpu-operator-ocp-olm` Subscription channel, which changes on a channel re-pin but not on every in-channel OLM auto-upgrade — so the stale-NVML rollout gate (#973) can still miss an in-channel driver bump on OCP; tracked in [#2135](https://github.com/NVIDIA/aicr/issues/2135). | [NVIDIA DRA Driver](https://github.com/kubernetes-sigs/dra-driver-nvidia-gpu) | | **k8s-nim-operator-ocp** | NVIDIA NIM Operator for OpenShift. Reuses the same upstream chart as `k8s-nim-operator`, with OCP-specific RBAC. Requires `cert-manager-ocp` for admission-webhook TLS. OCP-specific. | [K8s NIM Operator](https://github.com/NVIDIA/k8s-nim-operator) | ## How Components Are Selected diff --git a/pkg/bundler/bundler.go b/pkg/bundler/bundler.go index 23b7ea734..37bd444f8 100644 --- a/pkg/bundler/bundler.go +++ b/pkg/bundler/bundler.go @@ -2837,8 +2837,10 @@ const draChartVersionAnnotation = header.Domain + "/gpu-operator-chart-version" // enabled in the filtered resolved recipe before the annotation is // written; recipes that disable either remain untouched. const ( - gpuOperatorComponentName = "gpu-operator" - draComponentName = "nvidia-dra-driver-gpu" + gpuOperatorComponentName = "gpu-operator" + draComponentName = "nvidia-dra-driver-gpu" + gpuOperatorOCPComponentName = "gpu-operator-ocp" + gpuOperatorOCPOLMComponentName = "gpu-operator-ocp-olm" ) var ( @@ -2933,6 +2935,31 @@ func (b *DefaultBundler) injectDRAChartVersionAnnotation( // is exercised by the disabled-component unit tests. return } + if gpuOperatorComponentName == gpuOperatorOCPComponentName && gpuOperatorVersion == "" { + // gpu-operator-ocp is a ClusterPolicy CR, not a Helm chart, so + // ComponentRef.Version is never populated for it — the empty + // check below would always skip injection on OCP. Fall back to + // the OLM Subscription channel (gpu-operator-ocp-olm) as the + // rollout-trigger value instead. + // + // KNOWN LIMITATION: the channel pin (e.g. "v25.10") only + // changes on a channel re-pin, not on every operator update. + // With installPlanApproval: Automatic (the default — + // components/gpu-operator-ocp-olm/values.yaml), OLM can + // upgrade to newer CSVs inside the same channel — reloading + // the driver — without the channel string changing, so this + // annotation catches bundle-driven operator bumps (a recipe + // regenerated against a different channel) but NOT in-channel + // auto-upgrades. The stale-NVML gap this annotation exists to + // close (#973) remains open for that case on OCP. See #2135. + if olmValues, ok := componentValues[gpuOperatorOCPOLMComponentName]; ok { + if sub, ok := olmValues["subscription"].(map[string]any); ok { + if channel, ok := sub["channel"].(string); ok { + gpuOperatorVersion = channel + } + } + } + } if gpuOperatorVersion == "" { // gpu-operator is enabled but the resolver produced an empty // Version string. This shouldn't happen in normal recipe diff --git a/pkg/bundler/bundler_dra_annotation_test.go b/pkg/bundler/bundler_dra_annotation_test.go index 0b3de87d5..138c73468 100644 --- a/pkg/bundler/bundler_dra_annotation_test.go +++ b/pkg/bundler/bundler_dra_annotation_test.go @@ -193,6 +193,47 @@ func TestInjectDRAChartVersionAnnotation_PreservesExistingValues(t *testing.T) { } } +// TestInjectDRAChartVersionAnnotation_OCPFallbackToOLMChannel pins the +// OCP fallback added for #2135: gpu-operator-ocp is a ClusterPolicy +// CR, not a Helm chart, so ComponentRef.Version is always empty for +// it. Instead of skipping injection (the pre-fix behavior), the +// helper reads the OLM Subscription channel from the +// gpu-operator-ocp-olm component's values and mirrors that onto both +// nvidia-dra-driver-gpu-ocp pod templates. +func TestInjectDRAChartVersionAnnotation_OCPFallbackToOLMChannel(t *testing.T) { + b, err := New() + if err != nil { + t.Fatalf("New() error = %v", err) + } + + const draOCPComponentName = "nvidia-dra-driver-gpu-ocp" + componentValues := map[string]map[string]any{ + gpuOperatorOCPComponentName: {}, + draOCPComponentName: {}, + gpuOperatorOCPOLMComponentName: { + "subscription": map[string]any{ + "channel": "v25.10", + }, + }, + } + rr := &recipe.RecipeResult{ + ComponentRefs: []recipe.ComponentRef{ + {Name: gpuOperatorOCPComponentName, Version: ""}, + {Name: draOCPComponentName, Version: "0.4.1"}, + }, + } + + b.injectDRAChartVersionAnnotation(componentValues, rr) + + for _, podPath := range []string{"controller", "kubeletPlugin"} { + got := dig(componentValues[draOCPComponentName], podPath, "podAnnotations", draChartVersionAnnotation) + if got != "v25.10" { + t.Errorf("podAnnotations[%s][%s] = %v, want v25.10 (OLM channel fallback)", + podPath, draChartVersionAnnotation, got) + } + } +} + // TestInjectDRAChartVersionAnnotation_OverridesUserSet pins the // "internal annotation always reflects the actual chart version" // invariant. A user --set that wrote a stale value into the diff --git a/pkg/bundler/deployer/helm/helm.go b/pkg/bundler/deployer/helm/helm.go index cda48cf51..bc754a98d 100644 --- a/pkg/bundler/deployer/helm/helm.go +++ b/pkg/bundler/deployer/helm/helm.go @@ -56,6 +56,15 @@ type ComponentData struct { IsOCI bool Tag string // Git ref for Kustomize-typed components (tag/branch/commit) Path string // Path within the repository to the kustomization + + // DriverOperatorManaged is true when the bundle's effective values + // select an operator-managed NVIDIA driver — gpu-operator's or + // gpu-operator-ocp's driver.enabled is true. deploy.sh's DRA + // migration-wait block (see #2135, #973) uses this to tell "driver + // is host-managed" apart from "driver is operator-managed but the + // DaemonSet/node-label migration signal isn't observable yet", + // which live cluster state alone cannot distinguish. + DriverOperatorManaged bool } // compile-time interface check @@ -265,6 +274,42 @@ func (g *Generator) Generate(ctx context.Context, outputDir string) (*deployer.O // buildComponentDataList builds a sorted list of ComponentData from the recipe. // It validates that all component names are safe for use as directory names. +// driverOperatorManaged reports whether this bundle's effective values +// select an operator-managed NVIDIA driver: gpu-operator's or +// gpu-operator-ocp's driver.enabled is true. Checks both component names +// since only one is ever enabled in a given recipe (see +// pkg/bundler/bundler.go's gpuOperatorComponentNames for the canonical +// list this mirrors). +// gpuOperatorComponentName and gpuOperatorOCPComponentName are this +// package's copy of the canonical/OCP gpu-operator component names (a +// 4th duplicate alongside pkg/bundler/bundler.go, pkg/bundler/validations +// /checks.go, and their override-key constants — this package cannot +// import pkg/bundler due to the dependency cycle noted at +// componentOverrideKeys' godoc equivalent). Named here, rather than an +// inline literal, so a `grep gpuOperatorOCPComponentName` across the repo +// surfaces every copy that needs updating together. +const ( + gpuOperatorComponentName = "gpu-operator" + gpuOperatorOCPComponentName = "gpu-operator-ocp" +) + +func (g *Generator) driverOperatorManaged() bool { + for _, name := range []string{gpuOperatorComponentName, gpuOperatorOCPComponentName} { + values, ok := g.ComponentValues[name] + if !ok { + continue + } + driver, ok := values["driver"].(map[string]any) + if !ok { + continue + } + if enabled, ok := driver["enabled"].(bool); ok && enabled { + return true + } + } + return false +} + // Only the fields consumed by the orchestration templates are populated. func (g *Generator) buildComponentDataList() ([]ComponentData, error) { // Sort by deployment order @@ -273,6 +318,8 @@ func (g *Generator) buildComponentDataList() ([]ComponentData, error) { g.RecipeResult.DeploymentOrder, ) + driverOperatorManaged := g.driverOperatorManaged() + components := make([]ComponentData, 0, len(sorted)) for _, ref := range sorted { if !deployer.IsSafePathComponent(ref.Name) { @@ -283,14 +330,15 @@ func (g *Generator) buildComponentDataList() ([]ComponentData, error) { chartName := ref.EffectiveChart() components = append(components, ComponentData{ - Name: ref.Name, - Namespace: ref.Namespace, - Repository: ref.Source, - ChartName: chartName, - Version: ref.Version, - IsOCI: strings.HasPrefix(ref.Source, "oci://"), - Tag: ref.Tag, - Path: ref.Path, + Name: ref.Name, + Namespace: ref.Namespace, + Repository: ref.Source, + ChartName: chartName, + Version: ref.Version, + IsOCI: strings.HasPrefix(ref.Source, "oci://"), + Tag: ref.Tag, + Path: ref.Path, + DriverOperatorManaged: driverOperatorManaged, }) } diff --git a/pkg/bundler/deployer/helm/helm_test.go b/pkg/bundler/deployer/helm/helm_test.go index ebd46e3df..17ecd6a80 100644 --- a/pkg/bundler/deployer/helm/helm_test.go +++ b/pkg/bundler/deployer/helm/helm_test.go @@ -287,6 +287,176 @@ func TestGenerate_DeployScriptExecutable(t *testing.T) { } } +// TestGenerate_DeployScript_DRARestartGatedOnDriverOperatorManaged pins the +// fix for #2135's review follow-up: live cluster state alone (absent +// DaemonSet + no labeled node) cannot tell "driver is host-managed" apart +// from "driver is operator-managed but the migration gate hasn't converged +// yet" — the latter must block the DRA kubelet-plugin restart rather than +// running it unguarded, or it reproduces the invalid-CDI/ContainerCreating +// failure (#973). DriverOperatorManaged is derived at bundle time from +// gpu-operator's/gpu-operator-ocp's effective driver.enabled and threaded +// into the rendered script, so this only needs to check the generated +// text — no live cluster required. +func TestGenerate_DeployScript_DRARestartGatedOnDriverOperatorManaged(t *testing.T) { + recipeResult := func() *recipe.RecipeResult { + return &recipe.RecipeResult{ + Kind: "RecipeResult", + APIVersion: "aicr.run/v1alpha2", + Metadata: recipe.RecipeResultMetadata{Version: "v0.1.0"}, + Criteria: &recipe.Criteria{ + Service: "eks", + Accelerator: "h100", + Intent: "training", + }, + ComponentRefs: []recipe.ComponentRef{ + { + Name: "gpu-operator", + Namespace: "gpu-operator", + Chart: "gpu-operator", + Version: "v25.3.3", + Source: "https://helm.ngc.nvidia.com/nvidia", + }, + { + Name: "nvidia-dra-driver-gpu", + Namespace: "nvidia-dra-driver", + Chart: "nvidia-dra-driver-gpu", + Version: "0.4.1", + Source: "https://helm.ngc.nvidia.com/nvidia", + }, + }, + DeploymentOrder: []string{"gpu-operator", "nvidia-dra-driver-gpu"}, + } + } + + tests := []struct { + name string + recipeResultOCP bool // when true, uses OCP component names throughout instead of canonical + componentValues map[string]map[string]any + wantContains []string + wantNotContains []string + }{ + { + name: "operator-managed driver blocks restart until gate is observable", + componentValues: map[string]map[string]any{ + "gpu-operator": { + "driver": map[string]any{"enabled": true}, + }, + "nvidia-dra-driver-gpu": {}, + }, + wantContains: []string{ + `SKIP_RESTART="false"`, + `blocking the DRA plugin restart until the driver rollout is detectable`, + `SKIP_RESTART=true`, + `if [[ -n "${DRA_DS}" && "${SKIP_RESTART}" != "true" ]]; then`, + `no nodes labeled nvidia.com/gpu.deploy.driver=true yet; skipping migration wait and DRA restart`, + }, + wantNotContains: []string{ + `nvidia-driver-daemonset not present (host-managed driver); skipping migration wait"`, + }, + }, + { + name: "host-managed driver still skips the wait without blocking restart", + componentValues: map[string]map[string]any{ + "gpu-operator": { + "driver": map[string]any{"enabled": false}, + }, + "nvidia-dra-driver-gpu": {}, + }, + wantContains: []string{ + `nvidia-driver-daemonset not present (host-managed driver); skipping migration wait"`, + }, + wantNotContains: []string{ + `blocking the DRA plugin restart until the driver rollout is detectable`, + }, + }, + { + name: "OCP DRA component renders its own guard and is gated by gpu-operator-ocp's driver.enabled", + recipeResultOCP: true, + componentValues: map[string]map[string]any{ + "gpu-operator-ocp": { + "driver": map[string]any{"enabled": true}, + }, + "nvidia-dra-driver-gpu-ocp": {}, + }, + wantContains: []string{ + `if [[ "${name}" == "nvidia-dra-driver-gpu-ocp" ]]; then`, + `SKIP_RESTART="false"`, + `blocking the DRA plugin restart until the driver rollout is detectable`, + `SKIP_RESTART=true`, + }, + wantNotContains: []string{ + `if [[ "${name}" == "nvidia-dra-driver-gpu" ]]; then`, + `nvidia-driver-daemonset not present (host-managed driver); skipping migration wait"`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + outputDir := t.TempDir() + + rr := recipeResult() + if tt.recipeResultOCP { + rr = &recipe.RecipeResult{ + Kind: "RecipeResult", + APIVersion: "aicr.run/v1alpha2", + Metadata: recipe.RecipeResultMetadata{Version: "v0.1.0"}, + Criteria: &recipe.Criteria{ + Service: "ocp", + Accelerator: "h100", + Intent: "training", + }, + ComponentRefs: []recipe.ComponentRef{ + { + Name: "gpu-operator-ocp", + Namespace: "gpu-operator", + Chart: "gpu-operator", + Version: "", + Source: "", + }, + { + Name: "nvidia-dra-driver-gpu-ocp", + Namespace: "nvidia-dra-driver", + Chart: "nvidia-dra-driver-gpu", + Version: "0.4.1", + Source: "https://helm.ngc.nvidia.com/nvidia", + }, + }, + DeploymentOrder: []string{"gpu-operator-ocp", "nvidia-dra-driver-gpu-ocp"}, + } + } + + g := &Generator{ + RecipeResult: rr, + ComponentValues: tt.componentValues, + Version: "v1.0.0", + } + + if _, err := g.Generate(ctx, outputDir); err != nil { + t.Fatalf("Generate failed: %v", err) + } + + content, err := os.ReadFile(filepath.Join(outputDir, "deploy.sh")) + if err != nil { + t.Fatalf("failed to read deploy.sh: %v", err) + } + script := string(content) + + for _, want := range tt.wantContains { + if !strings.Contains(script, want) { + t.Errorf("deploy.sh missing %q", want) + } + } + for _, notWant := range tt.wantNotContains { + if strings.Contains(script, notWant) { + t.Errorf("deploy.sh unexpectedly contains %q", notWant) + } + } + }) + } +} + // --------------------------------------------------------------------------- // Property tests (helpers and data-shape preservation) // --------------------------------------------------------------------------- diff --git a/pkg/bundler/deployer/helm/templates/deploy.sh.tmpl b/pkg/bundler/deployer/helm/templates/deploy.sh.tmpl index de1d71147..aaf30e698 100644 --- a/pkg/bundler/deployer/helm/templates/deploy.sh.tmpl +++ b/pkg/bundler/deployer/helm/templates/deploy.sh.tmpl @@ -40,6 +40,7 @@ HELM_TIMEOUT="10m" NO_WAIT=false BEST_EFFORT=false FAILED_COMPONENTS="" +NEEDS_RETRY="" MAX_RETRIES=5 while [[ $# -gt 0 ]]; do @@ -413,8 +414,8 @@ for dir in "${SCRIPT_DIR}"/[0-9][0-9][0-9]-*/; do # --- post-install name-matched blocks --- {{- range .Components }} - {{- if eq .Name "nvidia-dra-driver-gpu" }} - if [[ "${name}" == "nvidia-dra-driver-gpu" ]]; then + {{- if or (eq .Name "nvidia-dra-driver-gpu") (eq .Name "nvidia-dra-driver-gpu-ocp") }} + if [[ "${name}" == "{{ .Name }}" ]]; then # gpu-operator's k8s-driver-manager reloads NVIDIA kernel modules # asynchronously per-node after `helm upgrade gpu-operator` returns. # If the DRA kubelet plugin pod re-rolls (via the chart's @@ -440,20 +441,49 @@ for dir in "${SCRIPT_DIR}"/[0-9][0-9][0-9]-*/; do # --accelerated-node-selector). Waiting on every # gpu.present=true node would block until the 15-min timeout # for any GPU node the operator deliberately excludes. - DRIVER_DS_NS=$(kubectl get daemonset -A -o jsonpath='{.items[?(@.metadata.name=="nvidia-driver-daemonset")].metadata.namespace}' 2>/dev/null | awk '{print $1}') - if [[ -z "${DRIVER_DS_NS}" ]]; then + # Driver-DaemonSet lookup is name-prefix matched, not exact: the + # certified OpenShift build renders it as + # nvidia-driver-daemonset- via the Driver Toolkit, + # so an exact-name match silently fails open on OCP and reports + # "host-managed driver" even when gpu-operator-ocp owns it + # (driver.enabled: true). Node-label presence (gate 2) is checked + # unconditionally and takes priority over the DaemonSet lookup for + # the same reason: the operator applies that label itself, so it + # is a naming-agnostic signal of who owns the driver. + DRIVER_DS_NS=$(kubectl get daemonset -A --no-headers 2>/dev/null | awk '$2 ~ /^nvidia-driver-daemonset(-|$)/ {print $1; exit}') + MANAGED_NODES=$(kubectl get nodes -l nvidia.com/gpu.deploy.driver=true -o name 2>/dev/null | wc -l | tr -d ' ') + SKIP_RESTART="false" + if [[ -z "${DRIVER_DS_NS}" && "${MANAGED_NODES}" -eq 0 ]]; then + {{- if .DriverOperatorManaged }} + # This bundle's effective values select an operator-managed + # driver (driver.enabled=true), so the absence of both runtime + # signals means the migration gate isn't observable YET (the + # operator hasn't converged), not that the driver is + # host-managed. Falling through to the restart below on that + # wrong assumption reproduces the exact invalid-CDI/ + # ContainerCreating failure this block exists to prevent (#973). + # Block the restart; a retried deploy will observe the gate once + # the operator creates the DaemonSet or labels a node. + echo " WARNING: gpu-operator driver.enabled=true but neither the driver DaemonSet nor the migration label is observable yet; blocking the DRA plugin restart until the driver rollout is detectable (retry the deploy)" + SKIP_RESTART=true + NEEDS_RETRY="${NEEDS_RETRY} {{ .Name }}" + {{- else }} echo " gpu-operator nvidia-driver-daemonset not present (host-managed driver); skipping migration wait" - else - MANAGED_NODES=$(kubectl get nodes -l nvidia.com/gpu.deploy.driver=true -o name 2>/dev/null | wc -l | tr -d ' ') - if [[ "${MANAGED_NODES}" -gt 0 ]]; then - echo " Waiting for gpu-operator driver migration on ${MANAGED_NODES} managed GPU node(s) to reach upgrade-done (ns=${DRIVER_DS_NS})..." - if ! kubectl wait --for=jsonpath='{.metadata.labels.nvidia\.com/gpu-driver-upgrade-state}=upgrade-done' \ - nodes -l nvidia.com/gpu.deploy.driver=true --timeout=15m; then - echo " WARNING: not all managed GPU nodes reached upgrade-done within 15m; proceeding with restart anyway" - fi - else - echo " No nodes labeled nvidia.com/gpu.deploy.driver=true yet; skipping migration wait" + {{- end }} + elif [[ "${MANAGED_NODES}" -gt 0 ]]; then + echo " Waiting for gpu-operator driver migration on ${MANAGED_NODES} managed GPU node(s) to reach upgrade-done (ns=${DRIVER_DS_NS:-})..." + if ! kubectl wait --for=jsonpath='{.metadata.labels.nvidia\.com/gpu-driver-upgrade-state}=upgrade-done' \ + nodes -l nvidia.com/gpu.deploy.driver=true --timeout=15m; then + echo " WARNING: not all managed GPU nodes reached upgrade-done within 15m; proceeding with restart anyway" fi + else + # DaemonSet present but no node carries the label yet (e.g. still + # scheduling): fail closed and block the DRA restart below — + # restarting DRA before the driver migration gate is reached can + # leave DRA pods stuck against a mid-migration driver. + echo " gpu-operator driver DaemonSet present (ns=${DRIVER_DS_NS}) but no nodes labeled nvidia.com/gpu.deploy.driver=true yet; skipping migration wait and DRA restart" + SKIP_RESTART="true" + NEEDS_RETRY="${NEEDS_RETRY} {{ .Name }}" fi # Best-effort mitigation for kubelet DRA plugin registration drift. # After uninstall/reinstall, kubelet's fsnotify watcher may not detect new @@ -461,7 +491,7 @@ for dir in "${SCRIPT_DIR}"/[0-9][0-9][0-9]-*/; do # This does NOT fix cases where kubelet itself has lost registration state — # a node reboot is required for that. See docs/user/cli-reference.md. DRA_DS=$(kubectl get daemonset -n {{ .Namespace }} -o name 2>/dev/null | awk '/kubelet-plugin/{print; exit}' || true) - if [[ -n "${DRA_DS}" ]]; then + if [[ -n "${DRA_DS}" && "${SKIP_RESTART}" != "true" ]]; then echo " Restarting DRA kubelet plugin (${DRA_DS##*/}) to ensure registration..." if ! kubectl rollout restart "${DRA_DS}" -n {{ .Namespace }}; then echo " WARNING: failed to restart DRA kubelet plugin daemonset" @@ -471,6 +501,8 @@ for dir in "${SCRIPT_DIR}"/[0-9][0-9][0-9]-*/; do # DRA plugin socket), not a readiness convenience like --wait. echo " WARNING: DRA kubelet plugin rollout did not complete within 120s" fi + elif [[ "${SKIP_RESTART}" == "true" ]]; then + echo " Skipping DRA kubelet plugin restart (driver migration gate not yet reached)" else echo " WARNING: no DRA kubelet plugin daemonset found in {{ .Namespace }}" fi @@ -484,6 +516,16 @@ if [[ -n "${FAILED_COMPONENTS}" ]]; then else _ok "All components installed successfully." fi +if [[ -n "${NEEDS_RETRY}" ]]; then + # Distinct from FAILED_COMPONENTS/helm_failed so --best-effort semantics + # are unaffected: helm itself succeeded, but the DRA kubelet-plugin + # restart was deliberately withheld because the driver-migration gate + # was not yet observable when this deploy ran (see the WARNING above). + # Surfaced with its own non-zero exit below so automated callers (UAT, + # ArgoCD hooks, CI) get an actionable non-success signal instead of + # reading "All components installed successfully." as fully done. + _warn_line "DRA kubelet plugin restart blocked, retry needed for:${NEEDS_RETRY} — re-run this deploy once the operator has converged (driver DaemonSet present or a node carries nvidia.com/gpu.deploy.driver=true)." +fi echo echo "NOTE: The above status reflects Helm install and manifest apply results," echo "not whether the cluster is ready for GPU workloads. On fresh" @@ -494,3 +536,7 @@ echo " - GPU operator operand rollout (driver, toolkit, device-plugin DS)" echo " - NVIDIA DRA kubelet plugin registration" echo echo "See: https://github.com/NVIDIA/aicr/blob/main/docs/user/cli-reference.md#deploy-script-behavior-deploysh" + +if [[ -n "${NEEDS_RETRY}" ]]; then + exit 2 +fi diff --git a/pkg/bundler/validations/checks.go b/pkg/bundler/validations/checks.go index 8e05477fc..d2cf13578 100644 --- a/pkg/bundler/validations/checks.go +++ b/pkg/bundler/validations/checks.go @@ -366,6 +366,10 @@ const gpuOperatorManagedOverrideSet = "--set gpuoperator:driver.enabled=true " + "--set gpuoperator:operator.runtimeClass=nvidia " + "--set dradriver:nvidiaDriverRoot=/run/nvidia/driver" +const ocpGPUOperatorManagedOverrideSet = "--set gpuoperatorocp:driver.enabled=true " + + "--set gpuoperatorocp:toolkit.enabled=true " + + "--set dradriverocp:nvidiaDriverRoot=/run/nvidia/driver" + // gkeGPUOperatorManagedOverrideSet extends the override tuple for GKE // remedies. GKE preinstalled-driver profiles (Google driver installer, // documented for both COS and Ubuntu node images) pin @@ -394,20 +398,25 @@ func legacyRecipeAlternativeRemedy(service recipe.CriteriaServiceType, os recipe "driver, so the GPU-Operator-managed override set is not available there; if the " + "GPU nodes use the GKE-managed driver install, retarget the DRA driver root " + "instead: --set dradriver:nvidiaDriverRoot=" + gkeManagedDriverRootPath + "." - if service != recipe.CriteriaServiceGKE { - return "Or supply the full GPU-Operator-managed override set: " + - gpuOperatorManagedOverrideSet + "." - } - switch os { //nolint:exhaustive // COS and Ubuntu are the only GKE node images with specific wording; everything else (unknown, any, or an OS GKE does not offer) gets both supported GKE paths - case recipe.CriteriaOSCOS: - return gkeCOSAlternative - case recipe.CriteriaOSUbuntu: + switch service { //nolint:exhaustive // only GKE and OCP need dedicated override-key wording; every other service takes the generic gpuOperatorManagedOverrideSet default + case recipe.CriteriaServiceOCP: return "Or supply the full GPU-Operator-managed override set: " + - gkeGPUOperatorManagedOverrideSet + "." + ocpGPUOperatorManagedOverrideSet + "." + case recipe.CriteriaServiceGKE: + switch os { //nolint:exhaustive // COS and Ubuntu are the only GKE node images with specific wording; everything else (unknown, any, or an OS GKE does not offer) gets both supported GKE paths + case recipe.CriteriaOSCOS: + return gkeCOSAlternative + case recipe.CriteriaOSUbuntu: + return "Or supply the full GPU-Operator-managed override set: " + + gkeGPUOperatorManagedOverrideSet + "." + default: + return gkeCOSAlternative + " On GKE Ubuntu node images the GPU Operator can manage " + + "the driver, so those may instead supply the full GPU-Operator-managed " + + "override set: " + gkeGPUOperatorManagedOverrideSet + "." + } default: - return gkeCOSAlternative + " On GKE Ubuntu node images the GPU Operator can manage " + - "the driver, so those may instead supply the full GPU-Operator-managed " + - "override set: " + gkeGPUOperatorManagedOverrideSet + "." + return "Or supply the full GPU-Operator-managed override set: " + + gpuOperatorManagedOverrideSet + "." } } @@ -425,7 +434,7 @@ func legacyRecipeAlternativeRemedy(service recipe.CriteriaServiceType, os recipe // (see gpuOperatorManagedOverrideSet above for why the duplication // exists). func driverAbsentRemedy(service recipe.CriteriaServiceType, os recipe.CriteriaOSType, profiled bool) string { - switch service { //nolint:exhaustive // only AKS and GKE have provider-specific wording; every other service takes the generic default + switch service { //nolint:exhaustive // only AKS, GKE, and OCP have provider-specific wording; every other service takes the generic default case recipe.CriteriaServiceAKS: if !profiled { // Legacy pre-profile artifact: the ownership lock does not @@ -477,6 +486,10 @@ func driverAbsentRemedy(service recipe.CriteriaServiceType, os recipe.CriteriaOS "driver, so those may bundle in GPU-Operator-managed mode: " + gkeGPUOperatorManagedOverrideSet + "." } + case recipe.CriteriaServiceOCP: + return "Either reprovision the GPU nodes with a platform-installed " + + "NVIDIA driver, or bundle in GPU-Operator-managed mode: " + + ocpGPUOperatorManagedOverrideSet + "." default: return "Either reprovision the GPU nodes with a platform-installed " + "NVIDIA driver, or bundle in GPU-Operator-managed mode: " + @@ -973,7 +986,7 @@ func draLockstepViolations(ctx context.Context, recipeResult *recipe.RecipeResul "populates that path when the operator does not manage the driver. This is "+ "commonly the signature of a recipe generated before the preinstalled-driver "+ "default flip: regenerate the recipe (aicr recipe ...) for this AICR version. %s", - componentName, draDriverComponentName, operatorContainerDriverRoot, + componentName, draRef.Name, operatorContainerDriverRoot, legacyRecipeAlternativeRemedy(service, osCriteria))) } return msgs, nil diff --git a/pkg/bundler/validations/checks_test.go b/pkg/bundler/validations/checks_test.go index cf2036bdc..035e3339a 100644 --- a/pkg/bundler/validations/checks_test.go +++ b/pkg/bundler/validations/checks_test.go @@ -717,6 +717,36 @@ func TestCheckConditions(t *testing.T) { } } +// TestDriverAbsentRemedy_OCP pins the OCP branch added for #2135: +// driverAbsentRemedy must use the OCP-specific override keys +// (gpuoperatorocp:/dradriverocp:) rather than falling through to the +// generic gpuoperator:/dradriver: default, since those keys don't +// resolve against an OCP recipe's registry aliases. Also asserts the +// generic-default keys are NOT present, so a future edit that +// accidentally reuses gpuOperatorManagedOverrideSet for OCP fails +// loudly instead of silently. +func TestDriverAbsentRemedy_OCP(t *testing.T) { + got := driverAbsentRemedy(recipe.CriteriaServiceOCP, "", false) + + wantContains := []string{ + "--set gpuoperatorocp:driver.enabled=true", + "--set gpuoperatorocp:toolkit.enabled=true", + "--set dradriverocp:nvidiaDriverRoot=/run/nvidia/driver", + } + for _, want := range wantContains { + if !strings.Contains(got, want) { + t.Errorf("driverAbsentRemedy(OCP) missing %q:\n%s", want, got) + } + } + + dontWant := []string{"gpuoperator:", "dradriver:"} + for _, unwanted := range dontWant { + if strings.Contains(got, unwanted) { + t.Errorf("driverAbsentRemedy(OCP) unexpectedly contains generic-default key %q:\n%s", unwanted, got) + } + } +} + // TestCheckDriverOwnershipCoherence covers the bundle-time // driver-ownership gate on FINAL effective values: Rule 1 (a recipe whose // snapshot observed no NVIDIA driver on the sampled GPU node — @@ -769,6 +799,7 @@ func TestCheckDriverOwnershipCoherence(t *testing.T) { recipeResult *recipe.RecipeResult bundlerConfig *config.Config conditions map[string][]string + componentName string // defaults to "gpu-operator" when empty; set to test an OCP-alias row through the exact-match componentName entrypoint wantMsgs int wantContains []string wantErrs int @@ -974,6 +1005,26 @@ func TestCheckDriverOwnershipCoherence(t *testing.T) { wantMsgs: 1, wantContains: []string{"nothing", "regenerate the recipe", "--set gpuoperator:driver.enabled=true"}, }, + { + // Same Rule-2 legacy signature, but through the OCP aliases: + // gpu-operator-ocp/nvidia-dra-driver-gpu-ocp. Verifies the + // remedy names the resolved OCP component (draRef.Name, not + // the canonical constant) and the OCP override keys — the + // generic gpuoperator:/dradriver: set would hard-fail on OCP + // since recipes/overlays/ocp.yaml disables the canonical + // gpu-operator component. + name: "Rule 2: OCP alias lockstep names the OCP component and OCP override keys", + componentName: "gpu-operator-ocp", + recipeResult: result("", recipe.CriteriaServiceOCP, + recipe.ComponentRef{Name: "gpu-operator-ocp", Overrides: driverOff()}, + recipe.ComponentRef{Name: "nvidia-dra-driver-gpu-ocp", Overrides: rootAt("/run/nvidia/driver")}), + wantMsgs: 1, + wantContains: []string{ + "nvidia-dra-driver-gpu-ocp", + "--set gpuoperatorocp:driver.enabled=true", + "--set dradriverocp:nvidiaDriverRoot=/run/nvidia/driver", + }, + }, { name: "Rule 2: driver off + DRA root=/ (preinstalled profile) → passes", recipeResult: result("", aks, @@ -1495,8 +1546,12 @@ func TestCheckDriverOwnershipCoherence(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + componentName := tt.componentName + if componentName == "" { + componentName = "gpu-operator" + } msgs, errs := CheckDriverOwnershipCoherence( - context.Background(), "gpu-operator", tt.recipeResult, tt.bundlerConfig, tt.conditions) + context.Background(), componentName, tt.recipeResult, tt.bundlerConfig, tt.conditions) if len(errs) != tt.wantErrs { t.Fatalf("hard errors = %d (%v), want %d", len(errs), errs, tt.wantErrs) } diff --git a/pkg/client/v1/gpu_driver_state.go b/pkg/client/v1/gpu_driver_state.go index db299ab7a..2e9bc6d87 100644 --- a/pkg/client/v1/gpu_driver_state.go +++ b/pkg/client/v1/gpu_driver_state.go @@ -50,6 +50,15 @@ const gpuOperatorManagedOverrideSet = "--set gpuoperator:driver.enabled=true " + "--set gpuoperator:operator.runtimeClass=nvidia " + "--set dradriver:nvidiaDriverRoot=/run/nvidia/driver" +// ocpGPUOperatorManagedOverrideSet is gpuOperatorManagedOverrideSet's OCP +// counterpart — see the sibling constant of the same name in +// pkg/bundler/validations/checks.go for why the keys and fields differ +// (gpuoperatorocp:/dradriverocp: aliases, no operator.runtimeClass on +// the ClusterPolicy CR). Keep both copies in sync. +const ocpGPUOperatorManagedOverrideSet = "--set gpuoperatorocp:driver.enabled=true " + + "--set gpuoperatorocp:toolkit.enabled=true " + + "--set dradriverocp:nvidiaDriverRoot=/run/nvidia/driver" + // gkeGPUOperatorManagedOverrideSet extends the override tuple for GKE // remedies. GKE preinstalled-driver profiles (Google driver installer, // documented for both COS and Ubuntu node images) pin @@ -72,7 +81,7 @@ const gkeGPUOperatorManagedOverrideSet = gpuOperatorManagedOverrideSet + // anything else gets the generic reprovision wording plus the override // set. func driverAbsentRemedy(service recipe.CriteriaServiceType, os recipe.CriteriaOSType, profiled bool) string { - switch service { //nolint:exhaustive // only AKS and GKE have provider-specific wording; every other service takes the generic default + switch service { //nolint:exhaustive // only AKS, GKE, and OCP have provider-specific wording; every other service takes the generic default case recipe.CriteriaServiceAKS: if !profiled { // Legacy pre-profile artifact: the ownership lock does not @@ -124,6 +133,10 @@ func driverAbsentRemedy(service recipe.CriteriaServiceType, os recipe.CriteriaOS "driver, so those may bundle in GPU-Operator-managed mode: " + gkeGPUOperatorManagedOverrideSet + "." } + case recipe.CriteriaServiceOCP: + return "Either reprovision the GPU nodes with a platform-installed " + + "NVIDIA driver, or bundle in GPU-Operator-managed mode: " + + ocpGPUOperatorManagedOverrideSet + "." default: return "Either reprovision the GPU nodes with a platform-installed " + "NVIDIA driver, or bundle in GPU-Operator-managed mode: " + diff --git a/pkg/client/v1/gpu_driver_state_test.go b/pkg/client/v1/gpu_driver_state_test.go index 30b0b325c..0c962f282 100644 --- a/pkg/client/v1/gpu_driver_state_test.go +++ b/pkg/client/v1/gpu_driver_state_test.go @@ -599,19 +599,22 @@ func TestDriverAbsentRemedyBranches(t *testing.T) { os recipe.CriteriaOSType profiled bool want string + notWant string // optional: substring that must NOT appear; skipped when empty }{ {"aks legacy keeps the four-flag tuple", recipe.CriteriaServiceAKS, recipe.CriteriaOSUbuntu, false, - "bundle in GPU-Operator-managed mode"}, + "bundle in GPU-Operator-managed mode", ""}, {"aks profiled points at --profile", recipe.CriteriaServiceAKS, recipe.CriteriaOSUbuntu, true, - "--profile gpuStack=operator-managed"}, + "--profile gpuStack=operator-managed", ""}, {"gke cos forbids operator install", recipe.CriteriaServiceGKE, recipe.CriteriaOSCOS, false, - "GPU Operator cannot install the driver"}, + "GPU Operator cannot install the driver", ""}, {"gke ubuntu allows operator mode", recipe.CriteriaServiceGKE, recipe.CriteriaOSUbuntu, false, - "GKE Ubuntu node images the GPU Operator can manage"}, + "GKE Ubuntu node images the GPU Operator can manage", ""}, {"gke unknown os presents both paths", recipe.CriteriaServiceGKE, recipe.CriteriaOSAny, false, - "those may bundle in GPU-Operator-managed mode"}, + "those may bundle in GPU-Operator-managed mode", ""}, {"generic service gets the platform wording", recipe.CriteriaServiceEKS, recipe.CriteriaOSUbuntu, false, - "reprovision the GPU nodes with a platform-installed"}, + "reprovision the GPU nodes with a platform-installed", ""}, + {"ocp uses the ocp-aliased override keys", recipe.CriteriaServiceOCP, recipe.CriteriaOSAny, false, + "gpuoperatorocp:driver.enabled=true", "gpuoperator:"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -620,6 +623,10 @@ func TestDriverAbsentRemedyBranches(t *testing.T) { t.Errorf("driverAbsentRemedy(%s,%s,%v) = %q, want substring %q", tt.service, tt.os, tt.profiled, got, tt.want) } + if tt.notWant != "" && strings.Contains(got, tt.notWant) { + t.Errorf("driverAbsentRemedy(%s,%s,%v) = %q, unexpectedly contains %q", + tt.service, tt.os, tt.profiled, got, tt.notWant) + } }) } // The AKS legacy and profiled remedies must be distinct: the lock only