There is no such thing as linux.resources.memorySwap (the mem+swap is
set as linux.resources.memory.swap).
As it is not used in this test anyway, remove it.
Fixes: 4929c05ad1
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Rename CGROUP_PATH to CGROUP_V2_PATH so it is more clear that it can
only be used for CGROUP_V2, and to resolve ambiguity with CGROUP_PATH
variable used in tests/rootless.sh.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Filter out rdma controller since systemd is unable to delegate it.
Similar to commits 05272718f4 and 601cf5825f.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit d08bc0c1b3 ("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>
The variable $ROOTLESS, as set by helpers.bash and used in many places,
provides the same value as $EUID which is always set by bash. Since we
are using bash, we can rely on $EUID being omnipresent.
Modify all uses accordingly, and since the value is known to be a
number, omit the quoting.
Similarly, replace all uses of $(id -u) to $EUID.
Do some trivial cleanups along the way, such as
- simplify some if A; then B; to A && B;
- do not use [[ instead of [ where not necessary.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This makes it work similar to all the other variables we use as binary
flags.
The new 'shellcheck disable' is due to a bug in shellcheck (basically,
it does not track the scope of variables or execution order, assuming
everything is executed as soon as it is seen).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Strictly speaking, == is for [[ only, not for [ / test,
and, unlike =, the right side is a pattern.
To avoid confusion, use =. In cases where we compare with empty string,
use -z instead.
Keep using [[ in some cases since it does not require quoting the left
and right side of comparison (I trust shellcheck on that one).
This should have no effect (other than the code being a tad more
strict).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Audit all checks for non-empty variables (i.e. ' -z ', ' -n ',
' != ""' and '= ""'), and fix those cases where a variable might be
unset. Those variables (that might not be set) are
- RUNC_USE_SYSTEMD
- BATS_RUN_TMPDIR
- AUX_UID
- AUX_DIR
- SD_PARENT_NAME
- REL_PARENT_PATH
- ROOT
- HAVE_CRIU
- ROOTLESS_FEATURES
- and a few test-specific or file-specific variables
This should allow us to enable set -u.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes a container cgroup already exists but is frozen.
When this happens, runc init hangs, and it's not clear what is going on.
Refuse to run in a frozen cgroup; add a test case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently runc allows multiple containers to share the same cgroup (for
example, by having the same cgroupPath in config.json). While such
shared configuration might be OK, there are some issues:
- When each container has its own resource limits, the order of
containers start determines whose limits will be effectively applied.
- When one of containers is paused, all others are paused, too.
- When a container is paused, any attempt to do runc create/run/exec
end up with runc init stuck inside a frozen cgroup.
- When a systemd cgroup manager is used, this becomes even worse -- such
as, stop (or even failed start) of any container results in
"stopTransientUnit" command being sent to systemd, and so (depending on
unit properties) other containers can receive SIGTERM, be killed after a
timeout etc.
Any of the above may lead to various hard-to-debug situations in production
(runc init stuck, cgroup removal error, wrong resource limits, init not
reaping zombies etc.).
One obvious solution is to refuse a non-empty cgroup when starting a new
container. This would be a breaking change though, so let's make it in
steps, with the first step is issue a warning and a deprecated notice
about a non-empty cgroup.
Later (in runc 1.2) we will replace this warning with an error.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently, if a container is paused (i.e. its cgroup is frozen), runc exec
just hangs, and it is not obvious why.
Refuse to exec in a paused container. Add a test case.
In case runc exec in a paused container is a legit use case,
add --ignore-paused option to override the check. Document it,
add a test case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The misc cgroup controller, introduced in Linux 5.13, is still unknown
to systemd, and thus it cannot delegate it. Add an appropriate fixup
to the test case, similar to an earlier commit 601cf5825f.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Check that runc run and runc exec put the process on the same cgroups v2
when using hybrid mode.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This removes libcontainer's own error wrapping system, consisting of a
few types and functions, aimed at typization, wrapping and unwrapping
of errors, as well as saving error stack traces.
Since Go 1.13 now provides its own error wrapping mechanism and a few
related functions, it makes sense to switch to it.
While doing that, improve some error messages so that they start
with "error", "unable to", or "can't".
A few things that are worth mentioning:
1. We lose stack traces (which were never shown anyway).
2. Users of libcontainer that relied on particular errors (like
ContainerNotExists) need to switch to using errors.Is with
the new errors defined in error.go.
3. encoding/json is unable to unmarshal the built-in error type,
so we have to introduce initError and wrap the errors into it
(basically passing the error as a string). This is the same
as it was before, just a tad simpler (actually the initError
is a type that got removed in commit afa844311; also suddenly
ierr variable name makes sense now).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Before this commit, "require rootless_cgroup" feature check required "cgroup"
to be present in ROOTLESS_FEATURES. The idea of the requirement, though, is
to ensure that rootless runc can manage cgroups.
In case of systemd + cgroup v2, rootless runc can manage cgroups,
thanks to systemd delegation, so modify the feature check accordingly.
Next, convert (simplify) some of the existing users to the modified check.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Systemd is not able to delegate hugetlb controller, and it is needed for
cgroup v2 + systemd + rootless case (which is currently skipped because
of "requires rootless_cgroup", but will be enabled by a later commit).
The failure being fixed looks like this:
> not ok 4 runc create (limits + cgrouppath + permission on the cgroup dir) succeeds
> # (from function `check_cgroup_value' in file /vagrant/tests/integration/helpers.bash, line 188,
> # in test file /vagrant/tests/integration/cgroups.bats, line 53)
> # `check_cgroup_value "cgroup.controllers" "$(cat /sys/fs/cgroup/user.slice/user-$(id -u).slice/cgroup.controllers)"' failed
> # <....>
> # /sys/fs/cgroup/user.slice/user-2000.slice/user@2000.service/machine.slice/runc-cgroups-integration-test-20341.scope/cgroup.controllers
> # current cpuset cpu io memory pids !? cpuset cpu io memory hugetlb pids
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This checks that in-container view of /sys/fs/cgroup does not
contain any extra cgroups (which was the case for rootless
before the previous commit).
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>
Commit 41670e21f removed BUSYBOX_BUNDLE env var, but c3ffd2ef81
was developed before 41670e21f was merged.
Everything still works because now BUSYBOX_BUNDLE has no value.
Nevertheless, let's remove it to avoid confusion.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
if bfq is not loaded, then io.bfq.weight is not available. io.weight
should always be available and is the next best equivalent thing.
Signed-off-by: Daniel Dao <dqminh89@gmail.com>
bfq weight controller (i.e. io.bfq.weight if present) is still using the
same bfq weight scheme (i.e 1->1000, see [1].) Unfortunately the
documentation for this was wrong, and only fixed recently [2].
Therefore, if we map blkio weight to io.bfq.weight, there's no need to
do any conversion. Otherwise, we will try to write invalid value which
results in error such as:
```
time="2021-02-03T14:55:30Z" level=error msg="container_linux.go:367: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write \"7475\": write /sys/fs/cgroup/runc-cgroups-integration-test/test-cgroup/io.bfq.weight: numerical result out of range"
```
[1] https://github.com/torvalds/linux/blob/master/Documentation/block/bfq-iosched.rst
[2] https://github.com/torvalds/linux/commit/65752aef0a407e1ef17ec78a7fc31ba4e0b360f9
Signed-off-by: Daniel Dao <dqminh89@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>
1. Get rid of fixed ROOT, *_BUNDLE, and CONSOLE_SOCKET dirs.
Now they are temporary directories created in setup_bundle.
2. Automate containers cleanup: instead of having to specify all
containers to be removed, list and destroy everything (which is
now possible since every test case has its own unique root).
3. Randomize cgroup paths so two tests running in parallel won't
use the same cgroup.
Now it's theoretically possible to execute tests in parallel.
Practically it's not possible yet because bats uses GNU parallel,
which do not provide a terminal for whatever it executes, and
many runc tests (all those that run containers with terminal:
true) needs a tty. This may possibly be addressed later.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
All the tests are run with cd to bundle directory, so
it does not make ense to explicitly specify it.
This relies on the feature of functions like set_cgroups_path
or update_config to use current directory by default.
In those cases where we need to change the directory
(runc create/run -b), save the current one and use it later.
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>
Various bats tests use various types of indentation (about half
is tabs, the rest is 2 spaces, 4 spaces, etc.).
Let's bring it to one style (tabs) using recently added
shfmt support for bats files (see [1]).
This commit is brought to you by
shfmt -ln bats -w tests/integration/*.bats
[1] https://github.com/mvdan/sh/issues/600
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When we call terminate(), we kill the process, and wait
returns the error indicating the process was killed.
This is exactly what we expect here, so there is no reason
to treat it as an error.
Before this patch, when a container with invalid cgroup parameters is
started:
> WARN[0000] unable to terminate initProcess error="signal: killed"
> ERRO[0000] container_linux.go:366: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write "555": open /sys/fs/cgroup/blkio/user.slice/xx33/blkio.weight: permission denied
After:
> ERRO[0000] container_linux.go:366: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write "555": open /sys/fs/cgroup/blkio/user.slice/xx33/blkio.weight: permission denied
I.e. the useless warning is gone.
NOTE this breaks a couple of integration test cases, since they were
expecting a particular message in the second line, and now due to
"signal: killed" removed it's in the first line. Fix those, too.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit d78ae51a2 adds this line, but the file is never created.
This is probably a leftover from copy-pasting update.bats code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Ignore the shellcheck warnings like this one:
> In tty.bats line 32:
> update_config '(.. | select(.[]? == "sh")) += ["-c", "stat -c %u:%g $(tty) | tr : \\\\n"]'
> ^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.
While at it, fix some minor whitespace issues in tty.bats.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix or ignore warnings like this one:
> In cgroups.bats line 107:
> if [ $(id -u) = "0" ]; then
> ^------^ SC2046: Quote this to prevent word splitting.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Those are pretty simple to allow shellcheck to fix these, so
this commit is courtesy of
> shellcheck -i SC2086 -i SC2006 -f diff *.bats > fix.diff
> patch -p1 < fix.diff
repeated 3 times ;)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fixes the following warning:
> In cgroups.bats line 58:
> if [ "$KERNEL_MAJOR" -lt 4 ] || [ "$KERNEL_MAJOR" -eq 4 -a "$KERNEL_MINOR" -le 5 ]; then
> ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin