Skip to content

check alive child process when delete task#3168

Closed
lifubang wants to merge 1 commit intocontainerd:masterfrom
lifubang:taskdelete
Closed

check alive child process when delete task#3168
lifubang wants to merge 1 commit intocontainerd:masterfrom
lifubang:taskdelete

Conversation

@lifubang
Copy link
Copy Markdown
Contributor

@lifubang lifubang commented Apr 3, 2019

As there are still shim service lock exists, we may check the container whether has alive child process or not before really delete the task. If has, raise an error to the caller to remind the user to kill all the child processes. Then the delete method without {all: true} will not be stuck.

One shim service lock case in #3149 : ( I think there are may be other cases we have not find out. )

# we use busybox rootfs and /test1 to start container
root@demo:/opt/busybox.test# cat rootfs/test1
#!/bin/sh
sleep 100000&
while true; do
  wait || true
done

# start container test
root@demo:/opt/busybox.test# ctr run -d --rootfs ./rootfs test /test1
root@demo:/opt/busybox.test# ctr t ls
TASK    PID      STATUS    
test    17974    RUNNING
root@demo:/opt/busybox.test# ctr t ps test
PID      INFO
17974    &ProcessDetails{ExecID:test,}
17995    -

# start container test-id with pid ns of container test
root@demo:/opt/busybox.test# ctr run -d --with-ns "pid:/proc/17974/ns/pid" --rootfs ./rootfs test-pid /test1
root@demo:/opt/busybox.test# ctr t ls
TASK        PID      STATUS    
test        17974    RUNNING
test-pid    18067    RUNNING
root@demo:/opt/busybox.test# ctr t ps test-pid
PID      INFO
18067    &ProcessDetails{ExecID:test-pid,}
18088    -

# container test-id's init process is dead
root@demo:/opt/busybox.test# ctr t kill -s 9 test-pid
root@demo:/opt/busybox.test# ctr t ps test-pid
PID      INFO
18088    -
root@demo:/opt/busybox.test# ctr t ls
TASK        PID      STATUS    
test        17974    RUNNING
test-pid    18067    STOPPED

# try to delete the task test-pid, it causes shim service stuck
root@demo:/opt/busybox.test# ctr t delete test-pid
^C
root@demo:/opt/busybox.test# ctr t ls
^C
root@demo:/opt/busybox.test# ctr t ps test
PID      INFO
17974    &ProcessDetails{ExecID:test,}
17995    -
root@demo:/opt/busybox.test# ctr t ps test-pid
^C
root@demo:/opt/busybox.test#

Signed-off-by: lifubang [email protected]

@Random-Liu
Copy link
Copy Markdown
Member

:( I'd rather always kill all in containerd to make it correct by default.

@lifubang
Copy link
Copy Markdown
Contributor Author

lifubang commented Apr 5, 2019

OK, let's wait us make a decision in #3149 .

@lifubang lifubang closed this Apr 5, 2019
@jterry75
Copy link
Copy Markdown
Contributor

@lifubang - Is it by contract that a call to delete with un-exited tasks/exec's will forcibly close these processes instead of returning an error? If so I for sure got this wrong in the Windows shim and will submit a fix. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants