Kir Kolyshkin (8):
tests/int/helpers: rm old code
libct/cg/sd/v2: call initPath from Path
tests/int/update.bats: don't set cpuset in setup
tests/int/helpers: generalize require cgroups_freezer
tests/int: enable/use requires cgroups_<ctrl>
tests/int/cgroups: don't check for hugetlb
tests/int: run rootless_cgroup tests for v2+systemd
Vagrantfile.fedora: set Delegate=yes
LGTMs: AkihiroSuda cyphar
Closes#2944
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>
1. In case of cgroup v2 + systemd + rootless, get the list of controllers from
the own cgroup, rather than from the root one (as systemd cgroup delegation
is required in this case, and it might not be set or fully working).
2. Use "requires cgroups_<controller>" in tests that need those
controllers.
Tested on Fedora 31 (which does not support delegation of cpuset controller).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Setting cpuset.cpus requires cpuset controller, which might
not be available in case of cgroup v2 + systemd < 244.
Rather than require cpuset support from every test case, do not set the initial
cpuset value from setup(), and do not check it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This was added by commit ca4f427af, together with set_resources_limit,
and was needed for the latter, since sed was used to update resources.
Now when we switched to jq in commit 79fe41d3c1, this kludge is no
longer needed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
`--parent-path` needs to be relative path of `--image-path`,
and points to previous checkpoint directory.
Signed-off-by: Liu Hua <weldonliu@tencent.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>
Criu creates symlink named "parent" under image-path while doing
interactive checkpoint, without checking wether symlink points to
right place. This patch corrects related test case.
Signed-off-by: Liu Hua <weldonliu@tencent.com>
For the fix, see previous commit. Without the fix, this test case fails:
> container_linux.go:380: starting container process caused:
> process_linux.go:545: container init caused: readonly path /proc/bus:
> operation not permitted
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>
In order to make 'runc --debug' actually useful for debugging nsexec
bugs, provide information about all the internal operations when in
debug mode.
[@kolyshkin: rebasing; fix formatting via indent for make validate to pass]
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Alas, the EPERM on chdir saga continues...
Unfortunately, the there were two releases between when https://github.com/opencontainers/runc/commit/5e0e67d76cc99d76c8228d48f38f37034503f315 was released
and when the workaround https://github.com/opencontainers/runc/pull/2712 was added.
Between this, folks started relying on the ability to have a workdir that the container user doesn't have access to.
Since this case was previously valid, we should continue support for it.
Now, we retry the chdir:
Once at the top of the function (to catch cases where the runc user has access, but container user does not)
and once after we setup user (to catch cases where the container user has access, and the runc user does not)
Add a test case for this as well.
Signed-off-by: Peter Hunt <pehunt@redhat.com>
"checkpoint --lazy-pages and restore" test sometimes fails on restore
in our CI on Fedora 33 when systemd cgroup driver is used:
> (00.076104) Error (compel/src/lib/infect.c:1513): Task 48521 is in unexpected state: f7f
> (00.076122) Error (compel/src/lib/infect.c:1520): Task stopped with 15: Terminated
> ...
> (00.078246) Error (criu/cr-restore.c:2483): Restoring FAILED.
I think what happens is
1. The test runs runc checkpoint in lazy-pages mode in background.
2. The test runs criu lazy-pages in background.
3. The test runs runc restore.
Now, all three are working in together: criu restore restores, criu
lazy-pages listens for page faults on a uffd and fetch missing pages
from runc checkpoint, who serves those pages.
At some point criu lazy-pages decides to fetch the rest of the pages,
and once it's done it exits, and runc checkpoint, as there are no more
pages to serve, exits too.
At the end of runc checkpoint the container is removed (see "defer
destroy(container)" in checkpoint.go. This involves a call to
cgroupManager.Destroy, which, in case systemd manager is used,
calls stopUnit, which makes systemd to not just remove the unit,
but also send SIGTERM to its processes, if there are any.
As the container is being restored into the same systemd unit,
sometimes this results in sending SIGTERM to a process which
criu restores, and thus restoring fails.
The remedy here is to change the name of systemd unit to which the
container is restored.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In check_pipes, make sure we
- close all fds we opened in setup_pipes;
- check that runc stderr is empty (and fail if it's not).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 41670e21f0 added some randomization to cgroup paths
and (if systemd cgroup driver is used) systemd unit names,
but the randomization was per bats instance, not per test.
Fix this by refactoring init_cgroups_path/set_cgroups_path
(moving variable/random part to set_cgroups_path).
NOTE though that the randomization is only performed for those tests
that explicitly call set_cgroups_path.
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>
Helper function init_cgroup_paths sets two sets of cgroup path variables
for cgroup v1 case (below XXX is cgroup controller name, e.g. MEMORY):
1. CGROUP_XXX_BASE_PATH -- path to XXX controller mount point
(e.g. CGROUP_MEMORY_BASE_PATH=/sys/fs/cgroup/memory);
2. CGROUP_XXX -- path to the particular container XXX controller cgroup
(e.g. CGROUP_MEMORY=/sys/fs/cgroup/memory/runc-cgroups-integration-test/test-cgroup).
The second set of variables is mostly used by check_cgroup_value(),
with only two exceptions:
- CGROUP_CPU in @test "update rt period and runtime";
- few CGROUP_XXX in @test "runc delete --force in cgroupv1 with
subcgroups".
Remove these variables, as their values are not used much
and are easy to get (as can be seen in modified test cases).
While at it, mark some variables as local.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The full log is very long and it did not gave us any additional clues.
This reverts commit 053e15c001.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
First, add runc --debug exec test cases, very similar to those in
debug.bats but for runc exec (rather than runc run). Do not include json
tests as it is already tested in debug.bats.
Second, add logrus debug to late stages of runc init, and amend the
integration tests to check for those messages. This serves two purposes:
- demonstrate that runc init can be amended with debug logrus which is
properly forwarded to and logged by the parent runc create/run/exec;
- improve the chances to catch the race fixed by the previous commit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Remove redundant "echo $output" from the first test case, as "runc"
helper function already logs the output.
2. Show the contents of log.out to stderr, so it case of error we can
see what is going on.
3. Remove the check that `log.out` file exists. This check is redundant,
since right after it we do `cat log.out` and check its exit code.
4. Factor out common checks into check_debug.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes the test fails without any clear diagnostics:
> not ok 16 checkpoint --lazy-pages and restore
> (in test file tests/integration/checkpoint.bats, line 191)
> `[ "$out" = "0000000 000000 0000001" ]' failed
> ...
> criu failed: type NOTIFY errno 3\nlog file: work-dir/dump.log
We look for and print errors via grep, but in the above case
there are nothing that is denoted error in the log.
So, let's show the damn log in its entirely (note it is only shown
if test fails).
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>
Sometimes github gives up 5xx errors, for example:
> panic: getImages error exit status 1 (output: curl: (22) The requested URL returned error: 502
> Failed to get https://github.com/docker-library/busybox/raw/dist-i386/stable/glibc/busybox.tar.xz
This is unfortunate but temporary, and adding --retry should handle
at least some cases, improving CI stability.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case log level is less than debug, this code does nothing,
so let's add a condition and skip it entirely.
Add a test case to make sure this code path is hit.
Signed-off-by: Kir Kolyshkin <kolyshkin@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>
Aleksa Sarai