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.

Sign up for GitHub

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

Jump to bottom

Fix systemd cgroup driver's Apply #3782

Merged
merged 4 commits into from
Apr 3, 2023
Merged

Fix systemd cgroup driver's Apply #3782

merged 4 commits into from
Apr 3, 2023

Commits on Mar 31, 2023

  1. libct/cg/sd: refactor startUnit 

    Move error handling earlier, removing "if err == nil" block.

No change of logic.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed Mar 31, 2023
    Configuration menu
    Copy the full SHA
    c6e8cb7 View commit details
    Browse the repository at this point in the history

  2. libct/cg/sd: ignore UnitExists only for Apply(-1) 

    Commit d223e2a ("Ignore error when starting transient unit
that already exists" modified the code handling errors from startUnit
to ignore UnitExists error.

Apparently it was done so that kubelet can create the same pod slice
over and over without hitting an error (see [1]).

While it works for a pod slice to ensure it exists, it is a gross bug
to ignore UnitExists when creating a container. In this case, the
container init PID won't be added to the systemd unit (and to the
required cgroup), and as a result the container will successfully
run in a current user cgroup, without any cgroup limits applied.

So, fix the code to only ignore UnitExists if we're not adding a process
to the systemd unit. This way, kubelet will keep working as is, but
runc will refuse to create containers which are not placed into a
requested cgroup.

[1] opencontainers#1124

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed Mar 31, 2023
    Configuration menu
    Copy the full SHA
    c253342 View commit details
    Browse the repository at this point in the history

  3. libct/cg/sd: reset-failed and retry startUnit on UnitExists 

    In case a systemd unit fails (for example, timed out or OOM-killed),
systemd keeps the unit. This prevents starting a new container with
the same systemd unit name.

The fix is to call reset-failed in case UnitExists error is returned,
and retry once.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed Mar 31, 2023
    Configuration menu
    Copy the full SHA
    1d18743 View commit details
    Browse the repository at this point in the history

  4. runc run: refuse a non-empty cgroup 

    Commit d08bc0c ("runc run: warn on non-empty cgroup") introduced
a warning when a container is started in a non-empty cgroup. Such
configuration has lots of issues.

In addition to that, such configuration is not possible at all when
using the systemd cgroup driver.

As planned, let's promote this warning to an error, and fix the test
case accordingly.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed Mar 31, 2023
    Configuration menu
    Copy the full SHA
    82bc89c View commit details
    Browse the repository at this point in the history