fix(validator): fail closed when a declared GPU driver floor cannot be measured

Skip on cordoned or busy GPU nodes is non-blocking, so a host-driver
floor would otherwise PASS unevaluated. Block those paths when
Deployment.gpu-driver.version is set, preserve Skip when it is not,
and keep Evaluate()'s InvalidRequest code.

Signed-off-by: Rohit Rajani <rorajani@nvidia.com>

Author: rorajani

docs/contributor/recipe.md

@@ -351,8 +351,15 @@ Host-managed driver floors (GKE COS / A4X Max and similar platforms where
 `check-nvidia-smi` evaluates it against the nvidia-smi banner on each
 verified node; when the constraint is absent the check keeps its
 banner-presence behavior and does not invent a floor (#1995).
-When the constraint is set but no parseable numeric driver version is
-available, the check fails closed.
+When the constraint is set but the host driver cannot be measured —
+unreadable nvidia-smi banner, no GPU nodes, all GPU nodes cordoned, or
+GPU nodes busy with workloads — the check fails closed rather than
+Skip. Skip on those paths is preserved only when no floor is
+configured. The value must carry a comparison operator (`>=`, `>`,
+`<=`, `<`) to behave as a floor; a bare version is exact string match,
+so a newer driver would fail. The constraint name is an exact match;
+a typo silently disables the floor (shared with
+`Deployment.gpu-operator.version`).
 
 For a query `{service: eks, accelerator: gb200, intent: training}`,
 the resolver returns three independent maximal leaves —

docs/contributor/validator.md

@@ -611,6 +611,11 @@ tagged with its cordon state, and:
    schedulable count: it is `0` on the all-cordoned and busy-skip
    paths (nothing was attempted yet) and the successful-node count on
    the failure path (a partial pass is not conflated with a full one).
+   Exception: when the recipe declares `Deployment.gpu-driver.version`,
+   those same all-cordoned and busy paths (and the no-GPU-nodes path)
+   fail closed instead of Skip — a declared host-driver floor that
+   cannot be measured must not PASS (#1995). Skip remains only when
+   the floor constraint is absent.
    The `RESULT:` prefix is `pkg/validator/validator.go`'s
    `resultSummaryPrefix` convention: the validator runtime echoes the
    trailing text of any such stdout line into live CLI output via

recipes/validators/catalog.yaml

@@ -62,7 +62,7 @@ validators:
     phase: deployment
     description: >-
       Verify nvidia-smi on schedulable GPU nodes; enforce
-      Deployment.gpu-driver.version when set (cordoned nodes disclosed)
+      Deployment.gpu-driver.version when set (unmeasurable floor blocks)
     image: ghcr.io/nvidia/aicr-validators/deployment:latest
     timeout: 10m
     args: ["check-nvidia-smi"]

validators/deployment/nvidia_smi.go

@@ -62,8 +62,12 @@ const (
 	// host-managed drivers (e.g. GKE A4X Max requiring R580.95.05+) cannot be
 	// gated by GPU Operator version alone when driver.enabled is false; this
 	// constraint is evaluated against the nvidia-smi banner on each verified
-	// node. Absent from the recipe, the check keeps its original banner-presence
-	// behavior and does not invent a floor.
+	// node. The value must carry a comparison operator (typically ">=") to
+	// behave as a floor; a bare version is exact string match. Absent from the
+	// recipe, the check keeps its original banner-presence behavior and does
+	// not invent a floor. When the constraint is set but no node can be
+	// measured (no GPU nodes, all cordoned, or busy), the check fails closed
+	// instead of Skip — a declared gate must not PASS unenforced.
 	gpuDriverVersionConstraint = "Deployment.gpu-driver.version"
 )
 
@@ -184,6 +188,10 @@ func checkNvidiaSMI(ctx *validators.Context) error {
 				"recipe declares gpu-operator but the cluster has no GPU nodes to verify — "+
 					"check node provisioning, the GPU Operator rollout, or validator RBAC")
 		}
+		if err := failClosedIfUnmeasurableGPUDriverFloor(ctx,
+			"no GPU nodes found in the cluster"); err != nil {
+			return err
+		}
 		emitExtraOrWarn(nvidiaSMISkipExtra(skipReasonNoGPUNodes))
 		return validators.Skip("no GPU nodes found in the cluster")
 	}
@@ -198,9 +206,13 @@ func checkNvidiaSMI(ctx *validators.Context) error {
 		// probe error. There is nothing schedulable to verify, so Skip with a
 		// distinct, accurate code — never a false "no-gpu-nodes" in the signed
 		// evidence.
+		reason := fmt.Sprintf(
+			"all %d GPU node(s) are cordoned; nothing to verify", len(coverage.cordoned))
+		if err := failClosedIfUnmeasurableGPUDriverFloor(ctx, reason); err != nil {
+			return err
+		}
 		emitExtraOrWarn(nvidiaSMISkipExtra(skipReasonNoSchedulableGPUNodes))
-		return validators.Skip(fmt.Sprintf(
-			"all %d GPU node(s) are cordoned; nothing to verify", len(coverage.cordoned)))
+		return validators.Skip(reason)
 	}
 
 	// Check if any nodes are busy.
@@ -231,11 +243,16 @@ func checkNvidiaSMI(ctx *validators.Context) error {
 	if len(busyNodes) > 0 {
 		printLines(coverage.coverageLine(0))
 		if confirmedBusy {
-			// At least one node was CONFIRMED occupied: a legitimate
-			// scope-narrowing Skip. Sign nodes-busy so the signed evidence records
-			// the occupancy. Probe errors on other nodes are already logged above.
+			// At least one node was CONFIRMED occupied. Without a host-driver
+			// floor this is a legitimate scope-narrowing Skip. With a declared
+			// floor, occupancy is not a reason to leave the gate unevaluated
+			// (#1995): Skip is non-blocking, so a below-floor cluster would PASS.
+			reason := fmt.Sprintf("GPU nodes busy with existing workloads: %v", busyNodes)
+			if err := failClosedIfUnmeasurableGPUDriverFloor(ctx, reason); err != nil {
+				return err
+			}
 			emitExtraOrWarn(nvidiaSMISkipExtra(skipReasonNodesBusy))
-			return validators.Skip(fmt.Sprintf("GPU nodes busy with existing workloads: %v", busyNodes))
+			return validators.Skip(reason)
 		}
 		// #2122 fail-closed: every "busy" node was actually a busy-probe ERROR —
 		// the probe proved occupancy on no node. An infra error (RBAC denial,
@@ -306,6 +323,21 @@ func nvidiaSMISkipExtra(reason string) map[string]string {
 	return map[string]string{"skipReason": reason}
 }
 
+// failClosedIfUnmeasurableGPUDriverFloor returns a blocking error when the
+// recipe declares Deployment.gpu-driver.version but check-nvidia-smi cannot
+// run per-node verification (no GPU nodes, all cordoned, or busy). Skip on
+// those paths is non-blocking; a declared floor that cannot be measured must
+// not PASS (#1995). No constraint keeps the existing Skip.
+func failClosedIfUnmeasurableGPUDriverFloor(ctx *validators.Context, reason string) error {
+	expr, found := findDeploymentConstraint(ctx, gpuDriverVersionConstraint)
+	if !found {
+		return nil
+	}
+	return errors.New(errors.ErrCodeNotFound,
+		fmt.Sprintf("%s %q is set but the host driver version could not be measured (%s)",
+			gpuDriverVersionConstraint, expr, reason))
+}
+
 // emitExtraOrWarn emits structured extra evidence, logging (never failing) on
 // error — a failed stdout write must not flip the check's verdict.
 func emitExtraOrWarn(extra map[string]string) {
@@ -446,12 +478,9 @@ func parseNvidiaSMIDriverVersion(podLogs string) (string, error) {
 		return "", errors.New(errors.ErrCodeNotFound,
 			"nvidia-smi driver version is not a three-component numeric field")
 	}
-	version := podLogs[loc[2]:loc[3]]
-	if version == "" {
-		return "", errors.New(errors.ErrCodeNotFound,
-			"nvidia-smi output has no parseable Driver Version / KMD Version")
-	}
-	return version, nil
+	// The capture is `[0-9]+(?:\.[0-9]+){0,2}` so loc[2]:loc[3] is never empty
+	// once FindStringSubmatchIndex returned a match.
+	return podLogs[loc[2]:loc[3]], nil
 }
 
 // driverVersionFieldTerminated reports whether the character after a captured
@@ -476,7 +505,9 @@ func driverVersionFieldTerminated(podLogs string, end int) bool {
 // the driver version parsed from nvidia-smi logs (issue #1995). No constraint
 // in the recipe is a no-op — the check must not invent a floor. A constraint
 // with an unreadable banner fails closed: a host-driver floor that cannot be
-// measured must not PASS.
+// measured must not PASS. Enumeration paths that would Skip (no GPU nodes,
+// all cordoned, busy) use failClosedIfUnmeasurableGPUDriverFloor for the
+// same contract before per-node verification runs.
 func enforceGPUDriverVersionFloor(ctx *validators.Context, podLogs, nodeName string) error {
 	constraintExpr, found := findDeploymentConstraint(ctx, gpuDriverVersionConstraint)
 	if !found {
@@ -502,9 +533,9 @@ func enforceGPUDriverVersionFloor(ctx *validators.Context, podLogs, nodeName str
 
 	passed, err := parsed.Evaluate(version)
 	if err != nil {
-		return errors.Wrap(errors.ErrCodeInternal,
+		return errors.PropagateOrWrap(err, errors.ErrCodeInternal,
 			fmt.Sprintf("%s constraint evaluation failed on node %s",
-				gpuDriverVersionConstraint, nodeName), err)
+				gpuDriverVersionConstraint, nodeName))
 	}
 
 	fmt.Printf("  %s: host driver %s, constraint %s → %v\n",

validators/deployment/nvidia_smi_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/NVIDIA/aicr/validators/helper"
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -304,6 +305,11 @@ func TestEnforceGPUDriverVersionFloor(t *testing.T) {
 			constraint: ">= 580.95.05",
 			logs:       goodLogs,
 		},
+		{
+			name:       "satisfies floor from KMD Version banner",
+			constraint: ">= 580.95.05",
+			logs:       "NVIDIA-SMI\nKMD Version: 580.95.05\nCUDA UMD Version: 13.0\n" + gpuCheckSuccessMsg,
+		},
 		{
 			// Lexical string compare would fail here ('.100' < '.99'); the
 			// constraint evaluator must compare components numerically.
@@ -329,6 +335,14 @@ func TestEnforceGPUDriverVersionFloor(t *testing.T) {
 			logs:       goodLogs,
 			wantErrSub: "invalid Deployment.gpu-driver.version constraint",
 		},
+		{
+			// Parses as >= with a non-version value; Evaluate returns
+			// ErrCodeInvalidRequest, which must not be recoded Internal.
+			name:       "unparseable floor value preserves InvalidRequest",
+			constraint: ">= not-a-version",
+			logs:       goodLogs,
+			wantErrSub: "[INVALID_REQUEST] cannot parse expected version",
+		},
 	}
 
 	for _, tt := range tests {
@@ -610,6 +624,136 @@ func TestCheckNvidiaSMI_CordonedKeepsSkipEvenWhenDeclared(t *testing.T) {
 	}
 }
 
+// TestCheckNvidiaSMI_DeclaredFloorUnmeasurableFailsClosed proves #1995: a
+// declared Deployment.gpu-driver.version must not ride the non-blocking Skip
+// paths. Busy workloads and all-cordoned nodes are supported Skip reasons
+// only when no floor is configured.
+func TestCheckNvidiaSMI_DeclaredFloorUnmeasurableFailsClosed(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		nodes       []runtime.Object
+		busy        bool
+		wantErrSubs []string
+	}{
+		{
+			name:  "no GPU nodes with floor fails closed",
+			nodes: []runtime.Object{},
+			wantErrSubs: []string{
+				"[NOT_FOUND]",
+				gpuDriverVersionConstraint,
+				"could not be measured",
+				"no GPU nodes found in the cluster",
+			},
+		},
+		{
+			name: "all GPU nodes cordoned with floor fails closed",
+			nodes: []runtime.Object{
+				cordon(gpuNode("cordoned-1", 8, -1)),
+				cordon(gpuNode("cordoned-2", 8, -1)),
+			},
+			wantErrSubs: []string{
+				"[NOT_FOUND]",
+				gpuDriverVersionConstraint,
+				"could not be measured",
+				"all 2 GPU node(s) are cordoned",
+			},
+		},
+		{
+			name: "busy GPU node with floor fails closed",
+			nodes: []runtime.Object{
+				gpuNode("gpu-1", 8, -1),
+			},
+			busy: true,
+			wantErrSubs: []string{
+				"[NOT_FOUND]",
+				gpuDriverVersionConstraint,
+				"could not be measured",
+				"GPU nodes busy with existing workloads",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx := newDeploymentTestContext(t, tt.nodes, nil, nil)
+			withGPUDriverFloor(ctx, ">= 580.95.05")
+			if tt.busy {
+				ctx.Clientset.(*k8sfake.Clientset).PrependReactor("list", "pods",
+					gpuBusyPodsReactor())
+			}
+
+			err := checkNvidiaSMI(ctx)
+			if validators.IsSkip(err) {
+				t.Fatalf("checkNvidiaSMI() = skip (%v), want fail closed", err)
+			}
+			if err == nil {
+				t.Fatal("checkNvidiaSMI() = nil, want a blocking error")
+			}
+			for _, sub := range tt.wantErrSubs {
+				if !strings.Contains(err.Error(), sub) {
+					t.Errorf("checkNvidiaSMI() error = %v, want it to contain %q", err, sub)
+				}
+			}
+		})
+	}
+}
+
+// withGPUDriverFloor installs Deployment.gpu-driver.version on a test context.
+func withGPUDriverFloor(ctx *validators.Context, expr string) {
+	ctx.ValidationInput.Config.Deployment = &validatorv1.ValidationPhase{
+		Constraints: []recipe.Constraint{{
+			Name:  gpuDriverVersionConstraint,
+			Value: expr,
+		}},
+	}
+}
+
+// gpuBusyPodsReactor makes IsNodeGpuBusy report confirmed occupancy: a
+// running pod with a nvidia.com/gpu limit, regardless of field selector
+// (the fake clientset does not honor spec.nodeName).
+func gpuBusyPodsReactor() clienttesting.ReactionFunc {
+	qty := resource.MustParse("1")
+	return func(clienttesting.Action) (bool, runtime.Object, error) {
+		return true, &v1.PodList{Items: []v1.Pod{{
+			ObjectMeta: metav1.ObjectMeta{Name: "gpu-workload", Namespace: "default"},
+			Spec: v1.PodSpec{
+				Containers: []v1.Container{{
+					Name: "work",
+					Resources: v1.ResourceRequirements{
+						Limits: v1.ResourceList{
+							v1.ResourceName(helper.GpuResourceName): qty,
+						},
+					},
+				}},
+			},
+			Status: v1.PodStatus{Phase: v1.PodRunning},
+		}}}, nil
+	}
+}
+
+// TestCheckNvidiaSMI_BusyWithoutFloorStillSkips preserves the pre-#1995 Skip
+// when occupancy is confirmed and no host-driver floor is declared.
+func TestCheckNvidiaSMI_BusyWithoutFloorStillSkips(t *testing.T) {
+	t.Parallel()
+
+	ctx := newDeploymentTestContext(t, []runtime.Object{
+		gpuNode("gpu-1", 8, -1),
+	}, nil, nil)
+	ctx.Clientset.(*k8sfake.Clientset).PrependReactor("list", "pods", gpuBusyPodsReactor())
+
+	err := checkNvidiaSMI(ctx)
+	if !validators.IsSkip(err) {
+		t.Fatalf("checkNvidiaSMI() error = %v, want a skip when no floor is set", err)
+	}
+	if !strings.Contains(err.Error(), "GPU nodes busy with existing workloads") {
+		t.Errorf("checkNvidiaSMI() error = %v, want the busy skip reason", err)
+	}
+}
+
 // TestCheckNvidiaSMI_BusyProbeAllErrorsFailClosed applies the #2122 contract to
 // the busy-probe path: when the GPU busy-probe ERRORS on every schedulable node
 // (nothing CONFIRMED busy), the probe proved no occupancy, so the check must