Skip to content

Commit 84ae15d

Browse files
tariq1890tariq1890
authored
Merge pull request #1732 from karthikvetrivel/refactor/update-transformForRuntime-pointer
Refactor transformForRuntime to accept container pointer; add kata-manager test
2 parents 656f069 + 6bccb48 commit 84ae15d

File tree

2 files changed

+68
-30
lines changed

2 files changed

+68
-30
lines changed

‎controllers/object_controls.go‎

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1327,36 +1327,25 @@ func TransformToolkit(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec, n
13271327

13281328
// configure runtime
13291329
runtime := n.runtime.String()
1330-
err = transformForRuntime(obj, config, runtime, mainContainerName)
1330+
err = transformForRuntime(obj, config, runtime, mainContainer)
13311331
if err != nil {
13321332
return fmt.Errorf("error transforming toolkit daemonset : %w", err)
13331333
}
13341334

13351335
return nil
13361336
}
13371337

1338-
func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec, runtime string, containerName string) error {
1339-
var mainContainer *corev1.Container
1340-
for i, ctr := range obj.Spec.Template.Spec.Containers {
1341-
if ctr.Name == containerName {
1342-
mainContainer = &obj.Spec.Template.Spec.Containers[i]
1343-
break
1344-
}
1345-
}
1346-
if mainContainer == nil {
1347-
return fmt.Errorf("failed to find main container %q", containerName)
1348-
}
1349-
1350-
setContainerEnv(mainContainer, "RUNTIME", runtime)
1338+
func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec, runtime string, container *corev1.Container) error {
1339+
setContainerEnv(container, "RUNTIME", runtime)
13511340

13521341
if runtime == gpuv1.Containerd.String() {
13531342
// Set the runtime class name that is to be configured for containerd
1354-
setContainerEnv(mainContainer, "CONTAINERD_RUNTIME_CLASS", getRuntimeClassName(config))
1343+
setContainerEnv(container, "CONTAINERD_RUNTIME_CLASS", getRuntimeClassName(config))
13551344
}
13561345

13571346
if runtime == gpuv1.CRIO.String() {
13581347
// We add the nvidia runtime to the cri-o config by default instead of installing the OCI prestart hook
1359-
setContainerEnv(mainContainer, CRIOConfigModeEnvName, "config")
1348+
setContainerEnv(container, CRIOConfigModeEnvName, "config")
13601349
}
13611350

13621351
// For runtime config files we have top-level configs and drop-in files.
@@ -1367,7 +1356,7 @@ func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec,
13671356
// but should not be updated.
13681357

13691358
// setup mounts for runtime config file
1370-
topLevelConfigFile, dropInConfigFile, err := getRuntimeConfigFiles(mainContainer, runtime)
1359+
topLevelConfigFile, dropInConfigFile, err := getRuntimeConfigFiles(container, runtime)
13711360
if err != nil {
13721361
return fmt.Errorf("error getting path to runtime config file: %w", err)
13731362
}
@@ -1387,12 +1376,12 @@ func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec,
13871376
sourceConfigFileName := path.Base(topLevelConfigFile)
13881377
sourceConfigDir := path.Dir(topLevelConfigFile)
13891378
containerConfigDir := DefaultRuntimeConfigTargetDir
1390-
setContainerEnv(mainContainer, "RUNTIME_CONFIG", containerConfigDir+sourceConfigFileName)
1391-
setContainerEnv(mainContainer, configEnvvarName, containerConfigDir+sourceConfigFileName)
1379+
setContainerEnv(container, "RUNTIME_CONFIG", containerConfigDir+sourceConfigFileName)
1380+
setContainerEnv(container, configEnvvarName, containerConfigDir+sourceConfigFileName)
13921381

13931382
volMountConfigName := fmt.Sprintf("%s-config", runtime)
13941383
volMountConfig := corev1.VolumeMount{Name: volMountConfigName, MountPath: containerConfigDir}
1395-
mainContainer.VolumeMounts = append(mainContainer.VolumeMounts, volMountConfig)
1384+
container.VolumeMounts = append(container.VolumeMounts, volMountConfig)
13961385

13971386
configVol := corev1.Volume{Name: volMountConfigName, VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{Path: sourceConfigDir, Type: newHostPathType(corev1.HostPathDirectoryOrCreate)}}}
13981387
obj.Spec.Template.Spec.Volumes = append(obj.Spec.Template.Spec.Volumes, configVol)
@@ -1405,23 +1394,23 @@ func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec,
14051394
// Note that we probably want to implement drop-in file support in the
14061395
// kata manager in any case -- in which case it will be good to use a
14071396
// similar implementation.
1408-
if dropInConfigFile != "" && containerName != "nvidia-kata-manager" {
1397+
if dropInConfigFile != "" && container.Name != "nvidia-kata-manager" {
14091398
sourceConfigFileName := path.Base(dropInConfigFile)
14101399
sourceConfigDir := path.Dir(dropInConfigFile)
14111400
containerConfigDir := DefaultRuntimeDropInConfigTargetDir
1412-
setContainerEnv(mainContainer, "RUNTIME_DROP_IN_CONFIG", containerConfigDir+sourceConfigFileName)
1413-
setContainerEnv(mainContainer, "RUNTIME_DROP_IN_CONFIG_HOST_PATH", dropInConfigFile)
1401+
setContainerEnv(container, "RUNTIME_DROP_IN_CONFIG", containerConfigDir+sourceConfigFileName)
1402+
setContainerEnv(container, "RUNTIME_DROP_IN_CONFIG_HOST_PATH", dropInConfigFile)
14141403

14151404
volMountConfigName := fmt.Sprintf("%s-drop-in-config", runtime)
14161405
volMountConfig := corev1.VolumeMount{Name: volMountConfigName, MountPath: containerConfigDir}
1417-
mainContainer.VolumeMounts = append(mainContainer.VolumeMounts, volMountConfig)
1406+
container.VolumeMounts = append(container.VolumeMounts, volMountConfig)
14181407

14191408
configVol := corev1.Volume{Name: volMountConfigName, VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{Path: sourceConfigDir, Type: newHostPathType(corev1.HostPathDirectoryOrCreate)}}}
14201409
obj.Spec.Template.Spec.Volumes = append(obj.Spec.Template.Spec.Volumes, configVol)
14211410
}
14221411

14231412
// setup mounts for runtime socket file
1424-
runtimeSocketFile, err := getRuntimeSocketFile(mainContainer, runtime)
1413+
runtimeSocketFile, err := getRuntimeSocketFile(container, runtime)
14251414
if err != nil {
14261415
return fmt.Errorf("error getting path to runtime socket: %w", err)
14271416
}
@@ -1434,12 +1423,12 @@ func transformForRuntime(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec,
14341423
} else if runtime == gpuv1.Docker.String() {
14351424
socketEnvvarName = "DOCKER_SOCKET"
14361425
}
1437-
setContainerEnv(mainContainer, "RUNTIME_SOCKET", DefaultRuntimeSocketTargetDir+sourceSocketFileName)
1438-
setContainerEnv(mainContainer, socketEnvvarName, DefaultRuntimeSocketTargetDir+sourceSocketFileName)
1426+
setContainerEnv(container, "RUNTIME_SOCKET", DefaultRuntimeSocketTargetDir+sourceSocketFileName)
1427+
setContainerEnv(container, socketEnvvarName, DefaultRuntimeSocketTargetDir+sourceSocketFileName)
14391428

14401429
volMountSocketName := fmt.Sprintf("%s-socket", runtime)
14411430
volMountSocket := corev1.VolumeMount{Name: volMountSocketName, MountPath: DefaultRuntimeSocketTargetDir}
1442-
mainContainer.VolumeMounts = append(mainContainer.VolumeMounts, volMountSocket)
1431+
container.VolumeMounts = append(container.VolumeMounts, volMountSocket)
14431432

14441433
socketVol := corev1.Volume{Name: volMountSocketName, VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{Path: path.Dir(runtimeSocketFile)}}}
14451434
obj.Spec.Template.Spec.Volumes = append(obj.Spec.Template.Spec.Volumes, socketVol)
@@ -1984,7 +1973,8 @@ func TransformKataManager(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec
19841973
// mount containerd config and socket
19851974
// setup mounts for runtime config file
19861975
runtime := n.runtime.String()
1987-
err = transformForRuntime(obj, config, runtime, "nvidia-kata-manager")
1976+
// kata manager is the only container in this daemonset
1977+
err = transformForRuntime(obj, config, runtime, &obj.Spec.Template.Spec.Containers[0])
19881978
if err != nil {
19891979
return fmt.Errorf("error transforming kata-manager daemonset : %w", err)
19901980
}

‎controllers/transforms_test.go‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,60 @@ func TestTransformForRuntime(t *testing.T) {
376376
},
377377
}),
378378
},
379+
// Cover the kata-manager naming case
380+
{
381+
description: "containerd skips drop-in for kata manager",
382+
runtime: gpuv1.Containerd,
383+
input: NewDaemonset().
384+
WithContainer(corev1.Container{Name: "nvidia-kata-manager"}),
385+
expectedOutput: NewDaemonset().
386+
WithHostPathVolume("containerd-config", filepath.Dir(DefaultContainerdConfigFile), newHostPathType(corev1.HostPathDirectoryOrCreate)).
387+
WithHostPathVolume("containerd-socket", filepath.Dir(DefaultContainerdSocketFile), nil).
388+
WithContainer(corev1.Container{
389+
Name: "nvidia-kata-manager",
390+
Env: []corev1.EnvVar{
391+
{Name: "RUNTIME", Value: gpuv1.Containerd.String()},
392+
{Name: "CONTAINERD_RUNTIME_CLASS", Value: DefaultRuntimeClass},
393+
{Name: "RUNTIME_CONFIG", Value: filepath.Join(DefaultRuntimeConfigTargetDir, filepath.Base(DefaultContainerdConfigFile))},
394+
{Name: "CONTAINERD_CONFIG", Value: filepath.Join(DefaultRuntimeConfigTargetDir, filepath.Base(DefaultContainerdConfigFile))},
395+
{Name: "RUNTIME_SOCKET", Value: filepath.Join(DefaultRuntimeSocketTargetDir, filepath.Base(DefaultContainerdSocketFile))},
396+
{Name: "CONTAINERD_SOCKET", Value: filepath.Join(DefaultRuntimeSocketTargetDir, filepath.Base(DefaultContainerdSocketFile))},
397+
},
398+
VolumeMounts: []corev1.VolumeMount{
399+
{Name: "containerd-config", MountPath: DefaultRuntimeConfigTargetDir},
400+
{Name: "containerd-socket", MountPath: DefaultRuntimeSocketTargetDir},
401+
},
402+
}),
403+
},
404+
{
405+
description: "docker",
406+
runtime: gpuv1.Docker,
407+
input: NewDaemonset().WithContainer(corev1.Container{Name: "test-ctr"}),
408+
expectedOutput: NewDaemonset().
409+
WithHostPathVolume("docker-config", filepath.Dir(DefaultDockerConfigFile), newHostPathType(corev1.HostPathDirectoryOrCreate)).
410+
WithHostPathVolume("docker-socket", filepath.Dir(DefaultDockerSocketFile), nil).
411+
WithContainer(corev1.Container{
412+
Name: "test-ctr",
413+
Env: []corev1.EnvVar{
414+
{Name: "RUNTIME", Value: gpuv1.Docker.String()},
415+
{Name: "RUNTIME_CONFIG", Value: filepath.Join(DefaultRuntimeConfigTargetDir, filepath.Base(DefaultDockerConfigFile))},
416+
{Name: "DOCKER_CONFIG", Value: filepath.Join(DefaultRuntimeConfigTargetDir, filepath.Base(DefaultDockerConfigFile))},
417+
{Name: "RUNTIME_SOCKET", Value: filepath.Join(DefaultRuntimeSocketTargetDir, filepath.Base(DefaultDockerSocketFile))},
418+
{Name: "DOCKER_SOCKET", Value: filepath.Join(DefaultRuntimeSocketTargetDir, filepath.Base(DefaultDockerSocketFile))},
419+
},
420+
VolumeMounts: []corev1.VolumeMount{
421+
{Name: "docker-config", MountPath: DefaultRuntimeConfigTargetDir},
422+
{Name: "docker-socket", MountPath: DefaultRuntimeSocketTargetDir},
423+
},
424+
}),
425+
},
379426
}
380427

381428
cp := &gpuv1.ClusterPolicySpec{Operator: gpuv1.OperatorSpec{RuntimeClass: DefaultRuntimeClass}}
382429
for _, tc := range testCases {
383430
t.Run(tc.description, func(t *testing.T) {
384-
err := transformForRuntime(tc.input.DaemonSet, cp, tc.runtime.String(), "test-ctr")
431+
// pass pointer to the target container
432+
err := transformForRuntime(tc.input.DaemonSet, cp, tc.runtime.String(), &tc.input.Spec.Template.Spec.Containers[0])
385433
require.NoError(t, err)
386434
require.EqualValues(t, tc.expectedOutput, tc.input)
387435
})

0 commit comments

Comments
 (0)