handle runc command context manually #28
Conversation
instead of defering to Go's stdlib to handle context, which only sends SIGKILL when the context timed out, handle the context timeout ourselves so we can inject a custom signal first ( default to SIGTERM, and send SIGKILL after 10 seconds) to stop container more gracefully. If no signal is specified, then fall back to sending SIGKILL just like stdlib. Fix containerd#21
|
How are we supposed to implement this in the reaper for containerd? Is this really worth fixing? If so, it maybe better to just update go's exec package to allow you to set a os.Signal on the context in the exec package that is used. |
|
@crosbymichael yah the reaper in containerd will need to be changed to handle context itself similar to how the current default reaper is changed. We have to handle signaling in Start / finish in Wait. I'm also trying to get this in stdlib, but so far there hasnt been any aggreements on the api yet golang/go#21135 |
|
I replied about a simple API change to enable it. |
|
@crosbymichael since we cant settle on the upstream design, should we do this ? |
|
How do you feel about it? Is it worth the extra complexity in our code and the extra go routine? |
|
@crosbymichael i think it makes sense to terminate gracefully. The goroutine is already there if you use CommandContext ( it creates one internally at the end of Start to watch for context timeout and sigkill ). |
| c.Process.Signal(unix.SIGKILL) | ||
| } else { | ||
| c.Process.Signal(m.defaultSignal) | ||
| if m.killTimeout > 0 { |
There was a problem hiding this comment.
If there's no killtimeout set better use a default value, the process may never die otherwise.
|
Do we have an idea how we will implement this in the reaper because its a little different than blocking in a go routine here. |
instead of defering to Go's stdlib to handle context, which only sends
SIGKILL when the context timed out, handle the context timeout ourselves
so we can inject a custom signal first ( default to SIGTERM, and send
SIGKILL after 10 seconds) to stop container more gracefully.
Fix #21
Replace #22