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

[backport] chown cgroup to process uid in container namespace #3311

Conversation

frasertweedale
Copy link
Contributor

(backport of #3057)

Delegating cgroups to the container enables more complex workloads,
including systemd-based workloads. The OCI runtime-spec was
recently updated to explicitly admit such delegation, through
specification of cgroup ownership semantics:

opencontainers/runtime-spec#1123

Pursuant to the updated OCI runtime-spec, change the ownership of
the container's cgroup directory and particular files therein, when
using cgroups v2 and when the cgroupfs is to be mounted read/write.

As a result of this change, systemd workloads can run in isolated
user namespaces on OpenShift when the sandbox's cgroupfs is mounted
read/write.

It might be possible to implement this feature in other cgroup
managers, but that work is deferred.

Delegating cgroups to the container enables more complex workloads,
including systemd-based workloads.  The OCI runtime-spec was
recently updated to explicitly admit such delegation, through
specification of cgroup ownership semantics:

  opencontainers/runtime-spec#1123

Pursuant to the updated OCI runtime-spec, change the ownership of
the container's cgroup directory and particular files therein, when
using cgroups v2 and when the cgroupfs is to be mounted read/write.

As a result of this change, systemd workloads can run in isolated
user namespaces on OpenShift when the sandbox's cgroupfs is mounted
read/write.

It might be possible to implement this feature in other cgroup
managers, but that work is deferred.

Signed-off-by: Fraser Tweedale <ftweedal@redhat.com>
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

I don't think this is safe to backport.

@frasertweedale
Copy link
Contributor Author

@AkihiroSuda what are your concerns?

Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

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

We will be doing a 1.1 release soon, which will include this patch. I don't think we have plans for another 1.0.x unless it's really necessary, so this patch doesn't need to be backported. It also is implementing a new runtime-spec feature which makes it not really eligible for backporting in the first place.

As for the safety point, there have been a lot of changes in the cgroup code in main which haven't been backported to release-1.0 which also makes me slightly nervous.

@frasertweedale
Copy link
Contributor Author

OK, wasn't aware of the timeline for 1.1. I'll close this PR.

@kolyshkin
Copy link
Contributor

1.0.x is a maintenance branch -- i.e. this is mostly for security and bug fixes. Unless the lack of a feature is critical, we do not add new features.

@frasertweedale frasertweedale deleted the feature/chown-cgroup-release-1.0 branch December 9, 2021 01:20
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.

4 participants