Skip to content

Commit

Permalink
qat: drop AppArmor annotations
Browse files Browse the repository at this point in the history
"unconfined" annotation was needed to get writes to new_id / bind
to succeed on AppArmor enabled OSes.

However, many things have changed:

* new_id should not be used anymore and it was dropped in the plugin.
* QAT initcontainer has assumed the role of HW initialization.
* vfio-pci is the preferred "dpdkDriver" and starting with QAT Gen4, it
is the only available VF driver so unbind isn't necessary.
* k8s AppArmor is "GA" since 1.30 and the annotation is deprecated.

As of now, the initcontainer will take care of binding QAT VFs to vfio-pci
so the plugin does not neeed to set AppArmor at all.

Signed-off-by: Mikko Ylinen <[email protected]>
  • Loading branch information
mythi committed Jan 15, 2025
1 parent 5d38cd3 commit 2989f53
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 61 deletions.
2 changes: 0 additions & 2 deletions cmd/qat_plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions deployments/qat_plugin/base/intel-qat-plugin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
13 changes: 0 additions & 13 deletions pkg/controllers/qat/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
11 changes: 1 addition & 10 deletions pkg/controllers/qat/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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{
Expand All @@ -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,
Expand Down Expand Up @@ -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()

Expand Down
21 changes: 1 addition & 20 deletions test/envtest/qatdeviceplugin_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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"
Expand Down

0 comments on commit 2989f53

Please sign in to comment.