fix(gke): address review — retrofit flow, gate nil-safety, ctx, tests

- Rewrite the retrofit procedure for the bundle flow: step 0 deletes any
  hand-applied nvidia-driver-installer DaemonSet (Helm will not adopt the
  identically named object), and the ordering becomes driver-mode ->
  opt-out label -> bundle, with the scheduling gap and rollback semantics
  restated for the bundle-carried installer.
- Make the installer's render gate nil-safe: a wholesale override that
  drops the installer key now renders nothing instead of failing the
  template (index of untyped nil).
- Thread the deployment-phase context through gatedHealthCheckSuppressed
  and emptyRenderHealthCheckSuppressed (ctx-aware provider variants plus a
  per-manifest cancellation check), preserving the validator cancellation
  contract.
- Add table-driven tests for the gated health-check dispatch: gated off,
  gated on, missing gate key, no manifests, unreadable manifest,
  non-gated component, canceled context.

Signed-off-by: Atif Mahmood <atif1996@users.noreply.github.com>

Author: atif1996

docs/integrator/gke-gpu-setup.md

@@ -240,24 +240,24 @@ section above apply here unchanged.
 #### Retrofitting an existing pool
 
 For a GPU node pool that already exists, do the retrofit in this order:
-standalone driver ready **first**, then the opt-out label, then the GPU
-Operator. The label takes effect the moment it lands, disabling the
-kube-system DaemonSet whose init container finalizes GKE's managed driver
-install — so on a labeled pool still set to `gpu-driver-version=default` (or
-`latest`), every node created in the interim (autoscaling, upgrade,
-auto-repair) comes up **driverless**. Deploying the standalone installer
-alone does not close that gap either — Google's `nvidia-driver-installer`
-DaemonSet
-[ignores nodes configured for automatic driver installation](https://cloud.google.com/kubernetes-engine/docs/troubleshooting/gpus#gpu_device_plugins_fail_with_crashloopbackoff_errors),
-so it skips every node of a pool whose driver mode is still `default`. Only
-the sequence below keeps the pool functional at every step.
-
-**Step 1 — apply the standalone
-[`nvidia-driver-installer` DaemonSet](https://cloud.google.com/kubernetes-engine/docs/how-to/gpus#installing_drivers).**
-Applying it early is safe: it skips automatic-install nodes, so it is a
-no-op until step 2 flips the pool's driver mode.
-
-**Step 2 — switch the pool to `gpu-driver-version=disabled`** (restate the
+driver mode first, then the opt-out label, then the bundle. The bundle
+carries the driver installer, so — unlike the hand-applied arrangement this
+replaces — there is nothing to apply out-of-band, but the installer only
+arrives with the bundle in step 3: between step 1 and step 3 the pool has a
+scheduling gap, described per step below. Plan the retrofit as one sitting
+with the bundle generated in advance.
+
+**Step 0 — if migrating from a hand-applied installer, delete it first.**
+The bundle's DaemonSet shares the name `nvidia-driver-installer` in
+`kube-system`, and Helm will not adopt a pre-existing object — step 3 would
+fail. Nodes keep their loaded drivers; only the provisioning workload is
+replaced.
+
+```bash
+kubectl delete daemonset -n kube-system nvidia-driver-installer --ignore-not-found
+```
+
+**Step 1 — switch the pool to `gpu-driver-version=disabled`** (restate the
 pool's actual accelerator type and count):
 
 ```bash
@@ -267,31 +267,20 @@ gcloud container node-pools update POOL_NAME \
   --accelerator type=nvidia-h100-80gb,count=8,gpu-driver-version=disabled
 ```
 
-The driver-mode update may re-create the pool's nodes; with the standalone
-installer already applied, re-created and future nodes come up with a
-driver, and GKE's device plugin (not yet disabled) keeps advertising
-`nvidia.com/gpu` — the pool stays schedulable throughout.
-
-**Step 3 — verify the driver before touching the label.** Every GPU node
-should be running the installer's pods and still report non-zero allocatable
-`nvidia.com/gpu` (advertised, for now, by GKE's plugin):
-
-```bash
-kubectl get pods -n kube-system -l k8s-app=nvidia-driver-installer -o wide
-kubectl get nodes -l cloud.google.com/gke-accelerator \
-  -o custom-columns='NAME:.metadata.name,GPU:.status.allocatable.nvidia\.com/gpu'
-```
-
-**Step 4 — apply the opt-out label.** This begins the handoff: the label
-immediately evicts GKE's managed plugin, so from this point until step 5's
-Operator plugin registers, the pool has **no** `nvidia.com/gpu` advertiser
-and GPU pods will not schedule. That brief advertiser-free window is the
-accepted cost of the handoff direction — do **not** invert it by deploying
-the Operator's plugin onto a still-unlabeled pool, which would put two
-advertisers on the same nodes (the dual-advertisement state the
+The driver-mode update may re-create the pool's nodes. Until step 3's
+installer runs, re-created nodes come up **driverless while GKE's plugin
+still advertises** `nvidia.com/gpu` on them — GPU pods scheduled there will
+fail. Avoid scheduling GPU work from here until the handoff completes
+(cordon the pool's nodes for GPU workloads if the cluster is busy).
+
+**Step 2 — apply the opt-out label.** This evicts GKE's managed plugin, so
+from this point until step 3's Operator plugin registers, the pool has
+**no** `nvidia.com/gpu` advertiser — which also stops the driverless nodes
+from being advertised. Do **not** invert the order by deploying the bundle
+onto a still-unlabeled pool: that would put two advertisers on the same
+nodes (the dual-advertisement state the
 [allocation-policy gates](#the-three-bundle-installer-settings) exist to
-prevent). Have the bundle from step 5 generated in advance to keep the
-window short, and avoid scheduling GPU work during it.
+prevent).
 Note that `--node-labels` on update **replaces** the pool's full user-label
 set: first list the labels the pool already carries, then pass the complete
 set with the new label appended:
@@ -315,18 +304,22 @@ expects — without it, `value(config.labels)` joins entries with semicolons,
 which the update rejects. Omitting an existing label removes it from the
 pool's nodes, which can break scheduling that depends on it.
 
-**Step 5 — deploy the GPU Operator and wait for its plugin.** Deploy the
-AICR bundle generated with `--profile gpuStack=bundle-installer`, then wait
-until the Operator's device-plugin pods are Running on the labeled nodes and
-every GPU node again reports non-zero allocatable `nvidia.com/gpu` — that
-closes the advertiser-free window opened in step 4. Confirm the full result
-with the checks in [Verifying the handoff](#verifying-the-handoff).
+**Step 3 — deploy the bundle.** Deploy the AICR bundle generated with
+`--profile gpuStack=bundle-installer`. Its `gcp-driver-installer` DaemonSet
+installs the pinned driver on the labeled, driver-mode-disabled nodes
+(nodes that already have a loaded driver are skipped), and the GPU
+Operator's device plugin registers once the driver is ready — closing the
+window opened in steps 1–2. Wait until every GPU node again reports
+non-zero allocatable `nvidia.com/gpu`, then confirm the full result with
+the checks in [Verifying the handoff](#verifying-the-handoff).
 
 **Rollback:** if the Operator's device plugin fails to come up after the
 label lands, remove the label (another `--node-labels` update passing the
-full set with the opt-out label omitted) — GKE's plugin returns and the pool
-resumes advertising GPUs, with the driver still supplied by the standalone
-installer.
+full set with the opt-out label omitted) — GKE's plugin returns and the
+pool resumes advertising GPUs. Nodes the bundle's installer already
+provisioned keep their driver; nodes re-created before step 3 ran are
+driverless until the pool's driver mode is restored
+(`gpu-driver-version=default`) or the bundle deploys.
 
 #### Verifying the handoff
 
@@ -426,7 +419,7 @@ stay `Pending`, even though the driver is present and healthy.
 (yet) registered. The label immediately evicts GKE's managed plugin, so
 until the Operator's plugin comes up, the node has no `nvidia.com/gpu`
 advertiser at all. A brief window in this state is the expected
-intermediate step of the retrofit handoff (step 4 of
+intermediate step of the retrofit handoff (step 2 of
 [Retrofitting an existing pool](#retrofitting-an-existing-pool)); it is a
 problem only when nothing closes it.
 

recipes/components/gcp-driver-installer/manifests/nvidia-driver-installer.yaml

@@ -52,7 +52,10 @@
 # bundle-installer profile value flips installer.enabled true.
 # Default-off fails closed: rendering the installer is a profile
 # decision, never a chart default.
-{{- if eq (toString (index (index $vals "installer") "enabled")) "true" }}
+{{- $installer := index $vals "installer" }}
+{{- $gate := "" }}
+{{- if $installer }}{{- $gate = toString (index $installer "enabled") }}{{- end }}
+{{- if eq $gate "true" }}
 ---
 apiVersion: apps/v1
 kind: DaemonSet

validators/deployment/expected_resources.go

@@ -246,7 +246,7 @@ func checkExpectedResources(ctx *validators.Context) error {
 			// in that case, mirroring the render-aware Go readiness check. Only
 			// nodewright-customizations is subject to this; a render/read error
 			// propagates rather than silently skipping. See #1844.
-			suppressed, reason, suppressErr := gatedHealthCheckSuppressed(ref)
+			suppressed, reason, suppressErr := gatedHealthCheckSuppressed(ctx.Ctx, ref)
 			if suppressErr != nil {
 				return suppressErr
 			}
@@ -686,13 +686,14 @@ func isRuntimeRequiredTaint(t *corev1.Taint) bool {
 // suppress. Every other component's assert queues unconditionally.
 // Fail-closed throughout: a render or read error propagates so a broken
 // template is never mistaken for "nothing to assert".
-func gatedHealthCheckSuppressed(ref recipe.ComponentRef) (bool, string, error) {
+func gatedHealthCheckSuppressed(goCtx context.Context, ref recipe.ComponentRef) (bool, string, error) {
 	switch ref.Name {
 	case nodewrightCustomizationsComponent:
+		//nolint:contextcheck // pre-existing ctx-less chain (expectedNodewrightNames); threading ctx through it is tracked separately from this dispatch.
 		suppressed, err := nodewrightHealthCheckSuppressed(ref)
 		return suppressed, "effective values suppress the tuning Skyhook CR (see #1844)", err
 	case gcpDriverInstallerComponent:
-		suppressed, err := emptyRenderHealthCheckSuppressed(ref)
+		suppressed, err := emptyRenderHealthCheckSuppressed(goCtx, ref)
 		return suppressed, "effective values gate the component off (installer.enabled=false); it renders no objects", err
 	default:
 		return false, "", nil
@@ -705,13 +706,13 @@ func gatedHealthCheckSuppressed(ref recipe.ComponentRef) (bool, string, error) {
 // profile values; a non-selected value renders an empty release). A static
 // health-check assert cannot see value gates and would fail on a healthy
 // cluster where the render is deliberately empty.
-func emptyRenderHealthCheckSuppressed(ref recipe.ComponentRef) (bool, error) {
+func emptyRenderHealthCheckSuppressed(goCtx context.Context, ref recipe.ComponentRef) (bool, error) {
 	if len(ref.ManifestFiles) == 0 {
 		// Nothing to render — leave the assert in place so its own failure
 		// surfaces the problem.
 		return false, nil
 	}
-	values, err := recipe.GetComponentValues(&ref)
+	values, err := recipe.GetComponentValuesWithContext(goCtx, nil, &ref)
 	if err != nil {
 		return false, errors.Wrap(errors.ErrCodeInternal,
 			fmt.Sprintf("failed to resolve effective values for component %s", ref.Name), err)
@@ -728,7 +729,15 @@ func emptyRenderHealthCheckSuppressed(ref recipe.ComponentRef) (bool, error) {
 		Values:        values,
 	}
 	for _, path := range ref.ManifestFiles {
-		content, err := recipe.GetManifestContent(path)
+		// Preserve the validator cancellation contract: reads and renders in
+		// this loop must stop once the deployment phase is canceled.
+		select {
+		case <-goCtx.Done():
+			return false, errors.Wrap(errors.ErrCodeTimeout,
+				"deployment validation canceled during gated health-check evaluation", goCtx.Err())
+		default:
+		}
+		content, err := recipe.GetManifestContentWithContext(goCtx, nil, path)
 		if err != nil {
 			return false, errors.Wrap(errors.ErrCodeInternal,
 				fmt.Sprintf("failed to load manifest %s for component %s", path, ref.Name), err)

validators/deployment/expected_resources_test.go

@@ -1526,3 +1526,115 @@ func TestRDMAFabricProbeCoverage_CountsCordonedOnFailClosed(t *testing.T) {
 		t.Errorf("total() = %d, want 2 (cordoned counted even on failure)", got)
 	}
 }
+
+// TestGatedHealthCheckSuppressed pins the render-aware static-assert
+// suppression dispatch for values-gated components: the gcp-driver-installer
+// health check is skipped exactly when the effective values gate the render
+// off, other components' asserts queue unconditionally, and render/read
+// failures propagate rather than being read as "nothing to assert".
+func TestGatedHealthCheckSuppressed(t *testing.T) {
+	t.Parallel()
+
+	installerManifest := "components/gcp-driver-installer/manifests/nvidia-driver-installer.yaml"
+	tests := []struct {
+		name           string
+		ref            recipe.ComponentRef
+		wantSuppressed bool
+		wantErr        bool
+	}{
+		{
+			name: "installer gated off (default values) suppresses the assert",
+			ref: recipe.ComponentRef{
+				Name:          "gcp-driver-installer",
+				Type:          recipe.ComponentTypeHelm,
+				ValuesFile:    "components/gcp-driver-installer/values.yaml",
+				ManifestFiles: []string{installerManifest},
+			},
+			wantSuppressed: true,
+		},
+		{
+			// A wholesale override can drop the gate key entirely; the
+			// template must fail closed to not-rendering, never panic.
+			name: "missing gate key renders nothing and suppresses",
+			ref: recipe.ComponentRef{
+				Name:          "gcp-driver-installer",
+				Type:          recipe.ComponentTypeHelm,
+				ManifestFiles: []string{installerManifest},
+			},
+			wantSuppressed: true,
+		},
+		{
+			name: "installer gated on renders objects and keeps the assert",
+			ref: recipe.ComponentRef{
+				Name:          "gcp-driver-installer",
+				Type:          recipe.ComponentTypeHelm,
+				ManifestFiles: []string{installerManifest},
+				Overrides: map[string]any{
+					"installer": map[string]any{"enabled": true},
+				},
+			},
+			wantSuppressed: false,
+		},
+		{
+			name: "no manifests leaves the assert in place",
+			ref: recipe.ComponentRef{
+				Name: "gcp-driver-installer",
+				Type: recipe.ComponentTypeHelm,
+			},
+			wantSuppressed: false,
+		},
+		{
+			name: "unreadable manifest fails closed",
+			ref: recipe.ComponentRef{
+				Name:          "gcp-driver-installer",
+				Type:          recipe.ComponentTypeHelm,
+				ManifestFiles: []string{"components/gcp-driver-installer/manifests/no-such-file.yaml"},
+			},
+			wantErr: true,
+		},
+		{
+			name: "non-gated component queues unconditionally",
+			ref: recipe.ComponentRef{
+				Name:          "gpu-operator",
+				Type:          recipe.ComponentTypeHelm,
+				ManifestFiles: []string{installerManifest},
+			},
+			wantSuppressed: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			suppressed, reason, err := gatedHealthCheckSuppressed(t.Context(), tt.ref)
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("gatedHealthCheckSuppressed() error = nil, want failure")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("gatedHealthCheckSuppressed() error = %v", err)
+			}
+			if suppressed != tt.wantSuppressed {
+				t.Errorf("suppressed = %v, want %v", suppressed, tt.wantSuppressed)
+			}
+			if suppressed && reason == "" {
+				t.Error("suppressed with an empty reason — operators need the why")
+			}
+		})
+	}
+
+	t.Run("canceled context stops manifest evaluation", func(t *testing.T) {
+		t.Parallel()
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		_, _, err := gatedHealthCheckSuppressed(ctx, recipe.ComponentRef{
+			Name:          "gcp-driver-installer",
+			Type:          recipe.ComponentTypeHelm,
+			ManifestFiles: []string{installerManifest},
+		})
+		if err == nil {
+			t.Fatal("gatedHealthCheckSuppressed() error = nil, want cancellation")
+		}
+	})
+}