Skip to content

qat: drop AppArmor annotations #1860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
23 changes: 1 addition & 22 deletions cmd/qat_plugin/dpdkdrv/dpdkdrv.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const (
pciDriverDirectory = "/sys/bus/pci/drivers"
uioSuffix = "uio"
iommuGroupSuffix = "iommu_group"
vendorPrefix = "8086 "
envVarPrefix = "QAT"

igbUio = "igb_uio"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions demo/qat-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
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
Loading