Using per cgroup manager dbus connection instances means
that every cgroup manager instance gets a new connection,
and those connections are never closed, ultimately resulting
in file descriptors limit being hit.
Revert back to using a single global dbus connection for everything,
without changing the callers.
NOTE that it is assumed a runtime can't use both root and rootless
dbus at the same time. If this happens, we panic.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 47ef9a104f forgot to wrap GetManagerProperty("ControlGroup")
into retryOnDisconnect. Since there's one other user of
GetManagerProperty, add getManagerProperty wrapper and use it.
Fixes: 47ef9a104f
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
A cgroup manager's Set method sets cgroup resources, but historically
it was accepting configs.Cgroups.
Refactor it to accept resources only. This is an improvement from the
API point of view, as the method can not change cgroup configuration
(such as path to the cgroup etc), it can only set (modify) its
resources/limits.
This also lays the foundation for complicated resource updates, as now
Set has two sets of resources -- the one that was previously specified
during cgroup manager creation (or the previous Set), and the one passed
in the argument, so it could deduce the difference between these. This
is a long term goal though.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For some reason, systemd cgroup v2 driver's Set is not using its
container argument when generating systemd unit properties.
This bug is not detected by our update tests as we run a new binary
every time and thus a new instance of a cgroup manager.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Instead of reconnecting to dbus after some failed operations, and
returning an error (so a caller has to retry), reconnect AND retry
in place for all such operations.
This should fix issues caused by a stale dbus connection after e.g.
a dbus daemon restart.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
[@kolyshkin: doc nits, use dbus.ErrClosed and isDbusError]
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Generalize isUnitExists as isDbusError, and use errors.As while at it
(which can handle wrapped errors as well).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 5d0ffbf9c8 added OOM kill count checking and better container
start/run/exec error reporting in case we hit OOM.
It also introduced warnings like these:
> level=warning msg="unable to get oom kill count" error="openat2
> /sys/fs/cgroup/user.slice/user-1000.slice/user@1000.service/test_hello/memory.events:
> no such file or directory"
In case of rootless containers, unless cgroup is delegated or systemd is
used, runc can not create a cgroup and thus it fails to get OOM kill
count. This is expected, and the warning should not be shown in this
case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is somewhat radical approach to deal with kernel memory.
Per-cgroup kernel memory limiting was always problematic. A few
examples:
- older kernels had bugs and were even oopsing sometimes (best example
is RHEL7 kernel);
- kernel is unable to reclaim the kernel memory so once the limit is
hit a cgroup is toasted;
- some kernel memory allocations don't allow failing.
In addition to that,
- users don't have a clue about how to set kernel memory limits
(as the concept is much more complicated than e.g. [user] memory);
- different kernels might have different kernel memory usage,
which is sort of unexpected;
- cgroup v2 do not have a [dedicated] kmem limit knob, and thus
runc silently ignores kernel memory limits for v2;
- kernel v5.4 made cgroup v1 kmem.limit obsoleted (see
https://github.com/torvalds/linux/commit/0158115f702b).
In view of all this, and as the runtime-spec lists memory.kernel
and memory.kernelTCP as OPTIONAL, let's ignore kernel memory
limits (for cgroup v1, same as we're already doing for v2).
This should result in less bugs and better user experience.
The only bad side effect from it might be that stat can show kernel
memory usage as 0 (since the accounting is not enabled).
[v2: add a warning in specconv that limits are ignored]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Moving these utilities to a separate package, so that consumers of this
package don't have to pull in the whole "system" package.
Looking at uses of these utilities (outside of runc itself);
`RunningInUserNS()` is used by [various external consumers][1],
so adding a "Deprecated" alias for this.
[1]: https://grep.app/search?current=2&q=.RunningInUserNS
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This unconditionally enables TasksAccounting for systemd unified (v2)
cgroup driver, making it work the same way as the legacy (v1) driver.
Practically, it is probably a no-op since DefaultTasksAccounting is
usually true.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In some cases, container init fails to start because it is killed by
the kernel OOM killer. The errors returned by runc in such cases are
semi-random and rather cryptic. Below are a few examples.
On cgroup v1 + systemd cgroup driver:
> process_linux.go:348: copying bootstrap data to pipe caused: write init-p: broken pipe
> process_linux.go:352: getting the final child's pid from pipe caused: EOF
On cgroup v2:
> process_linux.go:495: container init caused: read init-p: connection reset by peer
> process_linux.go:484: writing syncT 'resume' caused: write init-p: broken pipe
This commits adds the OOM method to cgroup managers, which tells whether
the container was OOM-killed. In case that has happened, the original error
is discarded (unless --debug is set), and the new OOM error is reported
instead:
> ERRO[0000] container_linux.go:367: starting container process caused: container init was OOM-killed (memory limit too low?)
Also, fix the rootless test cases that are failing because they expect
an error in the first line, and we have an additional warning now:
> unable to get oom kill count" error="no directory specified for memory.oom_control
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
All cgroup managers has Apply() and Set() methods:
- Apply is used to create a cgroup (and, in case of systemd,
a systemd unit) and/or put a PID into the cgroup (and unit);
- Set is used to set various cgroup resources and limits.
The fs/fs2 cgroup manager implements the functionality as described above.
The systemd v1/v2 manager deviate -- it sets *most* of cgroup limits
(those that can be projected to systemd unit properties) in Apply(),
and then again *all* cgroup limits in Set (first indirectly via systemd
properties -- same as in Apply, then via cgroupfs).
This commit removes setting the cgroup limits from Apply,
so now the systemd manager behaves the same way as the fs manager.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
A recent commit a35cad3b22 added warnings about systemd being too
old. While those warnings are valid, they break some existing tests,
and also don't add much value to a user (IOW no one is going to upgrade
systemd because runc says it's old).
Demote those to warnings.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Add a check to unifiedResToSystemdProps that systemd is recent enough
to support AllowedCPUs/AllowedMemoryNodes unit properties, and skip
setting the property if it is not supported.
Note that this is not an error as the setting is still applied to
the underlying cgroupfs -- it's just systemd unit property that is
being skipped.
2. In all the places we skip an unsupported property, warn about it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Support for systemd properties AllowedCPUs and AllowedMemoryNodes
was added by commit 13afa58d0e, but only for unified resources
of systemd v2 driver.
This adds support for Cpu.Cpus and Cpu.Mems resources to
both systemd v1 and v2 cgroup drivers.
An integration test is added to check that the settings work.
[v2: check for systemd version]
[v3: same in the test]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As the caller of this function just logs the error, it does not make
sense to pass it. Instead, log it (once) and return -1.
This is a preparation for the second user.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Move the Device-related types to libcontainer/devices, so that
the package can be used in isolation. Aliases have been created
in libcontainer/configs for backward compatibility.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
* cpuset.cpus -> AllowedCPUs
* cpuset.mems -> AllowedMemoryNodes
No test for cgroup v2 resources.unified override, as this requires a
separate test case, and all the unified resources are handled uniformly
so there's little sense to test all parameters.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case systemd is used as cgroups manager, and a user sets some
resources using unified resource map (as per [1]), systemd is not
aware of any parameters, so there will be a discrepancy between
the cgroupfs state and systemd unit state.
Let's try to fix that by converting known unified resources to systemd
properties.
Currently, this is only implemented for pids.max as a POC.
Some other parameters (that might or might not have systemd unit
property equivalents) are:
$ ls -l | grep w-
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.freeze
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.max.depth
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.max.descendants
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.procs
-rw-r--r--. 1 root root 0 Oct 21 09:43 cgroup.subtree_control
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.threads
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.type
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpu.max
-rw-r--r--. 1 root root 0 Oct 10 13:57 cpu.pressure
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpuset.cpus
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpuset.cpus.partition
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpuset.mems
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpu.weight
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpu.weight.nice
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.1GB.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.1GB.rsvd.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.2MB.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.2MB.rsvd.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.bfq.weight
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.latency
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.max
-rw-r--r--. 1 root root 0 Oct 10 13:57 io.pressure
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.weight
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.high
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.low
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.max
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.min
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.oom.group
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.pressure
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.swap.high
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.swap.max
Surely, it is a manual conversion for every such case...
[1] https://github.com/opencontainers/runtime-spec/pull/1040
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 27d3dd3df3 ("don't fail when subsystem not mounted") added
ignoring "not found" error to enableKmem, and as a result the function
now tries to call Mkdir with an empty path, which results in a weird
error message. For example, this is a failure from a
libcontainer/integration test:
> === RUN TestRunWithKernelMemorySystemd
> exec_test.go:704: runContainer failed with kernel memory limit: container_linux.go:370: starting container process caused: process_linux.go:327: applying cgroup configuration for process caused: mkdir : no such file or directory
I am not entirely sure if it is a good idea to silently ignore set
limits, but at least let's fix the error handling.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit a1d5398afa ("Respect container's cgroup path") added a
cgroupPath argument to FindCgroupMountpoint to make runc/libcontainer
work in a custom multitenant environment with multiple cgroup mount
points.
It also added passing c.Path as an argument to FindCgroupMountpoint
for systemd (v1) controller. This is wrong, because
1. systemd controller do not use c.Path at all (and c.Path is never set
by specconv) -- instead, it uses Name and Parent.
2. c.Path, if set, is not absolute -- it is relative to /sys/fs/cgroup
-- but it is used as an absolute path here.
Since c.Path is never set, the change did not result in any breakage, so
this code sit quietly for some time and the issue might not have been
discovered -- until we started running libcontainer/integration tests
in a CentOS 7 VM, which resulted in a following weird error:
> FAIL: TestPidsSystemd: utils_test.go:55: exec_test.go:630: unexpected error: container_linux.go:353: starting container process caused: process_linux.go:326: applying cgroup configuration for process caused: mountpoint for devices not found
The error was "fixed" in commit f57bb2fe3d by changing the tests'
cgroups Path to be "/sys/fs/cgroup/". This actually resulted in
creation of cgroup directories like /sys/fs/cgroup/memory/sys/fs/cgroup,
/sys/fs/cgroup/devices/sys/fs/cgroup and so on.
The proper fix to the test case is implemented in the previous commit,
which sets c.Name and c.Parent.
This commit just removes the invalid use of c.Path, and tells the whole
story.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case it takes more than 1 second for systemd to create a unit,
startUnit() times out with a warning and then runc proceeds
(to create cgroups using fs manager and so on).
Now runc and systemd are racing, and multiple scenarios are possible.
In one such scenario, by the time runc calls systemd manager's Apply()
the unit is not yet created, the dbusConnection.SetUnitProperties()
call fails with "unit xxx.scope not found", and the whole container
start also fails.
To eliminate the race, we need to return an error in case the timeout is
hit.
To reduce the chance to fail, increase the timeout from 1 to 30 seconds,
to not error out too early on a busy/slow system (and times like 3-5
seconds are not unrealistic).
While at it, as the timeout is quite long now, make sure to not leave
a stray timer.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In all these cases, getSubsystemPath() was already called, and its
result stored in m.paths map. It makes no sense to not reuse it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin