Skip to content
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

test: add retry for killing container with host pidns #4164

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lifubang
Copy link
Member

@lifubang lifubang commented Jan 7, 2024

When testing kill container with host pidns:

  1. After the init process gone, the container state will become stopped, so no need to wait it becomes stopped after killing;
  2. After we run runc kill cid KILL, there are still some process exists in the cgroup if we view it immediately, so add a retry to wait all processes gone.

Fix: #4163

@lifubang lifubang force-pushed the fix-test-kill-hostpidns branch 2 times, most recently from ee034a4 to c9fd3bd Compare January 7, 2024 06:48
Signed-off-by: lifubang <lifubang@acmcoder.com>
# Make sure all processes are gone.
pids=$(cat "$cgpath"/cgroup.procs) || true # OK if cgroup is gone
echo "pids: $pids"
if [ -n "$pids" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need for this condition

while [ $retry -lt 10 ] && [ -n "$pids" ]; do
# Make sure all processes are gone.
pids=$(cat "$cgpath"/cgroup.procs) || true # OK if cgroup is gone
echo "pids: $pids"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add retry counter here, e.g.

echo "try $retry; pids: $pids"

pids=$(cat "$cgpath"/cgroup.procs) || true # OK if cgroup is gone
echo "pids: $pids"
retry=0
while [ $retry -lt 10 ] && [ -n "$pids" ]; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do

while [ $((retry++)) -lt 10 ] && [ -n "$pids" ]; do

@kolyshkin
Copy link
Contributor

@lifubang please see #4179 as an alternative. Basically we want until those processes are gone.

Also, maybe it's better to move the logic to runc itself, i.e. make runc kill and runc delete -f wait till pids are gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flaky test: kill KILL [host pidns]
2 participants