This handles a corner case when joining a container having all
the processes running exclusively on isolated CPU cores to force
the kernel to schedule runc process on the first CPU core within the
cgroups cpuset.
The introduction of the kernel commit
46a87b3851f0d6eb05e6d83d5c5a30df0eca8f76 has affected this deterministic
scheduling behavior by distributing tasks across CPU cores within the
cgroups cpuset. Some intensive real-time application are relying on this
deterministic behavior and use the first CPU core to run a slow thread
while other CPU cores are fully used by real-time threads with SCHED_FIFO
policy. Such applications prevents runc process from joining a container
when the runc process is randomly scheduled on a CPU core owned by a
real-time thread.
Introduces isolated CPU affinity transition OCI runtime annotation
org.opencontainers.runc.exec.isolated-cpu-affinity-transition to restore
the behavior during runc exec.
Fix issue with kernel >= 6.2 not resetting CPU affinity for container processes.
Signed-off-by: Cédric Clerget <cedric.clerget@gmail.com>
With the idmap work, we will have a tainted Go thread in our
thread-group that has a different mount namespace to the other threads.
It seems that (due to some bad luck) the Go scheduler tends to make this
thread the thread-group leader in our tests, which results in very
baffling failures where /proc/self/mountinfo produces gibberish results.
In order to avoid this, switch to using /proc/thread-self for everything
that is thread-local. This primarily includes switching all file
descriptor paths (CLONE_FS), all of the places that check the current
cgroup (technically we never will run a single runc thread in a separate
cgroup, but better to be safe than sorry), and the aforementioned
mountinfo code. We don't need to do anything for the following because
the results we need aren't thread-local:
* Checks that certain namespaces are supported by stat(2)ing
/proc/self/ns/...
* /proc/self/exe and /proc/self/cmdline are not thread-local.
* While threads can be in different cgroups, we do not do this for the
runc binary (or libcontainer) and thus we do not need to switch to
the thread-local version of /proc/self/cgroups.
* All of the CLONE_NEWUSER files are not thread-local because you
cannot set the usernamespace of a single thread (setns(CLONE_NEWUSER)
is blocked for multi-threaded programs).
Note that we have to use runtime.LockOSThread when we have an open
handle to a tid-specific procfs file that we are operating on multiple
times. Go can reschedule us such that we are running on a different
thread and then kill the original thread (causing -ENOENT or similarly
confusing errors). This is not strictly necessary for most usages of
/proc/thread-self (such as using /proc/thread-self/fd/$n directly) since
only operating on the actual inodes associated with the tid requires
this locking, but because of the pre-3.17 fallback for CentOS, we have
to do this in most cases.
In addition, CentOS's kernel is too old for /proc/thread-self, which
requires us to emulate it -- however in rootfs_linux.go, we are in the
container pid namespace but /proc is the host's procfs. This leads to
the incredibly frustrating situation where there is no way (on pre-4.1
Linux) to figure out which /proc/self/task/... entry refers to the
current tid. We can just use /proc/self in this case.
Yes this is all pretty ugly. I also wish it wasn't necessary.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This field reports swap-only usage. For cgroupv1, `Usage` and `Failcnt`
are set by subtracting memory usage from memory+swap usage. For cgroupv2,
`Usage`, `Limit`, and `MaxUsage` are set. This commit also export `MaxUsage`
of memory under cgroupv2 mode, using `memory.peak` introduced in kernel 5.19.
Signed-off-by: Heran Yang <heran55@126.com>
This adds support for hugetlb.<pagesize>.rsvd limiting and accounting.
The previous non-rsvd max/limit_in_bytes does not account for reserved
huge page memory, making it possible for a processes to reserve all the
huge page memory, without being able to allocate it (due to cgroup
restrictions).
In practice this makes it possible to successfully mmap more huge page
memory than allowed via the cgroup settings, but when using the memory
the process will get a SIGBUS and crash. This is bad for applications
trying to mmap at startup (and it succeeds), but the program crashes
when starting to use the memory. eg. postgres is doing this by default.
This also keeps writing to the old max/limit_in_bytes, for backward
compatibility.
More info can be found here: https://lkml.org/lkml/2020/2/3/1153
(commit message mostly written by Odin Ugedal)
Co-authored-by: Odin Ugedal <odin@ugedal.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
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>
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>
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>
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>
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>
I have noticed that libct/cg/fs allocates 8K during init on every runc
execution:
> init github.com/opencontainers/runc/libcontainer/cgroups/fs @1.5 ms, 0.028 ms clock, 8512 bytes, 13 allocs
Apparently this is caused by global HugePageSizes variable init, which
is only used from GetStats (i.e. it is never used by runc itself).
Remove it, and use HugePageSizes() directly instead. Make it init-once,
so that GetStats (which, I guess, is periodically called by kubernetes)
does not re-read huge page sizes over and over.
This also removes 12 allocs and 8K from libct/cg/fs init section:
> $ time GODEBUG=inittrace=1 ./runc --help 2>&1 | grep cgroups/fs
> init github.com/opencontainers/runc/libcontainer/cgroups/fs @1.5 ms, 0.003 ms clock, 16 bytes, 1 allocs
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Since GetHugePageSize do not have any external users (checked by
sourcegraph), and no internal user ever uses its second return value
(the error), let's drop it.
2. Rename GetHugePageSize -> HugePageSizes (drop the Get prefix as per
Go guidelines, add suffix since we return many sizes).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case hugetlb is not supported, GetStats() should not error out,
and yet it does.
Assume that if GetHugePageSize return an error, hugetlb is
not supported (this is what cgroup v1 manager do).
Fixes: 89a87adb
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Cgroup controllers should never panic, and yet sometimes they do.
Add a unit test to check that controllers never panic when called with
nil arguments and/or resources, and fix a few found cases.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
cgName and cgParent are only used when cgPath is empty, so move
their cleaning to the body of the appropriate "if" statement.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Make Rootless and Systemd flags part of config.Cgroups.
2. Make all cgroup managers (not just fs2) return error (so it can do
more initialization -- added by the following commits).
3. Replace complicated cgroup manager instantiation in factory_linux
by a single (and simple) libcontainer/cgroups/manager.New() function.
4. getUnifiedPath is simplified to check that only a single path is
supplied (rather than checking that other paths, if supplied,
are the same).
[v2: can't -> cannot]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Only some libcontainer packages can be built on non-linux platforms
(not that it make sense, but at least go build succeeds). Let's call
these "good" packages.
For all other packages (i.e. ones that fail to build with GOOS other
than linux), it does not make sense to have linux build tag (as they
are broken already, and thus are not and can not be used on anything
other than Linux).
Remove linux build tag for all non-"good" packages.
This was mostly done by the following script, with just a few manual
fixes on top.
function list_good_pkgs() {
for pkg in $(find . -type d -print); do
GOOS=freebsd go build $pkg 2>/dev/null \
&& GOOS=solaris go build $pkg 2>/dev/null \
&& echo $pkg
done | sed -e 's|^./||' | tr '\n' '|' | sed -e 's/|$//'
}
function remove_tag() {
sed -i -e '\|^// +build linux$|d' $1
go fmt $1
}
SKIP="^("$(list_good_pkgs)")"
for f in $(git ls-files . | grep .go$); do
if echo $f | grep -qE "$SKIP"; then
echo skip $f
continue
fi
echo proc $f
remove_tag $f
done
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This was initially added by commits 41d9d26513 and 4a8f0b4db4,
apparently to implement docker run --cgroup container:ID, which was
never merged. Therefore, this code is not and was never used.
It needs to be removed mainly because having it makes it much harder to
understand how cgroup manager works (because with this in place we have
not one or two but three sets of cgroup paths to think about).
Note if the paths are known and there is a need to add a PID to existing
cgroup, cgroup manager is not needed at all -- something like
cgroups.WriteCgroupProc or cgroups.EnterPid is sufficient (and the
latter is what runc exec uses in (*setnsProcess).start).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Replace ioutil.TempDir (mostly) with t.TempDir, which require no
explicit cleanup.
While at it, fix incorrect usage of os.ModePerm in libcontainer/intelrdt
test. This is supposed to be a mask, not mode bits.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This builds on top of recently introduced fscommon.ParseError.
Errors returned from parsers (mostly ones used by GetStats()) are all
different, and many are incomplete. For example, in many cases errors
from strconv.ParseUint are returned as is, meaning there is no context
telling which file we were reading. Similarly, errors from
fscommon.ParseKeyValue should be wrapped to add more context.
Same is true for scanner.Err().
OTOH, errors from fscommon.GetCgroup* do have enough context and there
is no need to wrap them.
Fix all the above.
While at it, add missing scanner.Err() checks.
[v2: use parseError, not ParseError]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Do not wrap errors returned from fscommon.GetCgroupParamUint -- those
errors already have enough context.
2. Instead of parsing "max" ourselves, use GetCgroupParamUint which does
it, and then convert MaxUint64 to 0 (we do it historically since
commit 087b953dc5, and while using MaxUint64 as is seems fine,
there may be some existing users who rely on the old behavior).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Per-device weight is supported since kernel v5.4 (kernel commit
795fe54c2a8), so let's set those if supplied.
[v2: implement a more relaxed check in bfqDeviceWeightSupported]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is a better place as cgroups itself is using these.
Should help with moving more stuff common in between fs and fs2 to
fscommon.
Looks big, but this is just moving the code around:
fscommon/{fscommon,open}.go -> cgroups/file.go
fscommon/fscommon_test.go -> cgroups/file_test.go
and fixes for TestMode moved to a different package.
There's no functional change.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
gofumpt (mvdan.cc/gofumpt) is a fork of gofmt with stricter rules.
Brought to you by
git ls-files \*.go | grep -v ^vendor/ | xargs gofumpt -s -w
Looking at the diff, all these changes make sense.
Also, replace gofmt with gofumpt in golangci.yml.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
strconv.ParseUint(..., 0) is not really safe, because on 32-bit
architectures it will trigger runtime errors when trying to parse large
numbers (which in the case of the cgroupv2 io controller, is almost
certainly going to happen).
Fixes: 1932917b71 ("libcontainer: add initial support for cgroups v2")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Kubelet and cAdvisor depend on the metrics having the same values as in
cgroupv1, but we didn't correctly map the number of read and write IOs
to the correct cgroupv1 stats table (blkio.io_serviced).
In addition, don't leak any extra stats in our output -- if users need
that information we can always add a new field for it.
Reported-by: Yashpal Choudhary <yashpal.c1995@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
In the normal cases (only one existing filter or no existing filters),
just make use of BPF_F_REPLACE if there is one existing filter. However
if there is more than one filter applied, we should probably remove all
other filters since the alternative is that we will never remove our old
filters.
The only two other viable ways of solving this problem would be to use
BPF pins to either pin the eBPF program using a predictable name (so we
can always only replace *our* programs) or to switch away from custom
programs and instead use eBPF maps (which are pinned) and thus we just
update the map conntents to update the ruleset. Unfortunately these both
would add a hard requirement of bpffs and would require at least a minor
rewrite of the eBPF filtering code -- which is better left for another
time.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
There were several issues with the previous cgroupv2 devices filter
generator implementation, stemming from the previous implementation
using a few too many tricks to implement the correct cgroup behaviour
(rules were handled in reverse order, with wildcards having particularly
special interpretations). As a result, some slightly odd configurations
with rules in specific orders could result in incorrect filters being
generated.
By switching to the emulator which is already used by cgroupv1, we can
guarantee that the behaviour of filters in both cgroup versions will be
identical, as well as making use of the hardenings in the emulator (not
allowing users to add deny rules the kernel will ignore).
(Note that because the ordering of the devices emulator rules is
deterministic and based on the rule value, the existing test rules had
to be reordered slightly.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
According to cgroup v2 documentation [1]:
> Freezing of the cgroup may take some time; when this action is
> completed, the “frozen” value in the cgroup.events control file will
> be updated to “1” and the corresponding notification will be issued.
Implement polling of cgroup.events, waiting for "frozen 1" to appear.
In case something goes wrong, limit the maximum number of retries and
return "undefined" after some time (currently 10s).
[1] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Before this patch, setFreezer does
- open/read/close (to check if the freezer is supported)
- open/write/close (to set the value)
- open/read/close (to check the value)
Three opens is a bit excessive. Refactor to only open the file once:
- open (to check if the freezer is supported)
- write (to set the value)
- seek/read (to check the value)
- close
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case configs.Undefined or any wrong value is passed, there is no need
to check whether the freezer is supported.
Move arguments check to the beginning to avoid an unnecessary call to
supportFreezer().
While at it, simplify the "whether to return an error if freezer is not
supported" check.
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>
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>
For fs, commit fc620fdf81 made rootless field private,
and for fs2, it was always private, and yet comments in both
mention it as m.Rootless.
Fix it.
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>
Some controllers might still have stats available even if they are
disabled (this is definitely so for cpu.stat -- see earlier commit).
Some stat methods might implement sensible fallbacks (see previous
commit for statPids.
In the view of all that, it makes sense to not check if a particular
controller is available, but rather ignore ENOENT from it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When getting pids stats, instead of checking whether the pids controller
is available, let's use a fall back function in case pids.current does
not exist. This simplifies the logic in fs2.GetStats.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Cédric Clerget