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>
We call joinCgroups() from Apply, and in there we iterate through the
list of subsystems, calling getSubsystemPath() for each. This is
expensive, since every getSubsystemPath() involves parsing mountinfo.
At the end of Apply(), we iterate through the list of subsystems to fill
the m.paths, again calling getSubsystemPath() for every subsystem.
As a result, we parse mountinfo about 20 times here.
Let's find the paths first and reuse m.paths in joinCgroups().
While at it, since join() is just two calls now, inline it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When paths are set, we only need to place the PID into proper
cgroups, and we do know all the paths already.
Both fs/d.path() and systemd/v1/getSubsystemPath() parse
/proc/self/mountinfo, and the only reason they are used
here is to check whether the subsystem is available.
Use a much simpler/faster check instead.
Frankly, I am not sure why the check is needed at all. Maybe it should
be dropped.
Also, for fs driver, since d is no longer used in this code path,
move its initialization to after it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Instead of iterating over m.paths, iterate over subsystems and look up
the path for each. This is faster since a map lookup is faster than
iterating over the names in Get. A quick benchmark shows that the new
way is 2.5x faster than the old one.
Note though that this is not done to make things faster, as savings are
negligible, but to make things simpler by removing some code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
RemovePaths() deletes elements from the paths map for paths that has
been successfully removed.
Although, it does not empty the map itself (which is needed that AFAIK
Go garbage collector does not shrink the map), but all its callers do.
Move this operation from callers to RemovePaths.
No functional change, except the old map should be garbage collected now.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
some hierarchies were created directly by .Apply() on top of systemd
managed cgroups. systemd doesn't manage these and as a result we leak
these cgroups.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Iterating over the list of subsystems and comparing their names to get an
instance of fs.cgroupFreezer is useless and a waste of time, since it is
a shallow type (i.e. does not have any data/state) and we can create an
instance in place.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The kubelet uses libct/cgroups code to set up cgroups. It creates a
parent cgroup (kubepods) to put the containers into.
The problem (for cgroupv2 that uses eBPF for device configuration) is
the hard requirement to have devices cgroup configured results in
leaking an eBPF program upon every kubelet restart. program. If kubelet
is restarted 64+ times, the cgroup can't be configured anymore.
Work around this by adding a SkipDevices flag to Resources.
A check was added so that if SkipDevices is set, such a "container"
can't be started (to make sure it is only used for non-containers).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
there have been cases observed where instead of `v$VER.0-$OS` the systemdVersion returned is just `$VER`, or `$VER-1`.
handle these cases
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Qiang Huang