-
Notifications
You must be signed in to change notification settings - Fork 42
webhook: enforce min memory limits and allow privileged containers #418
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
Conversation
sprt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Redent0r Could you upstream the privileged commit? Do you know who uses the webhook upstream?
@Camelron @danmihai1 Does it make any sense to upstream the memory limit commit (which is already behind a flag) given how we handle memory?
Someone would have to carefully compare the upstream behavior with the behavior of CLH + MSHV. I expect that it's very different and the discussion with upstream would take a long time. |
Judging by CI usage, only openshift https://github.com/kata-containers/kata-containers/blob/main/ci/openshift-ci/peer-pods-azure.sh#L269 I'll reach out |
Agreed - we can open a PR and add Lukasz and Greg as reviewers. |
|
sprt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but the commit messages could be more descriptive for posterity.
If memory limit is set and less than minimum, set it to minimum. This is to to account for kata-containers@0ec3403 Signed-off-by: Saul Paredes <[email protected]>
As of https://microsoft.visualstudio.com/OS/_workitems/edit/48222512?src=WorkItemMention&src-action=artifact_link , we are able to run privileged containers on kata, so allow them through the webhook. Signed-off-by: Saul Paredes <[email protected]>
41a75a5 to
c7caa15
Compare
Merge Checklist
Summary
This merges to msft-main the current webhook implementation we use in conformance tests. We differ from the upstream implementation in 2 ways:
Associated issues
Links to CVEs
Test Methodology
https://dev.azure.com/mariner-org/mariner/_build/results?buildId=963302&view=results