diff --git a/controllers/object_controls.go b/controllers/object_controls.go index e8f243e51..11e71b61c 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -1651,6 +1651,15 @@ func TransformDevicePlugin(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpe } } + // user-provided env is applied after the CDI defaults and can silently override them, so validate + // the effective strategy here instead of waiting to encounter the failure at the pod workload creation stages. + // NOTE: device list strategy provided via configMaps are out of scope. + if config.CDI.IsEnabled() { + if err := validateCDIDeviceListStrategy(devicePluginMainContainer); err != nil { + return err + } + } + // set hostNetwork for device-plugin if specified applyHostNetworkConfig(&obj.Spec.Template.Spec, config.DevicePlugin.HostNetwork) @@ -5375,3 +5384,17 @@ func clearRuntimeClasses(n ClusterPolicyController, runtimeClasses []nodev1.Runt } return nil } + +// validateCDIDeviceListStrategy ensures the device-plugin's effective DEVICE_LIST_STRATEGY env var value +// includes a CDI strategy. +func validateCDIDeviceListStrategy(container *corev1.Container) error { + value := getContainerEnv(container, DeviceListStrategyEnvName) + for strategy := range strings.SplitSeq(value, ",") { + switch strings.TrimSpace(strategy) { + case "cdi-annotations", "cdi-cri": + return nil + } + } + return fmt.Errorf("%s %q must include \"cdi-annotations\" or \"cdi-cri\" when CDI is enabled", + DeviceListStrategyEnvName, value) +} diff --git a/controllers/transforms_test.go b/controllers/transforms_test.go index 23b99aa3c..2d3c246ed 100644 --- a/controllers/transforms_test.go +++ b/controllers/transforms_test.go @@ -1170,6 +1170,7 @@ func TestTransformDevicePlugin(t *testing.T) { ds Daemonset // Input DaemonSet cpSpec *gpuv1.ClusterPolicySpec // Input configuration expectedDs Daemonset // Expected output DaemonSet + expectedErr string }{ { description: "transform device plugin", @@ -1246,6 +1247,49 @@ func TestTransformDevicePlugin(t *testing.T) { }, }).WithRuntimeClassName("nvidia"), }, + { + description: "cdi enabled, env overrides device list strategy without a cdi strategy", + ds: NewDaemonset(). + WithContainer(corev1.Container{Name: "nvidia-device-plugin"}), + cpSpec: &gpuv1.ClusterPolicySpec{ + DevicePlugin: gpuv1.DevicePluginSpec{ + Repository: "nvcr.io/nvidia/cloud-native", + Image: "nvidia-device-plugin", + Version: "v1.0.0", + Env: []gpuv1.EnvVar{ + {Name: DeviceListStrategyEnvName, Value: "envvar"}, + }, + }, + }, + expectedErr: `DEVICE_LIST_STRATEGY "envvar" must include "cdi-annotations" or "cdi-cri" when CDI is enabled`, + }, + { + description: "cdi disabled, env overrides device list strategy without a cdi strategy", + ds: NewDaemonset(). + WithContainer(corev1.Container{Name: "nvidia-device-plugin"}), + cpSpec: &gpuv1.ClusterPolicySpec{ + DevicePlugin: gpuv1.DevicePluginSpec{ + Repository: "nvcr.io/nvidia/cloud-native", + Image: "nvidia-device-plugin", + Version: "v1.0.0", + Env: []gpuv1.EnvVar{ + {Name: DeviceListStrategyEnvName, Value: "envvar"}, + }, + }, + CDI: gpuv1.CDIConfigSpec{ + Enabled: newBoolPtr(false), + }, + }, + expectedDs: NewDaemonset().WithContainer(corev1.Container{ + Name: "nvidia-device-plugin", + Image: "nvcr.io/nvidia/cloud-native/nvidia-device-plugin:v1.0.0", + ImagePullPolicy: corev1.PullIfNotPresent, + Env: []corev1.EnvVar{ + {Name: "NVIDIA_MIG_MONITOR_DEVICES", Value: "all"}, + {Name: DeviceListStrategyEnvName, Value: "envvar"}, + }, + }).WithRuntimeClassName("nvidia"), + }, } for _, tc := range testCases { @@ -1254,12 +1298,60 @@ func TestTransformDevicePlugin(t *testing.T) { runtime: gpuv1.Containerd, logger: ctrl.Log.WithName("test"), }) + if tc.expectedErr != "" { + require.EqualError(t, err, tc.expectedErr) + return + } require.NoError(t, err) require.EqualValues(t, tc.expectedDs, tc.ds) }) } } +func TestValidateCDIDeviceListStrategy(t *testing.T) { + testCases := map[string]struct { + env []corev1.EnvVar + expectedErr string + }{ + "default cdi strategies": { + env: []corev1.EnvVar{{Name: DeviceListStrategyEnvName, Value: "cdi-annotations,cdi-cri"}}, + }, + "single cdi strategy": { + env: []corev1.EnvVar{{Name: DeviceListStrategyEnvName, Value: "cdi-cri"}}, + }, + "mixed strategies with whitespace": { + env: []corev1.EnvVar{{Name: DeviceListStrategyEnvName, Value: "envvar, cdi-annotations"}}, + }, + "non-cdi strategy": { + env: []corev1.EnvVar{{Name: DeviceListStrategyEnvName, Value: "envvar"}}, + expectedErr: `DEVICE_LIST_STRATEGY "envvar" must include "cdi-annotations" or "cdi-cri" when CDI is enabled`, + }, + "empty strategy": { + env: []corev1.EnvVar{{Name: DeviceListStrategyEnvName, Value: ""}}, + expectedErr: `DEVICE_LIST_STRATEGY "" must include "cdi-annotations" or "cdi-cri" when CDI is enabled`, + }, + "unset strategy": { + expectedErr: `DEVICE_LIST_STRATEGY "" must include "cdi-annotations" or "cdi-cri" when CDI is enabled`, + }, + "unknown cdi-prefixed strategy": { + env: []corev1.EnvVar{{Name: DeviceListStrategyEnvName, Value: "cdi-annotation"}}, + expectedErr: `DEVICE_LIST_STRATEGY "cdi-annotation" must include "cdi-annotations" or "cdi-cri" when CDI is enabled`, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + container := &corev1.Container{Env: tc.env} + err := validateCDIDeviceListStrategy(container) + if tc.expectedErr != "" { + require.EqualError(t, err, tc.expectedErr) + return + } + require.NoError(t, err) + }) + } +} + func TestTransformMPSControlDaemon(t *testing.T) { resources := corev1.ResourceRequirements{ Limits: corev1.ResourceList{