`signalAllProcesses()` depends on the cgroup and is expected to fail
when runc is running in rootless without an access to the cgroup.
When `RootlessCgroups` is set to `true`, runc just ignores the error
from `signalAllProcesses` and may leak some processes running.
(See the comments in PR 4395)
In the future, runc should walk the process tree to avoid such a leak.
Note that `RootlessCgroups` is a misnomer; it is set to `false` despite
the name when cgroup v2 delegation is configured.
This is expected to be renamed in a separate commit.
Fix issue 4394
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
It takes some time for the kernel to kill the process (and remove its
PID from cgroup.procs). To ensure we don't have flakes from reading
cgroup.procs right after the kill, check and wait for processes to
actually be gone.
Fixes: 4163
Reported-by: lifubang@acmcoder.com
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We had ignore lines for these warnings, but it turns out this is most
likely a bug in shellcheck and we can work around it by moving the
helper function definition before any of the functions that use the
helper function. Hopefully it'll be fixed soon.
Ref: https://github.com/koalaman/shellcheck/issues/2873
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Commit f8ad20f moved the kill logic from container destroy to container
kill (which is the right thing to do).
Alas, it broke the use case of doing "runc delete -f" for a container
which does not have its own private PID namespace, when its init process
is gone. In this case, some processes may still be running, and runc
delete -f should kill them (the same way as "runc kill" does).
It does not do that because the container status is "stopped" (as runc
considers the container with no init process as stopped), and so we only
call "destroy" (which was doing the killing before).
The fix is easy: if --force is set, call killContainer no matter what.
Add a test case, similar to the one in the previous commit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit f8ad20f made it impossible to kill leftover processes in a
stopped container that does not have its own PID namespace. In other
words, if a container init is gone, it is no longer possible to use
`runc kill` to kill the leftover processes.
Fix this by moving the check if container init exists to after the
special case of handling the container without own PID namespace.
While at it, fix the minor issue introduced by commit 9583b3d:
if signalAllProcesses is used, there is no need to thaw the
container (as freeze/thaw is either done in signalAllProcesses already,
or not needed at all).
Also, make signalAllProcesses return an error early if the container
cgroup does not exist (as it relies on it to do its job). This way, the
error message returned is more generic and easier to understand
("container not running" instead of "can't open file").
Finally, add a test case.
Fixes: f8ad20f
Fixes: 9583b3d
Co-authored-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As of previous commit, this is implied in a particular scenario. In
fact, this is the one and only scenario that justifies the use of -a.
Drop the option from the documentation. For backward compatibility, do
recognize it, and retain the feature of ignoring the "container is
stopped" error when set.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is roughly the same as TestPIDHostInitProcessWait in libct/int,
except that here we use separate processes to create and to kill a
container, so the processes inside a container are not children of "runc kill", and
also we hit different codepaths (nonChildProcess.signal rather than
initProcess.signal).
One other thing is, rootless is also tested.
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>
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>
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>
This fixes all of the tests that were broken as part of the console
rewrite. This includes fixing the integration tests that used TTY
handling inside libcontainer, as well as the bats integration tests that
needed to be rewritten to use recvtty (as they rely on detached
containers that are running).
This patch is part of the console rewrite patchset.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
This makes it much simpler to write tests, and you don't have to worry
about some of the oddness with bats.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Akihiro Suda