libct/cgroups/fs: rm getCgroupRoot #2497
Conversation
|
Do we have this officially in OCI Spec? Wondering this might break some embedded systems (if the previous code was actually functional with a strange mount point) |
I was not able to find anything. I guess it is considered to be an implementation detail. The closest thing I was able to find is
So, I consider this is an implementation detail and should not be in the spec.
It might. For example, Android mounts various v1 cgroups to I had an alternative to this patch -- that is, try the default (/sys/fs/cgroup) first and fall back to mountinfo parsing it if failed. |
|
I traced the introduction of this way to get a cgroup root directory down to this commit: moby/moby@c442586#diff-4ecfc3616c252e4c65bc87c539d9a459L18 Alas, it does not tell us why. But let me work on a less radical alternative. |
All the test cases are doing the same checks, only input differs, so we can unify those using a test data table. While at it: - use t.Fatalf where it makes sense (no further checks are possible); - remove the "XXX" comments as we won't get rid of cgroup Name/Parent. PS I tried using t.Parallel() as well but it did not result in any noticeable speedup, so I dropped it for simplicity. Signed-off-by: Kir Kolyshkin <[email protected]>
I think this was added during the dark ages where cgroupfs can be mounted to /cgroup, /dev/cgroup, or some other path. Now it have finally settled on /sys/fs/cgroup, so let's remove the guessing and use the standard path. Signed-off-by: Kir Kolyshkin <[email protected]>
Please see #2507 |
kolyshkin
force-pushed
the
rm-get-cgroup-root
branch
from
7cf9a0d to
1cb24ac
Compare
|
Rebased to resolve merge conflicts |
|
As much as I like this one, #2507 is now merged so let's close this one. Maybe revisit it in 5 years? :) |
2 participants
this is separated out from #2438 in order to make review easier
I think this was added during the dark ages where cgroupfs can be
mounted to /cgroup, /dev/cgroup, or some other path. Now it have
finally settled on /sys/fs/cgroup, so let's remove the guessing
and use the standard path.
Please see individual commits for details.
If you think this one is too extreme (because e.g. there are systems on which cgroup v1 mounts are not under
/sys/fs/cgroupbut some other path), please let us know, and see the alternative PR (#2507).