containerd-shim-runc-v2: return init pid when clean dead shim#6452
containerd-shim-runc-v2: return init pid when clean dead shim#6452fuweid merged 1 commit intocontainerd:mainfrom
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. DetailsInstructions 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.
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.
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 ? |
fuweid
left a comment
There was a problem hiding this comment.
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