-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
runc kill: add support for cgroup.kill #3825
Commits on Jun 8, 2023
-
tests/rootless.sh: drop set -x
It seems that set -x was temporarily added as a debug measure, but slipped into the final commit. Remove it, for the sake of test logs brevity. Fixes: 9f656db Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Configuration menu - View commit details
-
Copy full SHA for 67bc4bc - Browse repository at this point
Copy the full SHA 67bc4bcView commit details -
tests/int/kill: add kill -a with host pidns test
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>
Configuration menu - View commit details
-
Copy full SHA for e0e8d9c - Browse repository at this point
Copy the full SHA e0e8d9cView commit details -
libct: signalAllProcesses: remove child reaping
There are two very distinct usage scenarios for signalAllProcesses: * when used from the runc binary ("runc kill" command), the processes that it kills are not the children of "runc kill", and so calling wait(2) on each process is totally useless, as it will return ECHLD; * when used from a program that have created the container (such as libcontainer/integration test suite), that program can and should call wait(2), not the signalling code. So, the child reaping code is totally useless in the first case, and should be implemented by the program using libcontainer in the second case. I was not able to track down how this code was added, my best guess is it happened when this code was part of dockerd, which did not have a proper child reaper implemented at that time. Remove it, and add a proper documentation piece. Change the integration test accordingly. PS the first attempt to disable the child reaping code in signalAllProcesses was made in commit bb912eb, which used a questionable heuristic to figure out whether wait(2) should be called. This heuristic worked for a particular use case, but is not correct in general. While at it: - simplify signalAllProcesses to use unix.Kill; - document (container).Signal. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Configuration menu - View commit details
-
Copy full SHA for 5b8f871 - Browse repository at this point
Copy the full SHA 5b8f871View commit details -
libct: fix shared pidns detection
When someone is using libcontainer to start and kill containers from a long lived process (i.e. the same process creates and removes the container), initProcess.wait method is used, which has a kludge to work around killing containers that do not have their own PID namespace. The code that checks for own PID namespace is not entirely correct. To be exact, it does not set sharePidns flag when the host/caller PID namespace is implicitly used. As a result, the above mentioned kludge does not work. Fix the issue, add a test case (which fails without the fix). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Configuration menu - View commit details
-
Copy full SHA for 2a7dcbb - Browse repository at this point
Copy the full SHA 2a7dcbbView commit details -
libct: move killing logic to container.Signal
By default, the container has its own PID namespace, and killing (with SIGKILL) its init process from the parent PID namespace also kills all the other processes. Obviously, it does not work that way when the container is sharing its PID namespace with the host or another container, since init is no longer special (it's not PID 1). In this case, killing container's init will result in a bunch of other processes left running (and thus the inability to remove the cgroup). The solution to the above problem is killing all the container processes, not just init. The problem with the current implementation is, the killing logic is implemented in libcontainer's initProcess.wait, and thus only available to libcontainer users, but not the runc kill command (which uses nonChildProcess.kill and does not use wait at all). So, some workarounds exist: - func destroy(c *Container) calls signalAllProcesses; - runc kill implements -a flag. This code became very tangled over time. Let's simplify things by moving the killing all processes from initProcess.wait to container.Signal, and documents the new behavior. In essence, this also makes `runc kill` to automatically kill all container processes when the container does not have its own PID namespace. Document that as well. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Configuration menu - View commit details
-
Copy full SHA for 9583b3d - Browse repository at this point
Copy the full SHA 9583b3dView commit details -
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>
Configuration menu - View commit details
-
Copy full SHA for f8ad20f - Browse repository at this point
Copy the full SHA f8ad20fView commit details -
libct: implement support for cgroup.kill
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Configuration menu - View commit details
-
Copy full SHA for 7d09ba1 - Browse repository at this point
Copy the full SHA 7d09ba1View commit details