cgroup v2: support rootless systemd #2281

AkihiroSuda · 2020-04-01T14:00:31Z

Tested with both Podman (master) and Moby (master), on Ubuntu 19.10 .

$ podman --cgroup-manager=systemd run -it --rm --runtime=runc \
  --cgroupns=host --memory 42m --cpus 0.42 --pids-limit 42 alpine
/ # cat /proc/self/cgroup
0::/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope
/ # cat
/sys/fs/cgroup/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope/memory.max
44040192
/ # cat
/sys/fs/cgroup/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope/cpu.max
42000 100000
/ # cat
/sys/fs/cgroup/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope/pids.max
42

The behavior corresponds to crun v0.13.

To test with Moby, launch dockerd-rootless.sh with --exec-opt native.cgroupdriver=systemd.

Fix #2163

AkihiroSuda · 2020-04-01T14:00:45Z

@giuseppe @filbranden @mrunalp PTAL

libcontainer/cgroups/systemd/user.go

kolyshkin

maybe make detectuid sligthly more forgiving to the value of XDG_RUNTIME_DIR

filbranden

Thanks for working on this @AkihiroSuda !

I have some comments, I hope you find them useful. Overall, looks really good!

Cheers,
Filipe

libcontainer/cgroups/systemd/unified_hierarchy.go

libcontainer/cgroups/systemd/user.go

filbranden · 2020-04-01T21:31:08Z

libcontainer/cgroups/systemd/user.go

+
+// detectUID detects UID from $XDG_RUNTIME_DIR if running in userNS.
+// Otherwise returns os.Getuid() .
+func detectUID() (int, error) {


I'm hoping you wouldn't need this... But looks like you still do for the external auth of the user D-Bus connection... Let me think about this one see if there's a better solution than this.

Having said that, see my comment below on requiring that the environment variables are properly set. So you could say that this is a hard requirement here. Looks like your code will fail if it doesn't match /run/user/%d which I think is a good thing.

libsystemd (which is used by crun) seems extracting UID from the "0::/user.slice/user-%d.slice/..." string in /proc/self/cgroup:
https://github.com/systemd/systemd/blob/6829d8ce6927c151e4391c6a5e69b76906fbd2be/src/libsystemd/sd-bus/sd-bus.c#L104
https://github.com/systemd/systemd/blob/ce4c73eb5d3c9a23628c450db0e25a529620adbe/src/basic/cgroup-util.c#L1361-L1395

I feel this seems less robust than $XDG_RUNTIME_DIR, but maybe we should do this?

I'm 👍 for path.Base(xdr) proposed by @kolyshkin #2281 (comment)

Yet another way is to exec busctl --user status and grep ^OwnerUID=

busctl --user status ends up resolving it from getsockopt(fd, SOL_SOCKET, SO_PEERCRED, ...), see here.

I believe you can do the same in Golang with unix.GetsockoptUcred(int(fd), unix.SOL_SOCKET, unix.SO_PEERCRED) from the "golang.org/x/sys/unix" package. Does that work?

libsystemd (which is used by crun) seems extracting UID from the "0::/user.slice/user-%d.slice/..." string in /proc/self/cgroup:

Yes, I think that's possibly reasonable. It's still reimplementing C code in Golang but I think that's preferrable than some of the alternatives.

But see my message above about getting the real UID from a SO_PEERCRED call...

busctl --user status ends up resolving it from getsockopt(fd, SOL_SOCKET, SO_PEERCRED, ...), see here.

Seems not true for OwnerUID. OwnerUID seems extracted from cgroup.
https://github.com/systemd/systemd/blob/v245/src/libsystemd/sd-bus/bus-dump.c#L375-L377
https://github.com/systemd/systemd/blob/v245/src/libsystemd/sd-bus/bus-creds.c#L509-L526

Anyway, SO_PEERCRED isn't what we want here, because the kernel automatically translates the UID in the ucred struct.

Updated PR to use busctl --user status. I think this is the most robust way.

I guess eventually systemd may switch away from parsing cgroup and adopt more robust logic , but we won't be affected as long as we use busctl.

Ah yes good point sorry my search was too shallow for it...

I think I just worry a bit about busctl not being available, it was first introduced as a debugging tool so I can see how distributions might not want to ship it or not ship it by default and it might go missing in the future...

Anyways, it's a fallback and we can iterate on it. Looks good.

filbranden · 2020-04-02T16:18:32Z

LGTM

kolyshkin

LGTM

kolyshkin · 2020-04-04T22:49:44Z

@AkihiroSuda I have a WIP progress patchset to remove Paths map from the systemd UnifiedManager, and simplify its initialization (meaning, for example, that you won't have to add Rootless as exported field). I'm not ready to create a PR yet but it looks like adding rootless support on top of my patch will be very easy. Can you please take a look?

Again, it's not ready yet but you can get the idea of what I'm trying to do:
https://github.com/kolyshkin/runc/commits/fs2-init-ctrl

AkihiroSuda · 2020-04-04T23:02:11Z

@kolyshkin SGTM, do you want your commits to be merged before this PR or after?

kolyshkin · 2020-04-05T00:34:16Z

@AkihiroSuda I think yours will be easier to port on top of mine than vise versa

AkihiroSuda · 2020-04-18T22:49:24Z

If #2299 takes longer, can we merge this PR first?

kolyshkin · 2020-04-19T15:36:47Z

If #2299 takes longer, can we merge this PR first?

Sorry for the delay, it is ready now, and it will definitely be easier to work with the updated/refactored codebase.

kolyshkin · 2020-04-21T22:09:43Z

Need a rebase

AkihiroSuda · 2020-04-21T22:28:27Z

Needs #2340

AkihiroSuda · 2020-04-28T23:57:51Z

CI green. Ready to review/merge.

@opencontainers/runc-maintainers

tests/rootless.sh

tests/integration/cgroups.bats

AkihiroSuda · 2020-05-03T03:24:20Z

@kolyshkin @mrunalp PTAL

libcontainer/cgroups/systemd/user.go

AkihiroSuda · 2020-05-03T21:38:29Z

rebased

AkihiroSuda · 2020-05-06T21:04:12Z

@kolyshkin LGTY?

libcontainer/cgroups/systemd/user.go

libcontainer/cgroups/systemd/v2.go

kolyshkin · 2020-05-08T02:29:23Z

Left a few nits, but can be addressed as a followup

LGTM

AkihiroSuda · 2020-05-08T03:11:14Z

addressed comments

kolyshkin · 2020-05-08T03:30:30Z

still LGTM

kolyshkin · 2020-05-08T03:34:14Z

LGTM (for pullapprove)

Tested with both Podman (master) and Moby (master), on Ubuntu 19.10 . $ podman --cgroup-manager=systemd run -it --rm --runtime=runc \ --cgroupns=host --memory 42m --cpus 0.42 --pids-limit 42 alpine / # cat /proc/self/cgroup 0::/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope / # cat /sys/fs/cgroup/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope/memory.max 44040192 / # cat /sys/fs/cgroup/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope/cpu.max 42000 100000 / # cat /sys/fs/cgroup/user.slice/user-1001.slice/[email protected]/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope/pids.max 42 Signed-off-by: Akihiro Suda <[email protected]>

AkihiroSuda · 2020-05-08T03:51:47Z

rebased

mrunalp · 2020-05-08T03:53:54Z

LGTM

kolyshkin · 2020-05-08T04:45:26Z

LGTM

This was referenced Apr 1, 2020

cgroup2: does not work with rootless podman #2163

Closed

cgroup2: TODO list #2209

Closed

rootless: support --exec-opt native.cgroupdriver=systemd moby/moby#40486

Merged

giuseppe reviewed Apr 1, 2020

View reviewed changes

libcontainer/cgroups/systemd/user.go Outdated Show resolved Hide resolved

kolyshkin requested changes Apr 1, 2020

View reviewed changes

filbranden reviewed Apr 1, 2020

View reviewed changes

AkihiroSuda force-pushed the rootless-systemd branch 5 times, most recently from d5e4e38 to 159eecf Compare April 2, 2020 08:28

AkihiroSuda added the enhancement label Apr 3, 2020

kolyshkin approved these changes Apr 4, 2020

View reviewed changes

kolyshkin mentioned this pull request Apr 7, 2020

cgroupv2: fix fs2 driver initialization #2299

Merged

kolyshkin mentioned this pull request Apr 15, 2020

cgroupv2 support meta issue #2315

Closed

kolyshkin added the area/rootless label Apr 21, 2020

AkihiroSuda mentioned this pull request Apr 21, 2020

fs2: fix cgroup.subtree_control EPERM on rootless + add CI #2340

Merged

AkihiroSuda mentioned this pull request Apr 21, 2020

cgroup2: use "systemd" cgroup driver by default when available moby/moby#40846

Merged

AkihiroSuda added area/cgroupv2 area/systemd labels Apr 24, 2020

AkihiroSuda force-pushed the rootless-systemd branch from 159eecf to cfbcc3c Compare April 28, 2020 07:22

kolyshkin reviewed Apr 29, 2020

View reviewed changes

tests/rootless.sh Outdated Show resolved Hide resolved

AkihiroSuda force-pushed the rootless-systemd branch 2 times, most recently from 41faeb4 to 7e37c19 Compare April 29, 2020 05:07

h-vetinari reviewed Apr 29, 2020

View reviewed changes

tests/integration/cgroups.bats Outdated Show resolved Hide resolved

h-vetinari reviewed Apr 29, 2020

View reviewed changes

tests/integration/cgroups.bats Outdated Show resolved Hide resolved

AkihiroSuda force-pushed the rootless-systemd branch 2 times, most recently from fbc0cc7 to a251537 Compare April 29, 2020 07:25

h-vetinari mentioned this pull request Apr 29, 2020

Release rc11 #2364

Closed

mrunalp reviewed May 3, 2020

View reviewed changes

libcontainer/cgroups/systemd/user.go Show resolved Hide resolved

AkihiroSuda force-pushed the rootless-systemd branch from a251537 to 07bdb3e Compare May 3, 2020 21:38

kolyshkin reviewed May 8, 2020

View reviewed changes

libcontainer/cgroups/systemd/user.go Outdated Show resolved Hide resolved

kolyshkin reviewed May 8, 2020

View reviewed changes

libcontainer/cgroups/systemd/v2.go Show resolved Hide resolved

kolyshkin reviewed May 8, 2020

View reviewed changes

libcontainer/cgroups/systemd/v2.go Outdated Show resolved Hide resolved

AkihiroSuda force-pushed the rootless-systemd branch 2 times, most recently from 5552602 to e8389d3 Compare May 8, 2020 03:10

AkihiroSuda force-pushed the rootless-systemd branch from e8389d3 to bf15cc9 Compare May 8, 2020 03:51

kolyshkin merged commit 2b31437 into opencontainers:master May 8, 2020

huanghongxun mentioned this pull request Mar 10, 2021

Support rootless cgroup v2 kata-containers/cgroups-rs#42

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cgroup v2: support rootless systemd #2281

cgroup v2: support rootless systemd #2281

AkihiroSuda commented Apr 1, 2020 •

edited

Loading

AkihiroSuda commented Apr 1, 2020

kolyshkin left a comment

filbranden left a comment

filbranden Apr 1, 2020

AkihiroSuda Apr 1, 2020

AkihiroSuda Apr 1, 2020

AkihiroSuda Apr 2, 2020 •

edited

Loading

filbranden Apr 2, 2020

filbranden Apr 2, 2020

AkihiroSuda Apr 2, 2020

AkihiroSuda Apr 2, 2020

filbranden Apr 2, 2020

filbranden commented Apr 2, 2020

kolyshkin left a comment

kolyshkin commented Apr 4, 2020

AkihiroSuda commented Apr 4, 2020

kolyshkin commented Apr 5, 2020

AkihiroSuda commented Apr 18, 2020

kolyshkin commented Apr 19, 2020

kolyshkin commented Apr 21, 2020

AkihiroSuda commented Apr 21, 2020

AkihiroSuda commented Apr 28, 2020

AkihiroSuda commented May 3, 2020

AkihiroSuda commented May 3, 2020

AkihiroSuda commented May 6, 2020

kolyshkin commented May 8, 2020 •

edited by AkihiroSuda

Loading

AkihiroSuda commented May 8, 2020

kolyshkin commented May 8, 2020

kolyshkin commented May 8, 2020 •

edited by AkihiroSuda

Loading

AkihiroSuda commented May 8, 2020

mrunalp commented May 8, 2020 •

edited by AkihiroSuda

Loading

kolyshkin commented May 8, 2020 •

edited by AkihiroSuda

Loading

cgroup v2: support rootless systemd #2281

cgroup v2: support rootless systemd #2281

Conversation

AkihiroSuda commented Apr 1, 2020 • edited Loading

AkihiroSuda commented Apr 1, 2020

kolyshkin left a comment

Choose a reason for hiding this comment

filbranden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda Apr 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filbranden commented Apr 2, 2020

kolyshkin left a comment

Choose a reason for hiding this comment

kolyshkin commented Apr 4, 2020

AkihiroSuda commented Apr 4, 2020

kolyshkin commented Apr 5, 2020

AkihiroSuda commented Apr 18, 2020

kolyshkin commented Apr 19, 2020

kolyshkin commented Apr 21, 2020

AkihiroSuda commented Apr 21, 2020

AkihiroSuda commented Apr 28, 2020

AkihiroSuda commented May 3, 2020

AkihiroSuda commented May 3, 2020

AkihiroSuda commented May 6, 2020

kolyshkin commented May 8, 2020 • edited by AkihiroSuda Loading

AkihiroSuda commented May 8, 2020

kolyshkin commented May 8, 2020

kolyshkin commented May 8, 2020 • edited by AkihiroSuda Loading

AkihiroSuda commented May 8, 2020

mrunalp commented May 8, 2020 • edited by AkihiroSuda Loading

kolyshkin commented May 8, 2020 • edited by AkihiroSuda Loading

AkihiroSuda commented Apr 1, 2020 •

edited

Loading

AkihiroSuda Apr 2, 2020 •

edited

Loading

kolyshkin commented May 8, 2020 •

edited by AkihiroSuda

Loading

kolyshkin commented May 8, 2020 •

edited by AkihiroSuda

Loading

mrunalp commented May 8, 2020 •

edited by AkihiroSuda

Loading

kolyshkin commented May 8, 2020 •

edited by AkihiroSuda

Loading