Skip to content

qat: Add AppArmor unconfided anntotation configurability in the operator #1591

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 1 commit into from
Jan 10, 2024

Conversation

ozhuraki
Copy link
Contributor

@ozhuraki ozhuraki commented Nov 8, 2023

Fixes: #1575

@codecov-commenter
Copy link

codecov-commenter commented Nov 8, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (ff41eb8) 51.89% compared to head (d4c9a75) 51.91%.
Report is 2 commits behind head on main.

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

Files Patch % Lines
pkg/controllers/qat/controller.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1591      +/-   ##
==========================================
+ Coverage   51.89%   51.91%   +0.02%     
==========================================
  Files          42       42              
  Lines        4916     4914       -2     
==========================================
  Hits         2551     2551              
+ Misses       2215     2213       -2     
  Partials      150      150              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

thanks @ozhuraki for getting this started! two comments

Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

forgot to add 'request changes'

Copy link
Contributor

@tkatila tkatila left a comment

Choose a reason for hiding this comment

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

I'd avoid having qat specific args in the operator cli. @mythi 's proposal for having the annotation as default and then allowing user to change it by modifying the CR seems like a better option.

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Nov 9, 2023

@tkatila @mythi

I'd avoid having qat specific args in the operator cli. @mythi 's proposal for having the annotation as default and then allowing user to change it by modifying the CR seems like a better option.

Thanks, preliminary makes sense, will take a look at this option.

@ozhuraki
Copy link
Contributor Author

@tkatila @mythi

I haven't found a good way to provide a kustomization without moving / duplicating the sample CR deployment (deployments/operator/samples/deviceplugin_v1_qatdeviceplugin.yaml), so resorted to documenting the annotation configurability through CR.

@ozhuraki ozhuraki requested review from tkatila and mythi November 14, 2023 20:36
Copy link
Contributor

@tkatila tkatila left a comment

Choose a reason for hiding this comment

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

Looks good!

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Jan 8, 2024

@tkatila @mythi

Updated to congfigure only existing annotations in the daemonset, please take a look.

Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

LGTM but changes in deviceplugin_v1_qatdeviceplugin.yaml must be dropped

@mythi mythi merged commit 2c26672 into intel:main Jan 10, 2024
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.

Improve QAT+Apparmor case
4 participants