Burstable CFS controller is introduced in Linux 5.14. This helps with
parallel workloads that might be bursty. They can get throttled even
when their average utilization is under quota. And they may be latency
sensitive at the same time so that throttling them is undesired.
This feature borrows time now against the future underrun, at the cost
of increased interference against the other system users, by introducing
cfs_burst_us into CFS bandwidth control to enact the cap on unused
bandwidth accumulation, which will then used additionally for burst.
The patch adds the support/control for CFS bandwidth burst.
runtime-spec: https://github.com/opencontainers/runtime-spec/pull/1120
Co-authored-by: Akihiro Suda <suda.kyoto@gmail.com>
Co-authored-by: Nadeshiko Manju <me@manjusaka.me>
Signed-off-by: Kailun Qin <kailun.qin@intel.com>
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix.
Thus, these annotations are no longer necessary. Hooray!
[1] https://github.com/polyfloyd/go-errorlint/pull/47
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
A few cases relied on the fact that systemd is used, and thus
/sys/fs/cgroup/user.slice is available.
Guess what, in case of "make unittest" it might not be.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Split the test into two -- for fs and systemd cgroup managers, and only
run the second one if systemd is available.
Prevents the following failure during `make unittest`:
> === RUN TestNilResources
> manager_test.go:27: systemd not running on this host, cannot use systemd cgroups manager
> --- FAIL: TestNilResources (0.22s)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This aligns v2 usage calculations more closely with v1.
Current node-level reporting for v1 vs v2 on the same
machine under similar load may differ by ~250-750Mi.
Also return usage as combined swap + memory usage, aligned
with v1 and non-root v2 cgroups.
`mem_cgroup_usage` in the kernel counts NR_FILE_PAGES
+ NR_ANON_MAPPED + `nr_swap_pages` (if swap enabled) [^0].
Using total - free results in higher "usage" numbers.
This is likely due to various types of reclaimable
memory technically counted as in use (e.g. inactive anon).
See also https://github.com/kubernetes/kubernetes/issues/118916 for more context
[^0]: https://github.com/torvalds/linux/blob/06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5/mm/memcontrol.c#L3673-L3680
Signed-off-by: Alexander Eldeib <alexeldeib@gmail.com>
Since systemd v240 (commit 8e8b5d2e6d91180a), one can use
/dev/{char,block}-MAJOR syntax to specify that all MAJOR:*
devices are allowed.
Use it, if available, since it's more straightforward, plus
we can skip somewhat expensive parsing of /proc/devices.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
runc delete is supposed to remove all the container's artefacts.
In case systemd cgroup driver is used, and the systemd unit has failed
(e.g. oom-killed), systemd won't remove the unit (that is, unless the
"CollectMode: inactive-or-failed" property is set).
Call reset-failed from manager.Destroy so the failed unit will be
removed during "runc delete".
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes we call resetFailedUnit as a cleanup measure, and we don't
care if it fails or not. So, move error reporting to its callers, and
ignore error in cases we don't really expect it to succeed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Replace a panic with a warning, unless it's ENOENT and we're running in
a user namespace. In the latter case, do the same as before, i.e. report
the error but using a Debug logging level.
This prevents software that uses libcontainer from panicking in
some exotic setups.
This will also print a warning on some very old systems which does not
use /sys/fs/cgroup for cgroup mount point. My bet is such systems no
longer exist.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We read output from the following files if they exists:
- cpu.pressure
- memory.pressure
- io.pressure
Each are in format:
```
some avg10=0.00 avg60=0.00 avg300=0.00 total=0
full avg10=0.00 avg60=0.00 avg300=0.00 total=0
```
Signed-off-by: Daniel Dao <dqminh89@gmail.com>
Co-authored-by: Sandor Szücs <sandor.szuecs@zalando.de>
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In code we have frozen the cgroup to avoid the processes get
an occasional "permission denied" error, while the systemd's application of device
rules is done disruptively. When the processes in the container can not
be frozen over 2 seconds (which defined in fs/freezer.go),
we still update the cgroup which resulting the container get an occasional
"permission denied" error in some cases.
Return error directly without updating cgroup, when freeze fails.
Fixes: #3803
Signed-off-by: Zoe <hi@zoe.im>
1. Use strings.TrimPrefix instead of fmt.Sscanf and simplify the code.
2. Add a test case and a benchmark.
The benchmark shows some improvement, compared to the old
implementation:
name old time/op new time/op delta
FindDeviceGroup-4 39.7µs ± 2% 26.8µs ± 2% -32.63% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
FindDeviceGroup-4 6.08kB ± 0% 4.23kB ± 0% -30.39% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
FindDeviceGroup-4 117 ± 0% 6 ± 0% -94.87% (p=0.008 n=5+5)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 343951a22b added a call to os.Stat for the device path
when generating systemd device properties, to avoid systemd warning for
non-existing devices. The idea was, since systemd uses stat(2) to look
up device properties for a given path, it will fail anyway. In addition,
this allowed to suppress a warning like this from systemd:
> Couldn't stat device /dev/char/10:200
NOTE that this was done because:
- systemd could not add the rule anyway;
- runs puts its own set of rules on top of what systemd does.
Apparently, the above change broke some setups, resulting in inability
to use e.g. /dev/null inside a container. My guess is this is because
in cgroup v2 we add a second eBPF program, which is not used if the
first one (added by systemd) returns "access denied".
Next, commit 3b9582895b fixed that by adding a call to os.Stat for
"/sys/"+path (meaning, if "/dev/char/10:200" does not exist, we retry
with "/sys/dev/char/10:200", and if it exists, proceed with adding a
device rule with the original (non-"/sys") path).
How that second fix ever worked was a mystery, because the path we gave
to systemd still doesn't exist.
Well, I think now I know.
Since systemd v240 (commit 74c48bf5a8005f20) device access rules
specified as /dev/{block|char}/MM:mm are no longer looked up on the
filesystem, instead, if possible, those are parsed from the string.
So, we need to do different things, depending on systemd version:
- for systemd >= v240, use the /dev/{char,block}/MM:mm as is, without
doing stat() -- since systemd doesn't do stat() either;
- for older version, check if the path exists, and skip passing it on
to systemd otherwise.
- the check for /sys/dev/{block,char}/MM:mm is not needed in either
case.
Pass the systemd version to the function that generates the rules, and
fix it accordingly.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These functions were added in ancient times, facilitating the
docker-in-docker case when cgroup namespace was not available.
As pointed out in commit 2b28b3c276, using init 1 cgroup is not
correct because it won't work in case of host PID namespace.
The last user of GetInitCgroup was removed by commit
54e20217a8. GetInitCgroupPath was never used
as far as I can see, nor was I able to find any external users.
Remove both functions. Modify the comment in libct/cg/fs.subsysPath
to not refer to GetInitCgroupPath.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Systemd v252 (available in CentOS Stream 9 in our CI) added support
for setting cpu.idle (see [1]). The way it works is:
- if CPUWeight == 0, cpu.idle is set to 1;
- if CPUWeight != 0, cpu.idle is set to 0.
This commit implements setting cpu.idle in systemd cgroup driver via a
unit property. In case CPUIdle is set to non-zero value, the driver sets
adds CPUWeight=0 property, which will result in systemd setting cpu.idle
to 1.
Unfortunately, there's no way to set cpu.idle to 0 without also changing
the CPUWeight value, so the driver doesn't do anything if CPUIdle is
explicitly set to 0. This case is handled by the fs driver which is
always used as a followup to setting systemd unit properties.
Also, handle cpu.idle set via unified map. In case it is set to non-zero
value, add CPUWeight=0 property, and ignore cpu.weight (otherwise we'll
get two different CPUWeight properties set).
Add a unit test for new values in unified map, and an integration test case.
[1] https://github.com/systemd/systemd/pull/23299
[2] https://github.com/opencontainers/runc/issues/3786
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In code that checks that the resource name is in the for
Using strings.SplitN is an overkill in this case, resulting in
allocations and thus garbage to collect.
Using strings.IndexByte and checking that result is not less than 1
(meaning there is a period, and it is not the first character) is
sufficient here.
Fixes: 0cb8bf67a3
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
Commit d223e2adae ("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] https://github.com/opencontainers/runc/pull/1124
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 39914db679 this function is not used by runc (see
that commit to learn why this function is not that good).
I was not able to find any external users either.
Since it's not a good function, with no users, and it is rather trivial,
let's remove it right away (rather than mark as deprecated).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes, the init process is not in the root cgroup.
This can be noted by GetInitPath, which already scrubs the path of `init.scope`.
This was encountered when trying to patch the Kubelet to handle systemd being in a separate cpuset
from root (to allow load balance disabling for containers). At present, there's no way to have libcontainer or runc
manage cgroups in a hierarchy outside of the one init is in (unless the path contains `init.scope`, which is limiting)
Signed-off-by: Peter Hunt <pehunt@redhat.com>
There is some kind of a race in CentOS 7 which sometimes result in one
of these tests failing like this:
systemd_test.go:136: mkdir /sys/fs/cgroup/hugetlb/system.slice/system-runc_test_pods.slice: no such file or directory
or
systemd_test.go:187: open /sys/fs/cgroup/cpuset/system.slice/system-runc_test_pods.slice/cpuset.mems: no such file or directory
As this is only happening on CentOS 7, let's skip this test on this
platform.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
centos-9 unit test sometimes fails with:
=== RUN TestPodSkipDevicesUpdate
systemd_test.go:114: container stderr not empty: basename: missing operand
Try 'basename --help' for more information.
--- FAIL: TestPodSkipDevicesUpdate (0.11s)
I'm not sure why the container output is an error in basename. It seems
likely that the bashrc in that distro is kind of broken. Let's just run
a sleep command and forget about bash.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Rewrite systemdVersionAtoi to not use regexp, and fix two issues:
1. It was returning 0 (rather than -1) for some errors.
2. The comment was saying that the input string is without quotes,
while in fact it is.
Note the new function, similar to the old one, works on input either
with or without quotes. Amend the test to add test cases without quotes.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is aimed at solving the problem of cgroup v2 memory controller
behavior which is not compatible with that of cgroup v1.
In cgroup v1, if the new memory limit being set is lower than the
current usage, setting the new limit fails.
In cgroup v2, same operation succeeds, and the container is OOM killed.
Introduce a new setting, memory.checkBeforeUpdate, and use it to mimic
cgroup v1 behavior.
Note that this is not 100% reliable because of TOCTOU, but this is the
best we can do.
Add some test cases.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Runc parses cpuset range to bits in the case of cgroup v2 + systemd as cgroup driver.
The byte order representation differs from systemd expectation, which will set
different cpuset range in systemd transient unit if the length of parsed byte array exceeds one.
# cat config.json
...
"resources": {
...
"cpu": {
"cpus": "10-23"
}
},
...
# runc --systemd-cgroup run test
# cat /run/systemd/transient/runc-test.scope.d/50-AllowedCPUs.conf
# This is a drop-in unit file extension, created via "systemctl set-property"
# or an equivalent operation. Do not edit.
[Scope]
AllowedCPUs=0-7 10-15
The cpuset.cpus in cgroup will also be set to wrong value after reloading systemd manager configuration.
# systemctl daemon-reload
# cat /sys/fs/cgroup/system.slice/runc-test.scope/cpuset.cpus
0-7,10-15
Signed-off-by: seyeongkim <seyeong.kim@canonical.com>
Signed-off-by: Chengen, Du <chengen.du@canonical.com>
This is a forward port of https://github.com/opencontainers/runc/pull/3620
The original code depended on the origin filesystem to have
/dev/{block,char} populated. This is done by udev normally and while is
very common non-containerized systemd installs, it's very easy to start
systemd in a container created by runc itself and not have
/dev/{block,char} populated. When this occurs, the following error
output is observed:
$ docker run hello-world
docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error reopening /dev/null inside container: open /dev/null: operation not permitted: unknown.
/dev/null can't be opened because it was not added to the
deviceAllowList, as there was no /dev/char directory. The change here
utilizes the fact that when sysfs in in use, there is a
/sys/dev/{block,char} that is kernel maintained that we can check.
Signed-off-by: Evan Phoenix <evan@phx.io>
It's more idiomatic Go to define interfaces on the receiver, and constructors to
return concrete types.
This patch changes various constructors to return a concrete type, with the
exceptions of NewWithPaths, which needs the abstraction as it switches between
implementations.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A regression reported for runc v1.1.3 says that "runc exec -t" fails
after doing "systemctl daemon-reload":
> exec failed: unable to start container process: open /dev/pts/0: operation not permitted: unknown
Apparently, with commit 7219387eb7 we are no longer adding
"DeviceAllow=char-pts rwm" rule (because os.Stat("char-pts") returns
ENOENT).
The bug can only be seen after "systemctl daemon-reload" because runc
also applies the same rules manually (by writing to devices.allow for
cgroup v1), and apparently reloading systemd leads to re-applying the
rules that systemd has (thus removing the char-pts access).
The fix is to do os.Stat only for "/dev" paths.
Also, emit a warning that the path was skipped. Since the original idea
was to emit less warnings, demote the level to debug.
Note this also fixes the issue of not adding "m" permission for block-*
and char-* devices.
A test case is added, which reliably fails before the fix
on both cgroup v1 and v2.
Fixes: https://github.com/opencontainers/runc/issues/3551
Fixes: 7219387eb7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since Go 1.19, godoc recognizes lists, code blocks, headings etc. It
also reformats the sources making it more apparent that these features
are used.
Fix a few places where it misinterpreted the formatting (such as
indented vs unindented), and format the result using the gofumpt
from HEAD, which already incorporates gofmt 1.19 changes.
Some more fixes (and enhancements) might be required.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
systemd emits very loud warnings when the path specified doesn't exist
(which can be the case for some of our default rules). We don't need the
ruleset we give systemd to be completely accurate (we discard some kinds
of wildcard rules anyway) so we can safely skip adding these.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These are only used from inside the package, and we don't want them to
be public.
The only two methods left are Enable and Disable.
While at it, fix or suppress found lint-extra warnings.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit separates the functionality of setting cgroup device
rules out of libct/cgroups to libct/cgroups/devices package. This
package, if imported, sets the function variables in libct/cgroups and
libct/cgroups/systemd, so that a cgroup manager can use those to manage
devices. If those function variables are nil (when libct/cgroups/devices
are not imported), a cgroup manager returns the ErrDevicesUnsupported
in case any device rules are set in Resources.
It also consolidates the code from libct/cgroups/ebpf and
libct/cgroups/ebpf/devicefilter into libct/cgroups/devices.
Moved some tests in libct/cg/sd that require device management to
libct/sd/devices.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This moves the functionality related to devices, SkipDevices, and
SkipFreezeOnSet to a separate file, in preparation for the next commit.
No code changes.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Looking through git blame, this was added by commit 9fac18329
aka "Initial commit of runc binary", most probably by mistake.
Obviously, a container should not have access to tun/tap device, unless
it is explicitly specified in configuration.
Now, removing this might create a compatibility issue, but I see no
other choice.
Aside from the obvious misconfiguration, this should also fix the
annoying
> Apr 26 03:46:56 foo.bar systemd[1]: Couldn't stat device /dev/char/10:200: No such file or directory
messages from systemd on every container start, when runc uses systemd
cgroup driver, and the system runs an old (< v240) version of systemd
(the message was presumably eliminated by [1]).
[1] https://github.com/systemd/systemd/pull/10996/commits/d5aecba6e0b7c73657c4cf544ce57289115098e7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case statfs("/sys/fs/cgroup/unified") fails with any error other
than ENOENT, current code panics. As IsCgroup2HybridMode is called from
libcontainer/cgroups/fs's init function, this means that any user of
libcontainer may panic during initialization, which is ugly.
Avoid panicking; instead, do not enable hybrid hierarchy support and
report the error (under debug level, not to confuse anyone).
Basically, replace the panic with "turn off hybrid mode support"
(which makes total sense since we were unable to statfs its root).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since LinuxFactory has become the means to specify containers state
top directory (aka --root), and is only used by two methods (Create
and Load), it is easier to pass root to them directly.
Modify all the users and the docs accordingly.
While at it, fix Create and Load docs (those that were originally moved
from the Factory interface docs).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Jordan Rife