Add a shutdown signal handler #7999
Conversation
We need a unified package for handling signals that shut down Libpod and Podman. We need to be able to do different things on receiving such a signal (`system service` wants to shut down the service gracefully, while most other commands just want to exit) and we need to be able to inhibit this shutdown signal while we are waiting for some critical operations (e.g. creating a container) to finish. This takes the first step by defining the package that will handle this. Signed-off-by: Matthew Heon <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
Potential improvements:
|
mheon
force-pushed
the
signal_handler
branch
2 times, most recently
from
d8a0ccf to
3e8a118
Compare
Expand the use of the Shutdown package such that we now use it to handle signals any time we run Libpod. From there, add code to container creation to use the Inhibit function to prevent a shutdown from occuring during the critical parts of container creation. We also need to turn off signal handling when --sig-proxy is invoked - we don't want to catch the signals ourselves then, but instead to forward them into the container via the existing sig-proxy handler. Fixes containers#7941 Signed-off-by: Matthew Heon <[email protected]>
|
@jwhonce PTAL, I'd like to make sure I got signal handling for the server right. |
libpod/shutdown/handler.go
Outdated
| // Start begins handling SIGTERM and SIGINT and will run the given on-signal | ||
| // handlers when one is called. This can be cancelled by calling Stop(). | ||
| func Start() error { | ||
| if sigChan != nil && !stopped { |
There was a problem hiding this comment.
It would appear to be a race on reading/setting stopped
There was a problem hiding this comment.
Let me just remove that, it's not important that we be able to restart the handler right now.
| if _, ok := handlers[name]; ok { | ||
| return errors.Errorf("handler with name %s already exists", name) | ||
| } |
There was a problem hiding this comment.
| if _, ok := handlers[name]; ok { | |
| return errors.Errorf("handler with name %s already exists", name) | |
| } |
Why not just reset handler to new func?
There was a problem hiding this comment.
I'd like to allow multiple handlers at the same time - theoretically the Libpod handler could run after the server handler and do any extra shutdown needed there.
pkg/api/server/server.go
Outdated
| } | ||
| // Unregister the libpod handler, which just calls exit(1). | ||
| // Ignore errors if it doesn't exist. | ||
| _ = shutdown.Unregister("libpod") |
There was a problem hiding this comment.
Seems smelly to remove someone elses handler by name...
|
@jwhonce PTAL again, comments should be fixed |
This allows us to run both the Libpod and Server handlers at the same time without unregistering one. Also, pass the signal that killed us into the handlers, in case they want to use it to determine what to do (e.g. what exit code to set). Signed-off-by: Matthew Heon <[email protected]>
|
Restarted test flakes /lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
We need a unified package for handling signals that shut down Libpod and Podman. We need to be able to do different things on receiving such a signal (
system servicewants to shut down the service gracefully, while most other commands just want to exit) and we need to be able to inhibit this shutdown signal while we are waiting for some critical operations (e.g. creating a container) to finish. We also want to turn off this handling when--sig-proxyis active.Fixes #7941