CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
containerd-shim-runc-v2: return init pid when clean dead shim #6452
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
Conversation
Hi @zvier. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
related #6402 |
Build succeeded.
|
Build succeeded.
|
If containerd-shim-runc-v2 process dead abnormally, such as received kill 9 signal, panic or other unkown reasons, the containerd-shim-runc-v2 server can not reap runc container and forward init process exit event. This will lead the container leaked in dockerd. When shim dead, containerd will clean dead shim, here read init process pid and forward exit event with pid at the same time. Signed-off-by: Jeff Zvier <zvier20@gmail.com>
Build succeeded.
|
@kzys Hello, I had improved this PR last week, can you take a look again ? |
if err != nil { | ||
log.G(ctx).WithError(err).Warn("failed to read init pid file") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about returning this error instead of logging? The function should return either a valid value or error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is used to cleanup the resource, which is handled in command-line.
It should not return error here.
@kzys Hello, please review this PR agagin ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on Green.
Tested in my local
# Terminal 1
2022-02-09 14:59:07.994652691 +0000 UTC moby /tasks/exit {"container_id":"8857863cb748d2835781232a7d99e5ba4c0c497f6210513490b4add719993680","id":"8857863cb748d2835781232a7d99e5ba4c0c497f6210513490b4add719993680","pid":206640,"exit_status":137,"exited_at":"2022-02-09T14:59:07.993201143Z"}
2022-02-09 14:59:07.994680201 +0000 UTC moby /tasks/delete {"container_id":"8857863cb748d2835781232a7d99e5ba4c0c497f6210513490b4add719993680","pid":206640,"exit_status":137,"exited_at":"2022-02-09T14:59:07.993201143Z"}
2022-02-09 14:59:08.186136836 +0000 UTC moby /containers/delete {"id":"8857863cb748d2835781232a7d99e5ba4c0c497f6210513490b4add719993680"}
# Terminal 2
âžś containerd git:(review-6452) docker run -d busybox sleep 1d
8857863cb748d2835781232a7d99e5ba4c0c497f6210513490b4add719993680
âžś containerd git:(review-6452) ps -ef | grep sleep
root 206640 206619 0 22:58 ? 00:00:00 sleep 1d
chaofan 206700 152304 0 22:59 pts/0 00:00:00 grep --color=auto --exclude-dir=.bzr --exclude-dir=CVS --exclude-dir=.git --exclude-dir=.hg --exclude-dir=.svn --exclude-dir=.idea --exclude-dir=.tox sleep
âžś containerd git:(review-6452) sudo pkill -9 containerd-shim
âžś containerd git:(review-6452) docker ps -a | grep 88578
8857863cb748 busybox "sleep 1d" 2 minutes ago Exited (137) About a minute ago relaxed_leakey
/ok-to-test |
@dmcgowan Hello,can you review this PR ? |
/retest |
If containerd-shim-runc-v2 process dead abnormally, such as received
kill -s 9 signal, panic or other unkown reasons, the containerd-shim-runc-v2
server can not reap runc container and forward init process exit event.
This will lead the container leaked in dockerd. When shim dead, containerd
will clean dead shim, here read init process pid and forward exit event
with pid at the same time.
Signed-off-by: Jeff Zvier zvier20@gmail.com