Merge pull request #1860 from mythi/PR-2024-022

qat: drop AppArmor annotations

Author: tkatila

cmd/qat_plugin/README.md

@@ -149,8 +149,6 @@ There's also a possibility for a node specific congfiguration through passing a
 
 Existing DaemonSet annotations can be updated through CR annotations in [deviceplugin_v1_qatdeviceplugin.yaml](../../deployments/operator/samples/deviceplugin_v1_qatdeviceplugin.yaml).
 
-By default, the operator based deployment sets AppArmor policy to `"unconfined"` but this can be overridden by setting the AppArmor annotation to a new value in the CR annotations.
-
 For non-operator plugin deployments such annotations can be dropped with the kustomization if required.
 
 ### Verify Plugin Registration

cmd/qat_plugin/dpdkdrv/dpdkdrv.go

@@ -44,7 +44,6 @@ const (
 	pciDriverDirectory = "/sys/bus/pci/drivers"
 	uioSuffix          = "uio"
 	iommuGroupSuffix   = "iommu_group"
-	vendorPrefix       = "8086 "
 	envVarPrefix       = "QAT"
 
 	igbUio  = "igb_uio"
@@ -187,31 +186,10 @@ func newDevicePlugin(pciDriverDir, pciDeviceDir string, maxDevices int, kernelVf
 	}
 }
 
-func (dp *DevicePlugin) setupDeviceIDs() error {
-	for devID, driver := range qatDeviceDriver {
-		for _, enabledDriver := range dp.kernelVfDrivers {
-			if driver != enabledDriver {
-				continue
-			}
-
-			err := writeToDriver(filepath.Join(dp.pciDriverDir, dp.dpdkDriver, "new_id"), vendorPrefix+devID)
-			if err != nil && !errors.Is(err, os.ErrExist) {
-				return errors.WithMessagef(err, "failed to set device ID %s for %s. Driver module not loaded?", devID, dp.dpdkDriver)
-			}
-		}
-	}
-
-	return nil
-}
-
 // Scan implements Scanner interface for vfio based QAT plugin.
 func (dp *DevicePlugin) Scan(notifier dpapi.Notifier) error {
 	defer dp.scanTicker.Stop()
 
-	if err := dp.setupDeviceIDs(); err != nil {
-		return err
-	}
-
 	for {
 		devTree, err := dp.scan()
 		if err != nil {
@@ -629,6 +607,7 @@ func (dp *DevicePlugin) scan() (dpapi.DeviceTree, error) {
 	for _, vfDevice := range dp.getVfDevices() {
 		vfBdf := filepath.Base(vfDevice)
 
+		// TODO(mythi): can be dropped in a later release since the same is already done in qat-init.sh.
 		if drv := getCurrentDriver(vfDevice); drv != dp.dpdkDriver {
 			if drv != "" {
 				err := writeToDriver(filepath.Join(dp.pciDriverDir, drv, "unbind"), vfBdf)

demo/qat-init.sh

@@ -58,10 +58,32 @@ enable_sriov() {
     echo "error: $NUMVFS is not found or not writable. Check if QAT driver module is loaded"
     exit 1
   fi
+  if ! test -d /sys/bus/pci/drivers/vfio-pci; then
+    echo "error: vfio-pci driver needed by QAT VFs must be loaded"
+    exit 1
+  fi
   if [ "$(cat "$NUMVFS")" -ne 0 ]; then
     echo "$DEVPATH already configured"
   else
     tee "$NUMVFS" < "$DEVPATH/sriov_totalvfs"
+    VFDEVS=$(realpath -L "$DEVPATH"/virtfn*)
+    for vfdev in $VFDEVS; do
+      BSF=$(basename "$vfdev")
+      VF_DEV="/sys/bus/pci/devices/$BSF"
+      if test -e "$VF_DEV/driver"; then
+        P=$(realpath -L "$VF_DEV/driver")
+        VF_DRIVER=$(basename "$P")
+      else
+        VF_DRIVER=""
+      fi
+      if [ "$VF_DRIVER" != "vfio-pci" ]; then
+        if [ "$VF_DRIVER" ]; then
+          echo -n "$BSF" > /sys/bus/pci/drivers/"$VF_DRIVER"/unbind
+        fi
+        echo -n vfio-pci > /sys/bus/pci/devices/"$BSF"/driver_override
+        echo -n "$BSF" > /sys/bus/pci/drivers/vfio-pci/bind
+      fi
+    done
   fi
   done
 }

deployments/operator/samples/deviceplugin_v1_dlbdeviceplugin.yaml

@@ -2,12 +2,6 @@ apiVersion: deviceplugin.intel.com/v1
 kind: DlbDevicePlugin
 metadata:
   name: dlbdeviceplugin-sample
-  # example apparmor annotation
-  # see more details here:
-  #  - https://kubernetes.io/docs/tutorials/clusters/apparmor/#securing-a-pod
-  #  - https://github.com/intel/intel-device-plugins-for-kubernetes/issues/381
-  # annotations:
-  #   container.apparmor.security.beta.kubernetes.io/intel-dlb-plugin: unconfined
 spec:
   image: intel/intel-dlb-plugin:0.31.1
   initImage: intel/intel-dlb-initcontainer:0.31.1

deployments/operator/samples/deviceplugin_v1_qatdeviceplugin.yaml

@@ -2,12 +2,6 @@ apiVersion: deviceplugin.intel.com/v1
 kind: QatDevicePlugin
 metadata:
   name: qatdeviceplugin-sample
-  # example apparmor annotation
-  # see more details here:
-  #  - https://kubernetes.io/docs/tutorials/clusters/apparmor/#securing-a-pod
-  #  - https://github.com/intel/intel-device-plugins-for-kubernetes/issues/381
-  # annotations:
-  #   container.apparmor.security.beta.kubernetes.io/intel-qat-plugin: unconfined
 spec:
   image: intel/intel-qat-plugin:0.31.1
   initImage: intel/intel-qat-initcontainer:0.31.1

deployments/qat_plugin/base/intel-qat-plugin.yaml

@@ -4,8 +4,6 @@ metadata:
   name: intel-qat-plugin
   labels:
     app: intel-qat-plugin
-  annotations:
-    container.apparmor.security.beta.kubernetes.io/intel-qat-plugin: unconfined
 spec:
   selector:
     matchLabels:
@@ -19,8 +17,6 @@ spec:
     metadata:
       labels:
         app: intel-qat-plugin
-      annotations:
-        container.apparmor.security.beta.kubernetes.io/intel-qat-plugin: unconfined
     spec:
       automountServiceAccountToken: false
       containers:

pkg/controllers/qat/controller.go

@@ -78,12 +78,8 @@ func (c *controller) Upgrade(ctx context.Context, obj client.Object) bool {
 func (c *controller) NewDaemonSet(rawObj client.Object) *apps.DaemonSet {
 	devicePlugin := rawObj.(*devicepluginv1.QatDevicePlugin)
 
-	annotations := devicePlugin.ObjectMeta.DeepCopy().Annotations
-
 	daemonSet := deployments.QATPluginDaemonSet()
 	daemonSet.Name = controllers.SuffixedName(daemonSet.Name, devicePlugin.Name)
-	daemonSet.Annotations = annotations
-	daemonSet.Spec.Template.Annotations = annotations
 
 	if devicePlugin.Spec.Tolerations != nil {
 		daemonSet.Spec.Template.Spec.Tolerations = devicePlugin.Spec.Tolerations
@@ -107,15 +103,6 @@ func (c *controller) NewDaemonSet(rawObj client.Object) *apps.DaemonSet {
 func (c *controller) UpdateDaemonSet(rawObj client.Object, ds *apps.DaemonSet) (updated bool) {
 	dp := rawObj.(*devicepluginv1.QatDevicePlugin)
 
-	// Update only existing daemonset annotations
-	for k, v := range ds.ObjectMeta.Annotations {
-		if v2, ok := dp.ObjectMeta.Annotations[k]; ok && v2 != v {
-			ds.ObjectMeta.Annotations[k] = v2
-			ds.Spec.Template.Annotations[k] = v2
-			updated = true
-		}
-	}
-
 	if ds.Spec.Template.Spec.Containers[0].Image != dp.Spec.Image {
 		ds.Spec.Template.Spec.Containers[0].Image = dp.Spec.Image
 		updated = true

pkg/controllers/qat/controller_test.go

@@ -39,7 +39,6 @@ func (c *controller) newDaemonSetExpected(rawObj client.Object) *apps.DaemonSet
 	devicePlugin := rawObj.(*devicepluginv1.QatDevicePlugin)
 	yes := true
 	no := false
-	pluginAnnotations := devicePlugin.ObjectMeta.DeepCopy().Annotations
 	maxUnavailable := intstr.FromInt(1)
 	maxSurge := intstr.FromInt(0)
 
@@ -54,7 +53,6 @@ func (c *controller) newDaemonSetExpected(rawObj client.Object) *apps.DaemonSet
 			Labels: map[string]string{
 				"app": appLabel,
 			},
-			Annotations: pluginAnnotations,
 		},
 		Spec: apps.DaemonSetSpec{
 			Selector: &metav1.LabelSelector{
@@ -74,7 +72,6 @@ func (c *controller) newDaemonSetExpected(rawObj client.Object) *apps.DaemonSet
 					Labels: map[string]string{
 						"app": appLabel,
 					},
-					Annotations: pluginAnnotations,
 				},
 				Spec: v1.PodSpec{
 					AutomountServiceAccountToken: &no,
@@ -187,13 +184,7 @@ func (c *controller) newDaemonSetExpected(rawObj client.Object) *apps.DaemonSet
 func TestNewDaemonSetQAT(t *testing.T) {
 	c := &controller{}
 
-	plugin := &devicepluginv1.QatDevicePlugin{
-		ObjectMeta: metav1.ObjectMeta{
-			Annotations: map[string]string{
-				"container.apparmor.security.beta.kubernetes.io/intel-qat-plugin": "runtime/default",
-			},
-		},
-	}
+	plugin := &devicepluginv1.QatDevicePlugin{}
 	plugin.Name = "testing"
 	plugin.Spec.InitImage = "intel/intel-qat-initcontainer:" + controllers.ImageMinVersion.String()
 

test/envtest/qatdeviceplugin_controller_test.go

@@ -46,14 +46,9 @@ var _ = Describe("QatDevicePlugin Controller", func() {
 				Name: "qatdeviceplugin-test",
 			}
 
-			annotations := map[string]string{
-				"container.apparmor.security.beta.kubernetes.io/intel-qat-plugin": "unconfined",
-			}
-
 			toCreate := &devicepluginv1.QatDevicePlugin{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:        key.Name,
-					Annotations: annotations,
+					Name: key.Name,
 				},
 				Spec: spec,
 			}
@@ -80,20 +75,6 @@ var _ = Describe("QatDevicePlugin Controller", func() {
 			Expect(ds.Spec.Template.Spec.NodeSelector).To(Equal(spec.NodeSelector))
 			Expect(ds.Spec.Template.Spec.Tolerations).To(HaveLen(0))
 
-			By("copy annotations successfully")
-			Expect(&(fetched.Annotations) == &annotations).ShouldNot(BeTrue())
-			Eventually(fetched.Annotations).Should(Equal(annotations))
-
-			By("updating annotations successfully")
-			updatedAnnotations := map[string]string{"key": "value"}
-			fetched.Annotations = updatedAnnotations
-			Expect(k8sClient.Update(context.Background(), fetched)).Should(Succeed())
-			updated := &devicepluginv1.QatDevicePlugin{}
-			Eventually(func() map[string]string {
-				_ = k8sClient.Get(context.Background(), key, updated)
-				return updated.Annotations
-			}, timeout, interval).Should(Equal(updatedAnnotations))
-
 			By("updating QatDevicePlugin successfully")
 			updatedImage := "updated-qat-testimage"
 			updatedInitImage := "updated-qat-testinitimage"