Merge pull request #598 from jgehrcke/jp/test-strict-decode-on-prepare

Test for strict opaque config-decoding in prepare path

Author: jgehrcke

cmd/compute-domain-kubelet-plugin/driver.go

@@ -247,7 +247,11 @@ func (d *driver) nodePrepareResource(ctx context.Context, claim *resourceapi.Res
 		res := kubeletplugin.PrepareResult{
 			Err: fmt.Errorf("error preparing devices for claim %v: %w", claim.UID, err),
 		}
-		return isPermanentError(err), res
+		if isPermanentError(err) {
+			klog.V(6).Infof("Permanent error preparing devices for claim %v: %v", claim.UID, err)
+			return true, res
+		}
+		return false, res
 	}
 
 	klog.Infof("Returning newly prepared devices for claim '%v': %v", claim.UID, devs)

tests/bats/Makefile

@@ -90,8 +90,7 @@ tests: image
 		$(BATS_IMAGE) \
 		-c "cd /cwd; \
 			echo 'Running k8s cluster cleanup (invasive)... '; \
-			bash tests/bats/cleanup-from-previous-run.sh &> $${_RUNDIR}/cleanup.outerr || \
-				(echo 'Cleanup failed:'; cat $${_RUNDIR}/cleanup.outerr);  \
+			bash tests/bats/cleanup-from-previous-run.sh 2>&1 | tee -a $${_RUNDIR}/cleanup.outerr; \
 			TMPDIR=/cwd/$${_RUNDIR} bats \
 			--print-output-on-failure \
 			--no-tempdir-cleanup \

tests/bats/cleanup-from-previous-run.sh

@@ -36,16 +36,18 @@ kubectl apply -f "${CRD_URL}"
 # interrupted run. TODO: try to affect all-at-once, maybe with a special label.
 # Note: the following commands are OK to fail -- the `errexit` shell option is
 # deliberately not set here.
-timeout -v 5 kubectl delete -f demo/specs/imex/channel-injection.yaml
-timeout -v 5 kubectl delete -f demo/specs/imex/channel-injection-all.yaml
-timeout -v 5 kubectl delete jobs nickelpie-test
-timeout -v 5 kubectl delete computedomain nickelpie-test-compute-domain
-timeout -v 5 kubectl delete -f demo/specs/imex/nvbandwidth-test-job-1.yaml
+timeout -v 5 kubectl delete -f demo/specs/imex/channel-injection.yaml 2> /dev/null
+timeout -v 5 kubectl delete -f demo/specs/imex/channel-injection-all.yaml 2> /dev/null
+timeout -v 5 kubectl delete jobs nickelpie-test 2> /dev/null
+timeout -v 5 kubectl delete computedomain nickelpie-test-compute-domain 2> /dev/null
+timeout -v 5 kubectl delete -f demo/specs/imex/nvbandwidth-test-job-1.yaml 2> /dev/null
+timeout -v 5 kubectl delete pods -l env=batssuite 2> /dev/null
+timeout -v 2 kubectl delete resourceclaim batssuite-rc-bad-opaque-config --force 2> /dev/null
 
 # Delete any previous remainder of `clean-state-dirs-all-nodes.sh` invocation.
-kubectl delete pods privpod-rm-plugindirs
+kubectl delete pods privpod-rm-plugindirs 2> /dev/null
 
-helm uninstall nvidia-dra-driver-gpu-batssuite -n nvidia-dra-driver-gpu
+timeout -v 5 helm uninstall nvidia-dra-driver-gpu-batssuite -n nvidia-dra-driver-gpu
 
 kubectl wait \
     --for=delete pods -A \
@@ -58,7 +60,8 @@ kubectl wait \
 # properly.
 timeout -v 10 kubectl delete crds computedomains.resource.nvidia.com || echo "CRD deletion failed"
 
+# Remove kubelet plugin state directories from all nodes (critical part of
+# cleanup, fail hard if this does not succeed).
 set -e
-# Remove kubelet plugin state directories from all nodes.
 bash tests/bats/clean-state-dirs-all-nodes.sh
 set +x

tests/bats/helpers.sh

@@ -0,0 +1,40 @@
+#!/bin/bash
+#
+#  SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+#  SPDX-License-Identifier: Apache-2.0
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+# Events accumulate over time, so for certainty it's best to use a unique pod
+# name. Right now, this inspects an entire line which includes REASON, MESSAGE,
+# and OBJECT, so choose the needle (grepped for) precisely enough.
+# Example: wait_for_pod_event pod/testpod-ls09x FailedPrepareDynamicResources 60
+wait_for_pod_event() {
+  # Expect this to have the pod/ prefix
+  local POD_NAME="$1"
+  local REASON="$2"
+  local TIMEOUT="$3"
+
+  local START=$SECONDS
+  while true; do
+    if kubectl events --for "${POD_NAME}" | grep -q "${REASON}"; then
+      echo "Event detected: ${REASON} (for ${POD_NAME})"
+      return 0
+    fi
+    if (( SECONDS - START > TIMEOUT )); then
+      echo "Timeout (${TIMEOUT} s) waiting for '${REASON}' in events for ${POD_NAME}"
+      return 1
+    fi
+    sleep 2
+  done
+}

tests/bats/specs/rc-opaque-cfg-unknown-field.yaml

@@ -0,0 +1,36 @@
+apiVersion: resource.k8s.io/v1beta1
+kind: ResourceClaim
+metadata:
+  name: batssuite-rc-bad-opaque-config
+spec:
+  devices:
+    requests:
+    - deviceClassName: compute-domain-default-channel.nvidia.com
+      name: chan
+    config:
+    - opaque:
+        driver: compute-domain.nvidia.com
+        parameters:
+          apiVersion: resource.nvidia.com/v1beta1
+          domainID: not-needed-for-test
+          kind: ComputeDomainChannelConfig
+          unexpectedField: foo
+      requests:
+      - chan
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  generateName: batssuite-pod-boc-
+  labels:
+    env: batssuite
+spec:
+  containers:
+  - name: app
+    image: busybox
+    resources:
+      claims:
+      - name: batssuite-claim-boc
+  resourceClaims:
+  - name: batssuite-claim-boc
+    resourceClaimName: batssuite-rc-bad-opaque-config

tests/bats/tests.bats

@@ -4,6 +4,7 @@ setup() {
   load '/bats-libraries/bats-support/load.bash'
   load '/bats-libraries/bats-assert/load.bash'
   load '/bats-libraries/bats-file/load.bash'
+  load 'helpers.sh'
 }
 
 # Currently, the tests defined in this file deliberately depend on each other
@@ -162,6 +163,44 @@ apply_check_delete_workload_imex_chan_inject() {
   kubectl delete -f demo/specs/imex/channel-injection-all.yaml
 }
 
+@test "NodePrepareResources: catch unknown field in opaque cfg in ResourceClaim" {
+  local SPEC="tests/bats/specs/rc-opaque-cfg-unknown-field.yaml"
+
+  # Create pod with random name suffix.
+  # Store ref of the form `pod/batssuite-pod-boc-brs2l`.
+  local POD
+  POD=$(kubectl create -f "${SPEC}" | grep pod | awk '{print $1;}')
+
+  # Confirm ContainerCreating state (no failure yet though).
+  kubectl wait \
+    --for=jsonpath='{.status.containerStatuses[0].state.waiting.reason}'=ContainerCreating \
+    --timeout=10s \
+    "${POD}"
+
+  # Rather quickly, we expect an event with reason
+  # `FailedPrepareDynamicResources`. That's not typically the method users
+  # discover the error.
+  wait_for_pod_event "${POD}" FailedPrepareDynamicResources 10
+
+  # This is how users probably see this error first.
+  kubectl describe "${POD}" | grep FailedPrepareDynamicResources | \
+    grep "error preparing devices"  | \
+    grep 'strict decoding error: unknown field "unexpectedField"'
+
+  # Confirm that precise root cause can also be inferred from
+  # CD kubelet plugin logs.
+  kubectl logs \
+    -l nvidia-dra-driver-gpu-component=kubelet-plugin \
+    -n nvidia-dra-driver-gpu \
+    --prefix --tail=-1 | \
+      grep 'Permanent error' | \
+      grep 'strict decoding error: unknown field "unexpectedField"'
+
+  # Clean up.
+  kubectl delete "${POD}"
+  kubectl delete resourceclaim batssuite-rc-bad-opaque-config
+}
+
 @test "nickelpie (NCCL send/recv/broadcast, 2 pods, 2 nodes, small payload)" {
   # Do not run in checkout dir (to not pollute that).
   cd "${BATS_TEST_TMPDIR}"
@@ -188,7 +227,7 @@ apply_check_delete_workload_imex_chan_inject() {
   echo "${output}" | grep -E '^.*SUM multinode_device_to_device_memcpy_read_ce [0-9]+\.[0-9]+.*$'
 }
 
-@test "downgrade: current -> ${TEST_CHART_LASTSTABLE_REPO}/${TEST_CHART_LASTSTABLE_VERSION}" {
+@test "downgrade: current-dev -> last-stable" {
   # Stage 1: apply workload, but do not delete.
   kubectl apply -f demo/specs/imex/channel-injection.yaml
   kubectl wait --for=condition=READY pods imex-channel-injection --timeout=60s