libct/cg: rm dead code to improve clarity#3136
Conversation
This was initially added by commits 41d9d26 and 4a8f0b4, apparently to implement docker run --cgroup container:ID, which was never merged. Therefore, this code is not and was never used. It needs to be removed mainly because having it makes it much harder to understand how cgroup manager works (because with this in place we have not one or two but three sets of cgroup paths to think about). Note if the paths are known and there is a need to add a PID to existing cgroup, cgroup manager is not needed at all -- something like cgroups.WriteCgroupProc or cgroups.EnterPid is sufficient (and the latter is what runc exec uses in (*setnsProcess).start). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
@opencontainers/runc-maintainers PTAL. This removes the code that's not used but makes a huge roadblock preventing cgroup manager code understanding (at least this is the way it was for me), so its removal is quite important (if we want more people to work on runc). |
hqhq
left a comment
There was a problem hiding this comment.
LGTM.
I remembered --cgroup:container:<id> was a useful feature needed by some projects, I'm surprised it's never merged. 😂
@hqhq In terms of runc and/or OCI spec, specifying OTOH doing this creates some other problems -- see e.g. #3132 |
This was initially added by commits 41d9d26 and 4a8f0b4,
apparently to implement
docker run --cgroup container:ID(moby/moby#19244),which was never merged. Therefore, this code is not and was never used.
It needs to be removed mainly because having it makes it much harder to
understand how cgroup manager works (because with this in place we have
not one or two but three sets of cgroup paths to think about).
Note if the paths are known and there is a need to add a PID to existing
cgroup, cgroup manager is not needed at all -- something like
cgroups.WriteCgroupProcorcgroups.EnterPidis sufficient (and thelatter is what
runc execuses in(*setnsProcess).start). If, for some reason,the cgroup manager is still needed, it's better to write a separate shallow
implementation that would only do
WriteCgroupProc/EnterPidfromApply,and have other methods empty.