Repository: intel/intel-device-plugins-for-kubernetes (go)

Issue:
InitImage cannot be updated to DsaDevicePlugin
`UpdateDaemonSet` in dsa controller does not react to `InitImage` changes in the CRD.

Pull request:
operator: dsa: Handle initcontainer and ConfigMap updates
Closes #729

Patch:
diff --git a/cmd/sgx_plugin/README.md b/cmd/sgx_plugin/README.md
index 9d198a4b..97358ab9 100644
--- a/cmd/sgx_plugin/README.md
+++ b/cmd/sgx_plugin/README.md
@@ -44,8 +44,8 @@ The SGX plugin can take a number of command line arguments, summarised in the fo
 
 | Flag | Argument | Meaning |
 |:---- |:-------- |:------- |
-| -enclave-limit | int | the number of containers per worker node allowed to use `/dev/sgx_enclave` device node (default: `20`) |
-| -provision-limit | int | the number of containers per worker node allowed to use `/dev/sgx_provision` device node (default: `20`) |
+| -enclave-limit | int | the number of containers per node allowed to use `/dev/sgx/enclave` (default: `20`) |
+| -provision-limit | int | the number of containers per node allowed to use `/dev/sgx/provision` (default: `20`) |
 
 The plugin also accepts a number of other arguments related to logging. Please use the `-h` option to see
 the complete list of logging related options.
diff --git a/pkg/controllers/dsa/controller.go b/pkg/controllers/dsa/controller.go
index 6638672a..9483fb72 100644
--- a/pkg/controllers/dsa/controller.go
+++ b/pkg/controllers/dsa/controller.go
@@ -35,8 +35,9 @@ import (
 )
 
 const (
-	ownerKey = ".metadata.controller.dsa"
-	amd64    = "amd64"
+	ownerKey         = ".metadata.controller.dsa"
+	amd64            = "amd64"
+	inicontainerName = "intel-idxd-config-initcontainer"
 )
 
 // +kubebuilder:rbac:groups=deviceplugin.intel.com,resources=dsadeviceplugins,verbs=get;list;watch;create;update;patch;delete
@@ -76,33 +77,83 @@ func (c *controller) GetTotalObjectCount(ctx context.Context, clnt client.Client
 	return len(list.Items), nil
 }
 
-func setInitContainer(spec *v1.PodSpec, imageName string) {
+func removeInitContainer(ds *apps.DaemonSet, dp *devicepluginv1.DsaDevicePlugin) {
+	newInitContainers := []v1.Container{}
+	for _, container := range ds.Spec.Template.Spec.InitContainers {
+		if container.Name == inicontainerName {
+			continue
+		}
+		newInitContainers = append(newInitContainers, container)
+	}
+	ds.Spec.Template.Spec.InitContainers = newInitContainers
+
+	newVolumes := []v1.Volume{}
+	for _, volume := range ds.Spec.Template.Spec.Volumes {
+		if volume.Name == "intel-dsa-config-volume" || volume.Name == "sys-devices" {
+			continue
+		}
+		newVolumes = append(newVolumes, volume)
+	}
+	ds.Spec.Template.Spec.Volumes = newVolumes
+}
+
+func addInitContainer(ds *apps.DaemonSet, dp *devicepluginv1.DsaDevicePlugin) {
 	yes := true
-	spec.InitContainers = []v1.Container{
-		{
-			Image:           imageName,
-			ImagePullPolicy: "IfNotPresent",
-			Name:            "intel-idxd-config-initcontainer",
-			Env: []v1.EnvVar{
-				{
-					Name: "NODE_NAME",
-					ValueFrom: &v1.EnvVarSource{
-						FieldRef: &v1.ObjectFieldSelector{
-							FieldPath: "spec.nodeName",
-						},
+	ds.Spec.Template.Spec.InitContainers = append(ds.Spec.Template.Spec.InitContainers, v1.Container{
+		Image:           dp.Spec.InitImage,
+		ImagePullPolicy: "IfNotPresent",
+		Name:            inicontainerName,
+		Env: []v1.EnvVar{
+			{
+				Name: "NODE_NAME",
+				ValueFrom: &v1.EnvVarSource{
+					FieldRef: &v1.ObjectFieldSelector{
+						FieldPath: "spec.nodeName",
 					},
 				},
 			},
-			SecurityContext: &v1.SecurityContext{
-				Privileged: &yes,
+			{
+				Name:  "IDXD_DEVICE_TYPE",
+				Value: "dsa",
 			},
-			VolumeMounts: []v1.VolumeMount{
-				{
-					Name:      "sys-devices",
-					MountPath: "/sys/devices",
-				},
+		},
+		SecurityContext: &v1.SecurityContext{
+			Privileged: &yes,
+		},
+		VolumeMounts: []v1.VolumeMount{
+			{
+				Name:      "sys-devices",
+				MountPath: "/sys/devices",
 			},
-		}}
+		},
+	})
+	ds.Spec.Template.Spec.Volumes = append(ds.Spec.Template.Spec.Volumes, v1.Volume{
+		Name: "sys-devices",
+		VolumeSource: v1.VolumeSource{
+			HostPath: &v1.HostPathVolumeSource{
+				Path: "/sys/devices",
+			},
+		},
+	})
+
+	if dp.Spec.ProvisioningConfig != "" {
+		ds.Spec.Template.Spec.Volumes = append(ds.Spec.Template.Spec.Volumes, v1.Volume{
+			Name: "intel-dsa-config-volume",
+			VolumeSource: v1.VolumeSource{
+				ConfigMap: &v1.ConfigMapVolumeSource{
+					LocalObjectReference: v1.LocalObjectReference{Name: dp.Spec.ProvisioningConfig}},
+			},
+		})
+
+		for i, initcontainer := range ds.Spec.Template.Spec.InitContainers {
+			if initcontainer.Name == inicontainerName {
+				ds.Spec.Template.Spec.InitContainers[i].VolumeMounts = append(ds.Spec.Template.Spec.InitContainers[i].VolumeMounts, v1.VolumeMount{
+					Name:      "intel-dsa-config-volume",
+					MountPath: "/idxd-init/conf",
+				})
+			}
+		}
+	}
 }
 
 func (c *controller) NewDaemonSet(rawObj client.Object) *apps.DaemonSet {
@@ -126,40 +177,37 @@ func (c *controller) NewDaemonSet(rawObj client.Object) *apps.DaemonSet {
 	daemonSet.Spec.Template.Spec.Containers[0].Args = getPodArgs(devicePlugin)
 	daemonSet.Spec.Template.Spec.Containers[0].Image = devicePlugin.Spec.Image
 
-	// add the optional init container
 	if devicePlugin.Spec.InitImage != "" {
-		setInitContainer(&daemonSet.Spec.Template.Spec, devicePlugin.Spec.InitImage)
+		addInitContainer(daemonSet, devicePlugin)
+	}
 
-		daemonSet.Spec.Template.Spec.Volumes = append(daemonSet.Spec.Template.Spec.Volumes, v1.Volume{
-			Name: "sys-devices",
-			VolumeSource: v1.VolumeSource{
-				HostPath: &v1.HostPathVolumeSource{
-					Path: "/sys/devices",
-				},
-			},
-		})
+	return daemonSet
+}
 
-		if devicePlugin.Spec.ProvisioningConfig != "" {
-			daemonSet.Spec.Template.Spec.Volumes = append(daemonSet.Spec.Template.Spec.Volumes, v1.Volume{
-				Name: "intel-dsa-config-volume",
-				VolumeSource: v1.VolumeSource{
-					ConfigMap: &v1.ConfigMapVolumeSource{
-						LocalObjectReference: v1.LocalObjectReference{Name: devicePlugin.Spec.ProvisioningConfig}},
-				},
-			})
-
-			for i, initcontainer := range daemonSet.Spec.Template.Spec.InitContainers {
-				if initcontainer.Name == "intel-idxd-config-initcontainer" {
-					daemonSet.Spec.Template.Spec.InitContainers[i].VolumeMounts = append(daemonSet.Spec.Template.Spec.InitContainers[i].VolumeMounts, v1.VolumeMount{
-						Name:      "intel-dsa-config-volume",
-						MountPath: "/idxd-init/conf",
-					})
-				}
-			}
+func provisioningUpdate(ds *apps.DaemonSet, dp *devicepluginv1.DsaDevicePlugin) bool {
+	update := false
+	found := false
+
+	for _, container := range ds.Spec.Template.Spec.InitContainers {
+		if container.Name == inicontainerName && container.Image != dp.Spec.InitImage {
+			found = true
+			update = true
+			break
 		}
 	}
 
-	return daemonSet
+	for _, volume := range ds.Spec.Template.Spec.Volumes {
+		if volume.Name == "intel-dsa-config-volume" && volume.ConfigMap.Name != dp.Spec.ProvisioningConfig {
+			update = true
+			break
+		}
+	}
+
+	if !found && dp.Spec.InitImage != "" {
+		update = true
+	}
+
+	return update
 }
 
 func (c *controller) UpdateDaemonSet(rawObj client.Object, ds *apps.DaemonSet) (updated bool) {
@@ -170,6 +218,14 @@ func (c *controller) UpdateDaemonSet(rawObj client.Object, ds *apps.DaemonSet) (
 		updated = true
 	}
 
+	if provisioningUpdate(ds, dp) {
+		removeInitContainer(ds, dp)
+		if dp.Spec.InitImage != "" {
+			addInitContainer(ds, dp)
+		}
+		updated = true
+	}
+
 	if dp.Spec.NodeSelector == nil {
 		dp.Spec.NodeSelector = map[string]string{"kubernetes.io/arch": amd64}
 	} else {


Test patch:
diff --git a/pkg/controllers/dsa/controller_test.go b/pkg/controllers/dsa/controller_test.go
index 6bfc1c80..a33920f5 100644
--- a/pkg/controllers/dsa/controller_test.go
+++ b/pkg/controllers/dsa/controller_test.go
@@ -156,35 +156,7 @@ func (c *controller) newDaemonSetExpected(rawObj client.Object) *apps.DaemonSet
 
 	// add the optional init container
 	if devicePlugin.Spec.InitImage != "" {
-		setInitContainer(&daemonSet.Spec.Template.Spec, devicePlugin.Spec.InitImage)
-
-		daemonSet.Spec.Template.Spec.Volumes = append(daemonSet.Spec.Template.Spec.Volumes, v1.Volume{
-			Name: "sys-devices",
-			VolumeSource: v1.VolumeSource{
-				HostPath: &v1.HostPathVolumeSource{
-					Path: "/sys/devices",
-				},
-			},
-		})
-
-		if devicePlugin.Spec.ProvisioningConfig != "" {
-			daemonSet.Spec.Template.Spec.Volumes = append(daemonSet.Spec.Template.Spec.Volumes, v1.Volume{
-				Name: "intel-dsa-config-volume",
-				VolumeSource: v1.VolumeSource{
-					ConfigMap: &v1.ConfigMapVolumeSource{
-						LocalObjectReference: v1.LocalObjectReference{Name: devicePlugin.Spec.ProvisioningConfig}},
-				},
-			})
-
-			for i, initcontainer := range daemonSet.Spec.Template.Spec.InitContainers {
-				if initcontainer.Name == "intel-idxd-config-initcontainer" {
-					daemonSet.Spec.Template.Spec.InitContainers[i].VolumeMounts = append(daemonSet.Spec.Template.Spec.InitContainers[i].VolumeMounts, v1.VolumeMount{
-						Name:      "intel-dsa-config-volume",
-						MountPath: "/idxd-init/conf",
-					})
-				}
-			}
-		}
+		addInitContainer(&daemonSet, devicePlugin)
 	}
 
 	return &daemonSet