Merge pull request #2514 from frezbo/fix/drop-containerd-config-mounts-when-nri-enabled

fix: drop containerd config mounts when NRI is enabled

Author: tariq1890

controllers/object_controls.go

@@ -1379,11 +1379,50 @@ func TransformToolkit(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec, n
 func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec, runtime string, container *corev1.Container) error {
 	setContainerEnv(container, "RUNTIME", runtime)
 
-	if runtime == gpuv1.Containerd.String() {
-		// Set the runtime class name that is to be configured for containerd
-		setContainerEnv(container, "CONTAINERD_RUNTIME_CLASS", getRuntimeClassName(config))
+	// In NRI plugin mode the toolkit installer runs the noop runtime configurer
+	// (see nvidia-ctk-installer's NewConfigurer): it does not read or modify the
+	// container runtime configuration and does not restart the runtime. Mounting the
+	// runtime config files and socket is therefore unnecessary in this mode, and the
+	// hostPath mounts (e.g. /etc/containerd/conf.d with DirectoryOrCreate) break on
+	// read-only hosts such as Talos. Only the NRI socket mount (below) is required.
+	if config.CDI.IsNRIPluginEnabled() {
+		transformNRISocketMounts(obj, container)
+	} else {
+		if runtime == gpuv1.Containerd.String() {
+			// Set the runtime class name that is to be configured for containerd
+			setContainerEnv(container, "CONTAINERD_RUNTIME_CLASS", getRuntimeClassName(config))
+		}
+
+		if err := transformRuntimeConfigAndSocketMounts(obj, runtime, container); err != nil {
+			return err
+		}
 	}
 
+	return nil
+}
+
+// transformNRISocketMounts adds the required NRI socket environment variable and
+// sets up the NRI socket mount
+func transformNRISocketMounts(obj *appsv1.DaemonSet, container *corev1.Container) {
+	// setup mounts for the runtime NRI socket file
+	nriSocketFile := getContainerEnv(container, "NRI_SOCKET")
+	if nriSocketFile == "" {
+		nriSocketFile = DefaultRuntimeNRISocketFile
+	}
+	setContainerEnv(container, "NRI_SOCKET", path.Join(DefaultRuntimeNRISocketTargetDir, path.Base(nriSocketFile)))
+
+	nriVolMountSocketName := "nri-socket"
+	nriVolMountSocket := corev1.VolumeMount{Name: nriVolMountSocketName, MountPath: DefaultRuntimeNRISocketTargetDir}
+	container.VolumeMounts = append(container.VolumeMounts, nriVolMountSocket)
+
+	nriSocketVol := corev1.Volume{Name: nriVolMountSocketName, VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{Path: path.Dir(nriSocketFile), Type: ptr.To(corev1.HostPathDirectoryOrCreate)}}}
+	obj.Spec.Template.Spec.Volumes = append(obj.Spec.Template.Spec.Volumes, nriSocketVol)
+}
+
+// transformRuntimeConfigAndSocketMounts configures the toolkit container with the
+// mounts and environment required for the toolkit installer to update the container
+// runtime configuration (top-level config, drop-in config, and runtime socket).
+func transformRuntimeConfigAndSocketMounts(obj *appsv1.DaemonSet, runtime string, container *corev1.Container) error {
 	// For runtime config files we have top-level configs and drop-in files.
 	// These are supported as follows:
 	//   * Docker only supports top-level config files.
@@ -1489,22 +1528,6 @@ func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec,
 		obj.Spec.Template.Spec.Volumes = append(obj.Spec.Template.Spec.Volumes, socketVol)
 	}
 
-	if config.CDI.IsNRIPluginEnabled() {
-		// setup mounts for the runtime NRI socket file
-		nriSocketFile := getContainerEnv(container, "NRI_SOCKET")
-		if nriSocketFile == "" {
-			nriSocketFile = DefaultRuntimeNRISocketFile
-		}
-		setContainerEnv(container, "NRI_SOCKET", DefaultRuntimeNRISocketTargetDir+path.Base(nriSocketFile))
-
-		nriVolMountSocketName := "nri-socket"
-		nriVolMountSocket := corev1.VolumeMount{Name: nriVolMountSocketName, MountPath: DefaultRuntimeNRISocketTargetDir}
-		container.VolumeMounts = append(container.VolumeMounts, nriVolMountSocket)
-
-		nriSocketVol := corev1.Volume{Name: nriVolMountSocketName, VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{Path: path.Dir(nriSocketFile), Type: ptr.To(corev1.HostPathDirectoryOrCreate)}}}
-		obj.Spec.Template.Spec.Volumes = append(obj.Spec.Template.Spec.Volumes, nriSocketVol)
-	}
-
 	return nil
 }
 

controllers/transforms_test.go

@@ -17,6 +17,7 @@
 package controllers
 
 import (
+	"path"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -1073,6 +1074,65 @@ func TestTransformToolkit(t *testing.T) {
 				WithHostPathVolume("crio-config", "/etc/crio", ptr.To(corev1.HostPathDirectoryOrCreate)).
 				WithHostPathVolume("crio-drop-in-config", "/etc/crio/crio.conf.d", ptr.To(corev1.HostPathDirectoryOrCreate)),
 		},
+		{
+			description: "transform with NRI enabled has just the NRI socket mount",
+			ds: NewDaemonset().
+				WithContainer(corev1.Container{Name: "nvidia-container-toolkit-ctr"}),
+			runtime: gpuv1.Containerd,
+			cpSpec: &gpuv1.ClusterPolicySpec{
+				CDI: gpuv1.CDIConfigSpec{
+					Enabled:          new(true),
+					NRIPluginEnabled: new(true),
+				},
+				Toolkit: gpuv1.ToolkitSpec{
+					Repository:       "nvcr.io/nvidia/cloud-native",
+					Image:            "nvidia-container-toolkit",
+					Version:          "v1.0.0",
+					ImagePullPolicy:  "IfNotPresent",
+					ImagePullSecrets: []string{"pull-secret"},
+					Resources: &gpuv1.ResourceRequirements{
+						Limits: corev1.ResourceList{
+							corev1.ResourceCPU:    resource.MustParse("100m"),
+							corev1.ResourceMemory: resource.MustParse("100Mi"),
+						},
+						Requests: corev1.ResourceList{
+							corev1.ResourceCPU:    resource.MustParse("50m"),
+							corev1.ResourceMemory: resource.MustParse("50Mi"),
+						},
+					},
+				},
+			},
+			expectedDs: NewDaemonset().
+				WithHostPathVolume("nri-socket", filepath.Dir(DefaultRuntimeNRISocketFile), new(corev1.HostPathDirectoryOrCreate)).
+				WithContainer(corev1.Container{
+					Name:            "nvidia-container-toolkit-ctr",
+					Image:           "nvcr.io/nvidia/cloud-native/nvidia-container-toolkit:v1.0.0",
+					ImagePullPolicy: corev1.PullIfNotPresent,
+					Resources: corev1.ResourceRequirements{
+						Limits: corev1.ResourceList{
+							corev1.ResourceCPU:    resource.MustParse("100m"),
+							corev1.ResourceMemory: resource.MustParse("100Mi"),
+						},
+						Requests: corev1.ResourceList{
+							corev1.ResourceCPU:    resource.MustParse("50m"),
+							corev1.ResourceMemory: resource.MustParse("50Mi"),
+						},
+					},
+					Env: []corev1.EnvVar{
+						{Name: CDIEnabledEnvName, Value: "true"},
+						{Name: NvidiaRuntimeSetAsDefaultEnvName, Value: "false"},
+						{Name: NvidiaCtrRuntimeModeEnvName, Value: "cdi"},
+						{Name: CRIOConfigModeEnvName, Value: "config"},
+						{Name: "ENABLE_NRI_PLUGIN", Value: "true"},
+						{Name: "RUNTIME", Value: "containerd"},
+						{Name: "NRI_SOCKET", Value: path.Join(DefaultRuntimeNRISocketTargetDir, path.Base(DefaultRuntimeNRISocketFile))},
+					},
+					VolumeMounts: []corev1.VolumeMount{
+						{Name: "nri-socket", MountPath: DefaultRuntimeNRISocketTargetDir},
+					},
+				}).
+				WithPullSecret("pull-secret"),
+		},
 	}
 
 	for _, tc := range testCases {