runc run/create: refuse non-empty cgroup; runc exec: refuse frozen cgroup

[kolyshkin]

OK, this is being split into a few more digestible PRs:

• runc cli: nits #3214
• cgroupv1: refactor and optimize #3215
• cgroup: make paths available, simplify getting manager #3216
• runc delete -f: fix for cg v1 + paused container #3217
• run create/run/exec: refuse bad cgroup #3223

I. cgroup manager refactoring

Very high level overview:

• allow cgroup instantiation (NewManager) to return errors;
• initialize cgroup paths during instantiation, so paths are always available
  (and now we can use a new instance for e.g. Destroy or Exists,
  which was not possible before);
• simplifies cgroup manager instantiation, replacing the complicated
  logic of choosing a cgroup manager with a simple manager.New() call.
• fixes some panics in cgroup manager (with Resources==nil).

This helps further commits, and makes cgroup manager easier to use
from e.g. Kubernetes.

Fixes: #3177
Fixes: #3178

II. runc run/create: refuse non-empty cgroup

Currently runc allows multiple containers to share the same cgroup (for
example, by having the same cgroupPath in config.json). While such
shared configuration might be OK, there are some issues:

• When each container has its own resource limits, the order of
  containers start determines whose limits will be effectively applied.

• When one of containers is paused, all others are paused, too.

• When a container is paused, any attempt to do runc create/run/exec
  end up with runc init stuck inside a frozen cgroup.

• When a systemd cgroup manager is used, this becomes even worse -- such
  as, stop (or even failed start) of any container results in
  "stopTransientUnit" command being sent to systemd, and so (depending
  on unit properties) other containers can receive SIGTERM, be killed
  after a timeout etc.

All this may lead to various hard-to-debug situations in production
(runc init stuck, cgroup removal error, wrong resource limits, container
init not working as zombie reaper, etc).

One obvious solution is to require a non-existent or empty cgroup
for the new container, and fail otherwise.

This is exactly what this PR implements.

III. runc delete -f: don't hang on a paused container

runc delete -f used to hang if a container is paused (on cgroup v1).
Fix this, add a test case.

Fixes: #3134

IV. runc exec: refuse paused container

This bugged me a few times during runc development. A new container is
run, and suddenly runc init is stuck, 🎶 and nothing ever happens, and
I wonder... 🎶

Figuring out that the cause of it is (pre-created) frozen cgroup is not
very obvious (to say at least).

The fix is to add a check that refuses to exec in a paused container.
A (very bad) alternative to that would be to thaw the cgroup.

Implement the fix, add a test case.

V. runc run: refuse cgroup if frozen

Sometimes a container cgroup already exists but is frozen.
When this happens, runc init hangs, and it's not clear what is going on.

Refuse to run in a frozen cgroup; add a test case.

---

Fixes: #3132

Proposed changelog entry

libcontainer API changes:
* New configs.Cgroup structure fields (#3177):
  * Systemd (whether to use systemd cgroup manager);
  * Rootless (whether to use rootless cgroups).
* New cgroups/manager package aiming to simplify cgroup manager instantiation. (#3177)
* All cgroup managers' instantiation methods now initialize cgroup paths and can return errors.
  This allows to use any cgroup manager method (e.g. Exists, Destroy, Set, GetStats) right after
  instantiation, which was not possible before (as paths were initialized in Apply only). (#3178)
Bugfixes:
* runc delete -f now succeeds (rather than timeouts) on a paused container (#3134)
* runc exec now refuses a paused container (#3132)
* runc run/start now refuses to start a container in a non-empty cgroup (#3132)
* runc run/start now refuses to start a container if its cgroup is frozen (#3132)

[kolyshkin]

OK it seems that I have to split this one into a few smaller PRs. Alas, the biggest part is cgroup manager refactoring which seemingly can't be split.

[cyphar]

Regarding this part:

II. runc run/create: refuse non-empty cgroup

A long time ago there were discussions of adding docker run --cgroup=other-container. Was this feature never implemented / are we really sure nobody uses the shared-cgroup "feature" we have at the moment?

[kolyshkin]

Regarding this part:

II. runc run/create: refuse non-empty cgroup

A long time ago there were discussions of adding docker run --cgroup=other-container. Was this feature never implemented / are we really sure nobody uses the shared-cgroup "feature" we have at the moment?

I've found traces of this when working on #3136, and yes, this seems to be not implemented. Looking into moby/moby#18654 it seems that it was suggested to use a common cgroup parent for containers instead of this (ultimately this is what the "pod" concept, introduced later, does).

Yet I don't know if anyone actually shares a cgroup for multiple containers in the wild, and if yes, it can be a breaking change. I still think it is needed as such setups introduce a number of issues, outlined above as well as in #3132, and are therefore broken.

I guess unless we try this out we'll never know if this is used in the wild, and how often. In case this is used often, we can revert, in case it's used rarely, we can add a mechanism to skip the check (an env var, a flag file, or something).

[cyphar]

@kolyshkin

Yet I don't know if anyone actually shares a cgroup for multiple containers in the wild, and if yes, it can be a breaking change. I still think it is needed as such setups introduce a number of issues, outlined above as well as in #3132, and are therefore broken.

Yeah I agree the fact we haven't gotten any bug reports about how broken it is leads me to believe nobody is using it.

[kolyshkin]

Finally split into #3214, #3215, #3216, #3217, #3223, so this one can finally be closed now :)