search

containerd / go-runc

runc bindings for Go
Apache License 2.0
161 stars 71 forks source link

handle runc command context manually #28

Closed dqminh closed 2 years ago

dqminh commented 6 years ago

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

crosbymichael commented 6 years ago

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.

dqminh commented 6 years ago

@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 https://github.com/golang/go/issues/21135

crosbymichael commented 6 years ago

I replied about a simple API change to enable it.

dqminh commented 6 years ago

@crosbymichael since we cant settle on the upstream design, should we do this ?

crosbymichael commented 6 years ago

How do you feel about it? Is it worth the extra complexity in our code and the extra go routine?

dqminh commented 6 years ago

@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 ).

crosbymichael commented 6 years ago

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.