Skip to content

qat: Add a note on plugin deployment with AppArmor #1576

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

Closed
wants to merge 1 commit into from

Conversation

ozhuraki
Copy link
Contributor

No description provided.

@codecov-commenter
Copy link

Codecov Report

Merging #1576 (7e52142) into main (0ffe7d0) will decrease coverage by 0.07%.
Report is 14 commits behind head on main.
The diff coverage is 14.00%.

❗ Current head 7e52142 differs from pull request most recent head b9e6849. Consider uploading reports for the commit b9e6849 to get more accurate results

@@            Coverage Diff             @@
##             main    #1576      +/-   ##
==========================================
- Coverage   49.63%   49.56%   -0.07%     
==========================================
  Files          42       42              
  Lines        4948     4965      +17     
==========================================
+ Hits         2456     2461       +5     
- Misses       2351     2361      +10     
- Partials      141      143       +2     
Files Coverage Δ
pkg/controllers/reconciler.go 4.56% <ø> (ø)
pkg/controllers/dlb/controller.go 17.07% <0.00%> (-0.29%) ⬇️
pkg/controllers/qat/controller.go 9.52% <0.00%> (-0.21%) ⬇️
pkg/controllers/dsa/controller.go 6.74% <0.00%> (-0.04%) ⬇️
pkg/controllers/iaa/controller.go 6.74% <0.00%> (-0.04%) ⬇️
pkg/controllers/gpu/controller.go 48.01% <36.84%> (+0.31%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@hj-johannes-lee hj-johannes-lee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

NOTE: In case AppArmor kernel module is installed and enabled by default (Ubuntu, SUSE), use the customization overlay with AppArmor annnotations (otherwise plugin's daemonset will fail with bind/unbind errors):

```bash
$ kubectl apply -k 'https://github.com/intel/intel-device-plugins-for-kubernetes/deployments/qat_plugin/overlays/apparmor_unconfined?ref=<RELEASE_VERSION>'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will not work anymore if the user is also interested in Automatic Provisioning. We will have to re-think the apparmor case thoroughly in #1575 before adding random notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will not work anymore if the user is also interested in Automatic Provisioning

I re-tested, the annotation has no effect on provisioning, the provisioning is fine with or without it.

What connection this has to the automatic provisioning and why won't it work anymore?

We will have to re-think the apparmor case thoroughly in #1575 before adding random notes.

We already have this overlay, also the annotation is used in e2e tests.

In what way is this comment random and what's preventing us to document an existing behaviour?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What connection this has to the automatic provisioning and why won't it work anymore?

that if they deploy overlays/qat_initcontainer, they won't get the annotation.

In what way is this comment random and what's preventing us to document an existing behaviour?

it does not cover the helm/operator flows for example. I submitted #1575 so that the best approach to address the problem is agreed first

@mythi
Copy link
Contributor

mythi commented Nov 10, 2023

superseded by #1591

@mythi mythi closed this Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants