mirror of
https://github.com/kata-containers/kata-containers.git
synced 2026-07-16 17:04:14 +00:00
agent: send SIGTERM only to root container process on container deletion
When containerd wants to terminate a container, it sends a KillRequest with exec_id="", all=false, signal=SIGTERM. Kata-shim translates that to a SignalProcessRequest with exec_id="", signal=SIGTERM Kata agent used to interpret exec_id="" as implying all=true. That was correct for forceful deletion of containers, as it uses a KillRequest with all=true, signal=SIGKILL. However, for graceful termination, we want to only signal the root process of the container (all=false). This changes makes it so that Kata agent interprets exec_id="" && signal=SIGKILL as implying all=true, and uses all=false otherwise. It also fixes an unrelated bug, where Kata agent would signal the root process twice when all=true. Fixes #13152 - sending SIGTERM twice to the container's init process on graceful shutdown of a container. Signed-off-by: Bozhidar Marinov <bozhidar.marinov1@digits.schwarz> Co-authored-by: Markus Rudy <webmaster@burgerdev.de> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
Fabiano Fidêncio
parent
ba23b1ad50
commit
2212dbdd48
@@ -489,6 +489,14 @@ impl AgentService {
|
||||
async fn do_signal_process(&self, req: protocols::agent::SignalProcessRequest) -> Result<()> {
|
||||
let cid = req.container_id;
|
||||
let eid = req.exec_id;
|
||||
let mut sig: libc::c_int = req.signal as libc::c_int;
|
||||
|
||||
let all = eid.is_empty() && sig == libc::SIGKILL;
|
||||
// NOTE: kata runtime encodes all = true by setting eid = "".
|
||||
// However, containerd can send eid = "", sig = SIGTERM, all = false when deleting containers
|
||||
// (and containerd will send eid = "", sig = SIGKILL, all = true when forcefully deleting containers)
|
||||
// Luckily, containerd never sends eid = "", sig = SIGKILL, all = false outside of ctr
|
||||
// So we can recover the original value of all here by checking eid and sig
|
||||
|
||||
info!(
|
||||
sl(),
|
||||
@@ -496,10 +504,10 @@ impl AgentService {
|
||||
"container-id" => &cid,
|
||||
"exec-id" => &eid,
|
||||
"signal" => req.signal,
|
||||
"all" => all,
|
||||
);
|
||||
|
||||
let mut sig: libc::c_int = req.signal as libc::c_int;
|
||||
{
|
||||
if !all {
|
||||
let mut sandbox = self.sandbox.lock().await;
|
||||
let p = sandbox
|
||||
.find_container_process(cid.as_str(), eid.as_str())
|
||||
@@ -512,6 +520,15 @@ impl AgentService {
|
||||
sig = libc::SIGKILL;
|
||||
}
|
||||
|
||||
debug!(
|
||||
sl(),
|
||||
"signaling a container process";
|
||||
"container-id" => &cid,
|
||||
"exec-id" => &eid,
|
||||
"pid" => p.pid,
|
||||
"signal" => sig,
|
||||
);
|
||||
|
||||
match p.signal(sig) {
|
||||
Err(Errno::ESRCH) => {
|
||||
info!(
|
||||
@@ -526,10 +543,8 @@ impl AgentService {
|
||||
Err(err) => return Err(anyhow!(err)),
|
||||
Ok(()) => (),
|
||||
}
|
||||
};
|
||||
|
||||
if eid.is_empty() {
|
||||
// eid is empty, signal all the remaining processes in the container cgroup
|
||||
} else {
|
||||
// Signalling all processes in the cgroup
|
||||
info!(
|
||||
sl(),
|
||||
"signal all the remaining processes";
|
||||
|
||||
80
tests/integration/kubernetes/k8s-graceful-termination.bats
Normal file
80
tests/integration/kubernetes/k8s-graceful-termination.bats
Normal file
@@ -0,0 +1,80 @@
|
||||
#!/usr/bin/env bats
|
||||
#
|
||||
# Copyright (c) 2026 Schwarz Digits Cloud GmbH & Co. KG
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
#
|
||||
|
||||
load "${BATS_TEST_DIRNAME}/lib.sh"
|
||||
load "${BATS_TEST_DIRNAME}/../../common.bash"
|
||||
load "${BATS_TEST_DIRNAME}/tests_common.sh"
|
||||
|
||||
setup() {
|
||||
setup_common || die "setup_common failed"
|
||||
}
|
||||
|
||||
# Wait for a pod to reach a terminated state (Completed or Error).
|
||||
# Copyright (c) 2026 NVIDIA Corporation
|
||||
wait_for_pod_terminated() {
|
||||
local pod_name="$1"
|
||||
local elapsed=0
|
||||
local poll=5
|
||||
|
||||
while [ "${elapsed}" -lt "${wait_time}" ]; do
|
||||
local reason
|
||||
reason=$(kubectl get pod "${pod_name}" \
|
||||
-o jsonpath='{.status.containerStatuses[0].state.terminated.reason}' 2>/dev/null || true)
|
||||
if [[ "${reason}" == "Completed" ]] || [[ "${reason}" == "Error" ]]; then
|
||||
return 0
|
||||
fi
|
||||
sleep "${poll}"
|
||||
elapsed=$((elapsed + poll))
|
||||
done
|
||||
echo "Pod ${pod_name} did not terminate within ${wait_time}s" >&2
|
||||
return 1
|
||||
}
|
||||
|
||||
@test "On pod termination, SIGTERM is sent only once" {
|
||||
pod_name="graceful-termination-test"
|
||||
yaml_file="${pod_config_dir}/pod-graceful-termination.yaml"
|
||||
# Add policy to the yaml file
|
||||
policy_settings_dir="$(create_tmp_policy_settings_dir "${pod_config_dir}")"
|
||||
add_requests_to_policy_settings "${policy_settings_dir}" "GetDiagnosticDataRequest"
|
||||
auto_generate_policy "${policy_settings_dir}" "${yaml_file}"
|
||||
|
||||
# Create pod
|
||||
kubectl create -f "${yaml_file}"
|
||||
|
||||
# Add finalizer so the pod is not fully removed during the test
|
||||
kubectl patch pod "$pod_name" --patch '{"metadata":{"finalizers":["katacontainers.io/test-finalizer"]}}' --type=merge
|
||||
|
||||
# Check pod creation
|
||||
kubectl wait --for=condition=Ready --timeout=$timeout pod "$pod_name"
|
||||
|
||||
# Send a SIGTERM by deleting the pod
|
||||
kubectl delete pod "$pod_name" --wait=false
|
||||
|
||||
# Wait for pod deletion
|
||||
wait_for_pod_terminated "${pod_name}"
|
||||
|
||||
local reason
|
||||
reason=$(kubectl get pod "${pod_name}" \
|
||||
-o jsonpath='{.status.containerStatuses[0].state.terminated.reason}')
|
||||
[ "${reason}" = "Error" ] # Error since we rely on terminationGracePeriodSeconds to terminate the Pod
|
||||
|
||||
local message
|
||||
message=$(kubectl get pod "${pod_name}" \
|
||||
-o jsonpath='{.status.containerStatuses[0].state.terminated.message}')
|
||||
echo "termination message: ${message}"
|
||||
[ "${message}" = "start, received SIGTERM" ] # Must be this exact message, with only one SIGTERM
|
||||
|
||||
# Remove the finalizer to let kubernetes fully delete the pod
|
||||
kubectl patch pod "$pod_name" --patch '{"metadata":{"finalizers":null}}' --type=merge
|
||||
}
|
||||
|
||||
teardown() {
|
||||
kubectl patch pod "$pod_name" --patch '{"metadata":{"finalizers":null}}' --type=merge || true
|
||||
kubectl delete pod "$pod_name"
|
||||
delete_tmp_policy_settings_dir "${policy_settings_dir}"
|
||||
teardown_common "${node}" "${node_start_time:-}"
|
||||
}
|
||||
@@ -78,6 +78,7 @@ else
|
||||
"k8s-erofs-dmverity.bats" \
|
||||
"k8s-exec.bats" \
|
||||
"k8s-file-volume.bats" \
|
||||
"k8s-graceful-termination.bats" \
|
||||
"k8s-hostname.bats" \
|
||||
"k8s-hostpath-volume.bats" \
|
||||
"k8s-inotify.bats" \
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
#
|
||||
# Copyright (c) 2026 Schwarz Digits Cloud GmbH & Co. KG
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
#
|
||||
apiVersion: v1
|
||||
kind: Pod
|
||||
metadata:
|
||||
name: graceful-termination-test
|
||||
spec:
|
||||
runtimeClassName: kata
|
||||
securityContext:
|
||||
supplementalGroups: [10]
|
||||
containers:
|
||||
- name: sigterm-trap
|
||||
image: quay.io/prometheus/busybox:latest
|
||||
command:
|
||||
- "/bin/sh"
|
||||
- "-c"
|
||||
- |
|
||||
echo -n 'start' > /dev/termination-log
|
||||
trap "echo -n ', received SIGINT' >> /dev/termination-log; wait" SIGINT
|
||||
trap "echo -n ', received SIGTERM' >> /dev/termination-log; wait" SIGTERM
|
||||
tail -f /dev/null & wait
|
||||
# (We use wait to allow busybox ash to receive the TERM signal, see e.g. https://unix.stackexchange.com/a/387854)
|
||||
# (wait in the traps is needed so we continue waiting after the first wait is interrupted)
|
||||
# (Ultimatelly, the container will be killed by a SIGKILL after terminationGracePeriodSeconds)
|
||||
echo -n ', normal exit' >> /dev/termination-log
|
||||
terminationGracePeriodSeconds: 1
|
||||
Reference in New Issue
Block a user