ON-15386: Add configurable file mount locations to CRD

Author: tcrawley-xilinx

api/v1alpha1/onload_types.go

@@ -104,7 +104,7 @@ type DevicePluginSpec struct {
 	// +optional
 	// MountOnload is used by the Onload Device Plugin to decide whether to
 	// mount the `onload` script as a file in the container's filesystem.
-	// `onload` is mounted at `/usr/bin/onload`
+	// `onload` is mounted at `<baseMountPath>/<binMountpath>`
 	// Mutually exclusive with Preload
 	// +kubebuilder:default:=false
 	MountOnload *bool `json:"mountOnload,omitempty"`
@@ -117,19 +117,19 @@ type DevicePluginSpec struct {
 
 	// +optional
 	// BaseMountPath is a prefix to be applied to all onload file mounts in the
-	// container's file system.
+	// container's filesystem.
 	// +kubebuilder:default=/opt/onload
 	BaseMountPath *string `json:"baseMountPath,omitempty"`
 
 	// +optional
 	// BinMountPath is the location to mount onload binaries in the container's
-	// file system.
+	// filesystem.
 	// +kubebuilder:default=/usr/bin
 	BinMountPath *string `json:"binMountPath,omitempty"`
 
 	// +optional
 	// LibMountPath is the location to mount onload libraries in the container's
-	// file system.
+	// filesystem.
 	// +kubebuilder:default=/usr/lib64
 	LibMountPath *string `json:"libMounthPath,omitempty"`
 }

api/v1alpha1/zz_generated.deepcopy.go

@@ -26,14 +26,18 @@ func main() {
 	flag.BoolVar(&config.MountOnload, "mountOnload",
 		deviceplugin.DefaultConfig.MountOnload,
 		"Should the device plugin mount the onload script into the pod")
-	flag.StringVar(&config.HostPathPrefix, "hostOnloadPath", "/opt/onload/",
+	flag.StringVar(&config.HostPathPrefix, "hostOnloadPath",
+		deviceplugin.DefaultConfig.HostPathPrefix,
 		"Base location of onload files on the host filesystem")
-	flag.StringVar(&config.BaseMountPath, "baseMountPath", "/opt/onload",
-		"Prefix to be applied to all file mounts in the container's file system")
-	flag.StringVar(&config.BinMountPath, "binMountPath", "/usr/bin/",
-		"Location to mount onload binaries in the container's file system")
-	flag.StringVar(&config.LibMountPath, "libMountPath", "/usr/lib64",
-		"Location to mount onload libraries in the container's file system")
+	flag.StringVar(&config.BaseMountPath, "baseMountPath",
+		deviceplugin.DefaultConfig.BaseMountPath,
+		"Prefix to be applied to all file mounts in the container's filesystem")
+	flag.StringVar(&config.BinMountPath, "binMountPath",
+		deviceplugin.DefaultConfig.BinMountPath,
+		"Location to mount onload binaries in the container's filesystem")
+	flag.StringVar(&config.LibMountPath, "libMountPath",
+		deviceplugin.DefaultConfig.LibMountPath,
+		"Location to mount onload libraries in the container's filesystem")
 	flag.Parse()
 	err := flag.Lookup("logtostderr").Value.Set("true")
 	if err != nil {

cmd/deviceplugin/main.go

@@ -41,13 +41,33 @@ spec:
                 description: DevicePlugin is the specification of the device plugin
                   that will add a new onload resource into the cluster.
                 properties:
+                  baseMountPath:
+                    default: /opt/onload
+                    description: BaseMountPath is a prefix to be applied to all onload
+                      file mounts in the container's filesystem.
+                    type: string
+                  binMountPath:
+                    default: /usr/bin
+                    description: BinMountPath is the location to mount onload binaries
+                      in the container's filesystem.
+                    type: string
                   devicePluginImage:
                     description: DevicePluginImage
                     type: string
+                  hostOnloadPath:
+                    default: /opt/onload/
+                    description: HostOnloadPath is the base location of onload files
+                      on the host filesystem.
+                    type: string
                   imagePullPolicy:
                     description: 'ImagePullPolicy is the policy used when pulling
                       images. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images'
                     type: string
+                  libMounthPath:
+                    default: /usr/lib64
+                    description: LibMountPath is the location to mount onload libraries
+                      in the container's filesystem.
+                    type: string
                   maxPodsPerNode:
                     default: 100
                     description: MaxPodsPerNode is the number of Kubernetes devices
@@ -59,7 +79,7 @@ spec:
                     default: false
                     description: MountOnload is used by the Onload Device Plugin to
                       decide whether to mount the `onload` script as a file in the
-                      container's filesystem. `onload` is mounted at `/usr/bin/onload`
+                      container's filesystem. `onload` is mounted at `<baseMountPath>/<binMountpath>`
                       Mutually exclusive with Preload
                     type: boolean
                   setPreload:

config/crd/bases/onload.amd.com_onloads.yaml

@@ -58,3 +58,7 @@ spec:
     # Uncomment to select whether LD_PRELOAD should be set by the device plugin
     # preload: true
     # mountOnload: false
+    # hostOnloadPath: /opt/onload
+    # baseMountPath: /opt/onload
+    # binMountPath: /usr/bin
+    # libMountPath: /usr/lib64

config/samples/onload/base/onload_v1alpha1_onload.yaml

@@ -882,20 +882,51 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(
 		return nil, err
 	}
 
+	hostOnloadPath := "/opt/onload"
+	if onload.Spec.DevicePlugin.HostOnloadPath != nil {
+		hostOnloadPath = *onload.Spec.DevicePlugin.HostOnloadPath
+	}
+
+	postStart := &corev1.LifecycleHandler{
+		Exec: &corev1.ExecAction{
+			Command: []string{
+				"/bin/sh", "-c",
+				fmt.Sprintf(`set -e;
+				chcon --type container_file_t --recursive %s ||
+				echo "chcon failed. System may not be SELinux enabled.";`, hostOnloadPath),
+			},
+		},
+	}
+
+	preStop := &corev1.LifecycleHandler{
+		Exec: &corev1.ExecAction{
+			Command: []string{
+				"/bin/sh", "-c",
+				fmt.Sprintf(`set -e;
+				rm -r %s;`, hostOnloadPath),
+			},
+		},
+	}
+
 	devicePluginContainer := corev1.Container{
 		Name:            "device-plugin",
 		Image:           onload.Spec.DevicePlugin.DevicePluginImage,
 		ImagePullPolicy: onload.Spec.DevicePlugin.ImagePullPolicy,
 		SecurityContext: &corev1.SecurityContext{
 			Privileged: ptr.To(true),
 		},
+		// Lifecycle to manage the Onload files in the host.
+		Lifecycle: &corev1.Lifecycle{
+			PostStart: postStart,
+			PreStop:   preStop,
+		},
 		VolumeMounts: []corev1.VolumeMount{
 			{
 				MountPath: "/var/lib/kubelet/device-plugins",
 				Name:      "kubelet-socket",
 			},
 			{
-				MountPath: "/opt/onload",
+				MountPath: hostOnloadPath,
 				Name:      "host-onload",
 			},
 		},
@@ -917,6 +948,30 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(
 			fmt.Sprintf("-mountOnload=%t", *onload.Spec.DevicePlugin.MountOnload))
 	}
 
+	if onload.Spec.DevicePlugin.HostOnloadPath != nil {
+		devicePluginArgs = append(devicePluginArgs,
+			fmt.Sprintf("-hostOnloadPath=%s",
+				*onload.Spec.DevicePlugin.HostOnloadPath))
+	}
+
+	if onload.Spec.DevicePlugin.BaseMountPath != nil {
+		devicePluginArgs = append(devicePluginArgs,
+			fmt.Sprintf("-baseMountPath=%s",
+				*onload.Spec.DevicePlugin.BaseMountPath))
+	}
+
+	if onload.Spec.DevicePlugin.BinMountPath != nil {
+		devicePluginArgs = append(devicePluginArgs,
+			fmt.Sprintf("-binMountPath=%s",
+				*onload.Spec.DevicePlugin.BinMountPath))
+	}
+
+	if onload.Spec.DevicePlugin.LibMountPath != nil {
+		devicePluginArgs = append(devicePluginArgs,
+			fmt.Sprintf("-libMountPath=%s",
+				*onload.Spec.DevicePlugin.LibMountPath))
+	}
+
 	if len(devicePluginArgs) > 0 {
 		devicePluginContainer.Args = devicePluginArgs
 	}
@@ -950,27 +1005,6 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(
 		},
 	}
 
-	postStart := &corev1.LifecycleHandler{
-		Exec: &corev1.ExecAction{
-			Command: []string{
-				"/bin/sh", "-c",
-				`set -e;
-				chcon --type container_file_t --recursive /opt/onload/ ||
-				echo "chcon failed. System may not be SELinux enabled.";`,
-			},
-		},
-	}
-
-	preStop := &corev1.LifecycleHandler{
-		Exec: &corev1.ExecAction{
-			Command: []string{
-				"/bin/sh", "-c",
-				`set -e;
-				rm -r /opt/onload;`,
-			},
-		},
-	}
-
 	workerContainer := corev1.Container{
 		Name:            workerContainerName,
 		Image:           onload.Spec.DevicePlugin.DevicePluginImage,
@@ -983,21 +1017,11 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(
 			Privileged: ptr.To(true),
 		},
 		VolumeMounts: []corev1.VolumeMount{
-			{
-				MountPath: "/opt/onload",
-				Name:      "host-onload",
-			},
 			{
 				MountPath: "/mnt/onload",
 				Name:      "worker-volume",
 			},
 		},
-
-		// Lifecycle to manage the Onload files in the host.
-		Lifecycle: &corev1.Lifecycle{
-			PostStart: postStart,
-			PreStop:   preStop,
-		},
 		Env: workerContainerEnv,
 	}
 
@@ -1023,6 +1047,10 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(
 		},
 		VolumeMounts: []corev1.VolumeMount{
 			{
+				// MounthPath here doesn't have to match hostOnloadPath. The
+				// location in the initContainer's filesystem doesn't affect the
+				// device plugin, only whether they are both looking at the same
+				// volumeMount.
 				MountPath: "/host/onload",
 				Name:      "host-onload",
 			},
@@ -1049,7 +1077,7 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(
 		Name: "host-onload",
 		VolumeSource: corev1.VolumeSource{
 			HostPath: &corev1.HostPathVolumeSource{
-				Path: "/opt/onload",
+				Path: hostOnloadPath,
 				Type: ptr.To(corev1.HostPathDirectoryOrCreate),
 			},
 		},

controllers/onload_controller.go

@@ -4,7 +4,6 @@ package controllers
 
 import (
 	"errors"
-	"fmt"
 	"strconv"
 	"time"
 
@@ -793,7 +792,7 @@ var _ = Describe("onload controller", func() {
 		)
 
 		DescribeTable("Testing Device Plugin options",
-			func(dev *onloadv1alpha1.DevicePluginSpec) {
+			func(dev *onloadv1alpha1.DevicePluginSpec, args string) {
 				devicePlugin := appsv1.DaemonSet{}
 				devicePluginName := types.NamespacedName{
 					Name:      onload.Name + "-onload-device-plugin-ds",
@@ -812,44 +811,43 @@ var _ = Describe("onload controller", func() {
 					return err == nil
 				}, timeout, pollingInterval).Should(BeTrue())
 
-				Expect(devicePlugin.Spec.Template.Spec.Containers).Should(
-					ContainElement(MatchFields(IgnoreExtras, Fields{
-						"Args": BeEmpty(),
-					})),
-				)
-				if dev != nil {
-					if dev.MaxPodsPerNode != nil {
-						Expect(devicePlugin.Spec.Template.Spec.Containers).Should(
-							ContainElement(MatchFields(IgnoreExtras, Fields{
-								"Args": ContainElement(Equal(fmt.Sprintf("-maxPods=%d", *dev.MaxPodsPerNode))),
-							})),
-						)
-					}
-					if dev.SetPreload != nil {
-						Expect(devicePlugin.Spec.Template.Spec.Containers).Should(
-							ContainElement(MatchFields(IgnoreExtras, Fields{
-								"Args": ContainElement(Equal(fmt.Sprintf("-setPreload=%t", *dev.SetPreload))),
-							})),
-						)
-					}
-					if dev.MountOnload != nil {
-						Expect(devicePlugin.Spec.Template.Spec.Containers).Should(
-							ContainElement(MatchFields(IgnoreExtras, Fields{
-								"Args": ContainElement(Equal(fmt.Sprintf("-mountOnload=%t", *dev.MountOnload))),
-							})),
-						)
-					}
+				if args != "" {
+					Expect(devicePlugin.Spec.Template.Spec.Containers).Should(
+						ContainElement(MatchFields(IgnoreExtras, Fields{
+							"Args": ContainElement(args),
+						})),
+					)
 				}
+
 			},
-			Entry( /*It*/ "shouldn't add anything when empty", nil),
+			Entry( /*It*/ "shouldn't add anything when empty", nil, ""),
 			Entry( /*It*/ "should pass the value of maxPodsPerNode through",
 				&onloadv1alpha1.DevicePluginSpec{MaxPodsPerNode: ptr.To(1)},
+				"-maxPods=1",
 			),
 			Entry( /*It*/ "should pass the value of setPreload through",
 				&onloadv1alpha1.DevicePluginSpec{SetPreload: ptr.To(false)},
+				"-setPreload=false",
 			),
 			Entry( /*It*/ "should pass the value of mountOnload through",
 				&onloadv1alpha1.DevicePluginSpec{MountOnload: ptr.To(false)},
+				"-mountOnload=false",
+			),
+			Entry( /*It*/ "should pass the value of hostOnloadPath through",
+				&onloadv1alpha1.DevicePluginSpec{HostOnloadPath: ptr.To("foo")},
+				"-hostOnloadPath=foo",
+			),
+			Entry( /*It*/ "should pass the value of baseMountPath through",
+				&onloadv1alpha1.DevicePluginSpec{BaseMountPath: ptr.To("bar")},
+				"-baseMountPath=bar",
+			),
+			Entry( /*It*/ "should pass the value of binMountPath through",
+				&onloadv1alpha1.DevicePluginSpec{BinMountPath: ptr.To("baz")},
+				"-binMountPath=baz",
+			),
+			Entry( /*It*/ "should pass the value of libMountPath through",
+				&onloadv1alpha1.DevicePluginSpec{LibMountPath: ptr.To("qux")},
+				"-libMountPath=qux",
 			),
 		)
 	})