-
Notifications
You must be signed in to change notification settings - Fork 2.1k
docs: Clarify jailer's behavior when --parent-cgroup provided but --cgroup not provided #5431
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
docs: Clarify jailer's behavior when --parent-cgroup provided but --cgroup not provided #5431
Conversation
5137349
to
608246a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5431 +/- ##
=======================================
Coverage 82.68% 82.68%
=======================================
Files 263 263
Lines 27473 27473
=======================================
Hits 22717 22717
Misses 4756 4756
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
608246a
to
027fd24
Compare
The jailer CLI usage was missing trailing backslashes on several lines. Signed-off-by: Takahiro Itazuri <[email protected]>
The jailer CLI usage uses underscore for placeholders of all the parameters other than --cgroup-version Signed-off-by: Takahiro Itazuri <[email protected]>
The parenthesis was not closed. Signed-off-by: Takahiro Itazuri <[email protected]>
Since we removed the --node parameter in v1.0, we should not have description for the parameter. Fixes: b4d51ac ("jailer: remove --node parameter") Signed-off-by: Takahiro Itazuri <[email protected]>
ec4c031
to
b8ed66e
Compare
- Added `--` prefix when referring to the CLI option itself. - Wrapped placeholders for parameter values with angle brackets. Signed-off-by: Takahiro Itazuri <[email protected]>
Since the behavior of --parent-cgroup depends on whether --cgroup is provided or not and wether --cgroup-version is 1 or 2, explain the dependencies first and then --parent-cgroup. Signed-off-by: Takahiro Itazuri <[email protected]>
Reorganized the description of --parent-cgroup into bullet points to make clear that its behavior depends on the combination of parameters. Signed-off-by: Takahiro Itazuri <[email protected]>
If no --cgroup parameters are specified and --cgroup-version=2 is passed, the jailer moves the process to the cgroup specified with --parent-cgroup rather than creating a cgroup under it, contrary to its name. This move fails if the destination cgroup has domain controllers (e.g. memory) enabled in cgroup.subtree_control, which is called "no internal process constraint [1]. [1]: https://docs.kernel.org/admin-guide/cgroup-v2.html#no-internal-process-constraint Signed-off-by: Takahiro Itazuri <[email protected]>
The jailer's behavior when --parent-cgroup specified but no --cgroup provided varies depending on the existence and subtree_control configuration of the specified cgroup. Test all the cases for such a combination of parameters. Signed-off-by: Takahiro Itazuri <[email protected]>
We removed the requirement for jailer where the file name of `--exec-file` needs to contain `firecracker`. Fixes: aedee56 ("feat(jailer): remove requirement for an executable name") Signed-off-by: Takahiro Itazuri <[email protected]>
b8ed66e
to
ae32fb1
Compare
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
Changes
Reason
The behavior of
--parent-cgroup
parameter on cgroup v2 is a bit complicated:License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md
.PR Checklist
tools/devtool checkbuild --all
to verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyle
to verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
[ ] I have mentioned all user-facing changes inCHANGELOG.md
.[ ] If a specific issue led to this PR, this PR closes the issue.[ ] When making API changes, I have followed theRunbook for Firecracker API changes.
integration tests.
[ ] I have linked an issue to every newTODO
.rust-vmm
.