Skip to content
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

[RFC] change systemd drivers to use sub-cgroups #2602

Closed
kolyshkin opened this issue Sep 28, 2020 · 8 comments
Closed

[RFC] change systemd drivers to use sub-cgroups #2602

kolyshkin opened this issue Sep 28, 2020 · 8 comments

Comments

@kolyshkin
Copy link
Contributor

Currently, runc's systemd cgroup driver (both v1 and v2) set all the resources they can using systemd properties (and also sets Delegate=yes if possible, then sets all the resources using fs[2] driver. This seems wrong, as with Delegate=yes (see https://systemd.io/CGROUP_DELEGATION/) one is supposed to create and manage sub-cgroup(s) (as opposed to the cgroup of the systemd unit itself like it's currently done).

A possible fix is to create a systemd scope unit with minimal set of properties (Delegate=yes, DefaultDependencies=false, and those from org.systemd.property annotations (see https://github.com/opencontainers/runc/blob/master/docs/systemd-properties.md), then use fs[2] cgroup manager to create a sub-cgroup under it and set all the parameters.

This way, though, we won't see or be able to change any limits using systemd tooling.

@kolyshkin
Copy link
Contributor Author

IOW, I think we violate this rule (from https://systemd.io/CGROUP_DELEGATION/):

🚫 Never write to any of the attributes of a cgroup systemd created for you. It’s systemd’s private property. You are welcome to manipulate the attributes of cgroups you created in your own delegated sub-tree, but the cgroup tree of systemd itself is out of limits for you. It’s fine to read from any attribute you like however. That’s totally OK and welcome.

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Sep 29, 2020

Related: #2448 (Support "run.oci.systemd.subgroup" annotation)

@AkihiroSuda
Copy link
Member

SGTM. This behavior also corresponds to crun AFAIK.

@AkihiroSuda AkihiroSuda added this to the 1.0.0-rc93 milestone Sep 29, 2020
@AkihiroSuda
Copy link
Member

Added to rc93 milestone, but feel free to change.

@kolyshkin
Copy link
Contributor Author

OK so we have discussed it with @mrunalp and @giuseppe and looks like we can proceed with the plan outlined in #2603.

The biggest concern about switching systemd drivers to create a sub-cgroup is that software that uses libcontainer (kubelet etc.) might break. We could do it as an option but I don't think there will be any benefit.

@kolyshkin kolyshkin removed this from the 1.0.0-rc93 milestone Oct 1, 2020
@AkihiroSuda
Copy link
Member

So shall we close this?

@AkihiroSuda
Copy link
Member

Ping @kolyshkin

@kolyshkin
Copy link
Contributor Author

Yes, let's close this one (in favor of #2603).

Another thing why sub-cgroups are bad is the deeper the hierarchy is, the slower the kernel (this might eventually be fixed in the kernel, but in general it's a concern)/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants