Support resource restrictions for rootless containers #499
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Use systemd client to find systemd cgroup root - Add error context - Manager debug impl - Comments - Set default slice name for rootless and rootfull containers
Furisto
force-pushed
the
systemd-part3
branch
from
a306996
to
9cff024
Compare
Furisto
force-pushed
the
systemd-part3
branch
from
5d07e81
to
7e3bdcc
Compare
Codecov Report
@@ Coverage Diff @@
## main #499 +/- ##
==========================================
- Coverage 61.09% 60.55% -0.55%
==========================================
Files 85 86 +1
Lines 12417 12604 +187
==========================================
+ Hits 7586 7632 +46
- Misses 4831 4972 +141 |
Furisto
force-pushed
the
systemd-part3
branch
from
7e3bdcc
to
f92b265
Compare
utam0k
reviewed
Nov 28, 2021
| } else { | ||
| let parts = cgroups_path | ||
| .to_str() | ||
| .ok_or_else(|| anyhow!("failed to parse cgroupsPath field"))? | ||
| .ok_or_else(|| anyhow!("failed to parse cgroups path"))? |
There was a problem hiding this comment.
Here, the error is raised when the cgroup path is not in the format of [slice]:[scope_prefix]:[name], right?
I want to output the error message properly in case it fails.
utam0k
reviewed
Nov 28, 2021
| print_feature_status(&content, "CONFIG_USER_NS", FeatureDisplay::new("user")); | ||
|
|
||
| let user_display = match rootless::unprivileged_user_ns_enabled() { | ||
| Ok(false) => FeatureDisplay::with_status("user", "enabled (root only)", "disabled"), |
utam0k
reviewed
Nov 28, 2021
| // this needs to be done before we create the init process, so that the init | ||
| // process will already be captured by the cgroup. It also needs to be done | ||
| // before we enter the user namespace because if a privileged user starts a | ||
| // rootless container on a cgroup v1 system we can still fullfill resource |
There was a problem hiding this comment.
Suggested change
| // rootless container on a cgroup v1 system we can still fullfill resource | |
| // rootless container on a cgroup v1 system we can still fulfill resource |
utam0k
reviewed
Nov 28, 2021
Comment on lines
25
to
40
| // this needs to be done before we create the init process, so that the init | ||
| // process will already be captured by the cgroup. It also needs to be done | ||
| // before we enter the user namespace because if a privileged user starts a | ||
| // rootless container on a cgroup v1 system we can still fullfill resource | ||
| // restrictions through the cgroup fs support (delegation through systemd is | ||
| // not supported for v1 by us). This only works if the user has not yet been | ||
| // mapped to an unprivileged user by the user namespace however. | ||
| // In addition this needs to be done before we enter the cgroup namespace as | ||
| // the cgroup of the process will form the root of the cgroup hierarchy in | ||
| // the cgroup namespace. | ||
| apply_cgroups( | ||
| args.cgroup_manager.as_ref(), | ||
| linux.resources().as_ref(), | ||
| args.init, | ||
| ) | ||
| .context("failed to apply cgroups")?; |
There was a problem hiding this comment.
Could I ask you to update the group part of the sequence diagram in README?
utam0k
reviewed
Nov 28, 2021
Comment on lines
+36
to
+49
| /// Root path of the cgroup hierarchy e.g. /sys/fs/cgroup | ||
| root_path: PathBuf, | ||
| /// Path relative to the root path e.g. /system.slice/youki-569d5ce3afe1074769f67.scope for rootfull containers | ||
| /// and e.g. /user.slice/user-1000/user@1000.service/youki-569d5ce3afe1074769f67.scope for rootless containers | ||
| cgroups_path: PathBuf, | ||
| /// Combination of root path and cgroups path | ||
| full_path: PathBuf, | ||
| /// Destructured cgroups path as specified in the runtime spec e.g. system.slice:youki:569d5ce3afe1074769f67 | ||
| destructured_path: CgroupsPath, | ||
| /// Name of the container e.g. 569d5ce3afe1074769f67 | ||
| container_name: String, | ||
| /// Name of the systemd unit e.g. youki-569d5ce3afe1074769f67.scope | ||
| unit_name: String, | ||
| /// Client for communicating with systemd |
utam0k
reviewed
Nov 28, 2021
| }) | ||
| } | ||
|
|
||
| fn destructure_cgroups_path(cgroups_path: PathBuf) -> Result<CgroupsPath> { | ||
| log::debug!("CGROUPS PATH IS {:?}", cgroups_path); | ||
| fn destructure_cgroups_path(cgroups_path: &Path) -> Result<CgroupsPath> { |
There was a problem hiding this comment.
Maybe this should be implemented impl TryFrom<Path> for CgroupPath. What do you think?
- Add cgroups path to error context - Correct spelling mistake - Update sequence diagram - Implement TryFrom for CgroupsPath
|
@utam0k PTAL |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resource restrictions can now be applied to rootless containers (using systemd).
Systemd places units started by unprivileged users under /user.slice/user-$(id -u).slice/user@$(id -u).service. In order for resource restrictions to be applied correctly, you have to enable delegation for this service unit. This describes how to do that.