From 8381f3feeebbbeef269909e4abba83191c8d9590 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 12 Oct 2020 14:25:20 -0400 Subject: Add a shutdown handler package 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 --- libpod/shutdown/handler.go | 105 +++++++++++++++++++++++++++++++++++++++++++++ pkg/api/server/server.go | 17 +++++--- 2 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 libpod/shutdown/handler.go diff --git a/libpod/shutdown/handler.go b/libpod/shutdown/handler.go new file mode 100644 index 000000000..79f236ab6 --- /dev/null +++ b/libpod/shutdown/handler.go @@ -0,0 +1,105 @@ +package shutdown + +import ( + "os" + "os/signal" + "sync" + "syscall" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +var ( + stopped bool + sigChan chan os.Signal + cancelChan chan bool + handlers map[string]func() error + shutdownInhibit sync.RWMutex +) + +// 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 { + // Already running, do nothing. + return nil + } + + sigChan = make(chan os.Signal, 1) + cancelChan = make(chan bool, 1) + stopped = false + + signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) + + go func() { + select { + case <-cancelChan: + signal.Stop(sigChan) + close(sigChan) + close(cancelChan) + stopped = true + return + case sig := <-sigChan: + logrus.Infof("Received shutdown signal %v, terminating!", sig) + shutdownInhibit.Lock() + for name, handler := range handlers { + logrus.Infof("Invoking shutdown handler %s", name) + if err := handler(); err != nil { + logrus.Errorf("Error running shutdown handler %s: %v", name, err) + } + } + shutdownInhibit.Unlock() + return + } + }() + + return nil +} + +// Stop the shutdown signal handler. +func Stop() error { + if cancelChan == nil { + return errors.New("shutdown signal handler has not yet been started") + } + if stopped { + return nil + } + + cancelChan <- true + + return nil +} + +// Temporarily inhibit signals from shutting down Libpod. +func Inhibit() { + shutdownInhibit.RLock() +} + +// Stop inhibiting signals from shutting down Libpod. +func Uninhibit() { + shutdownInhibit.RUnlock() +} + +// Register registers a function that will be executed when Podman is terminated +// by a signal. +func Register(name string, handler func() error) error { + if _, ok := handlers[name]; ok { + return errors.Errorf("handler with name %s already exists", name) + } + + handlers[name] = handler + + return nil +} + +// Unregister un-registers a given shutdown handler. +func Unregister(name string) error { + if _, ok := handlers[name]; !ok { + return errors.Errorf("no handler with name %s found", name) + } + + delete(handlers, name) + + return nil +} diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index 355a46fb7..cc5b45a65 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -7,7 +7,6 @@ import ( "net" "net/http" "os" - "os/signal" goRuntime "runtime" "strings" "sync" @@ -15,6 +14,7 @@ import ( "time" "github.com/containers/podman/v2/libpod" + "github.com/containers/podman/v2/libpod/shutdown" "github.com/containers/podman/v2/pkg/api/handlers" "github.com/containers/podman/v2/pkg/api/server/idle" "github.com/coreos/go-systemd/v22/activation" @@ -180,8 +180,17 @@ func setupSystemd() { // Serve starts responding to HTTP requests. func (s *APIServer) Serve() error { setupSystemd() - sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) + + // Start the shutdown signal handler. + if err := shutdown.Start(); err != nil { + return err + } + if err := shutdown.Register("server", func() error { + return s.Shutdown() + }); err != nil { + return err + } + errChan := make(chan error, 1) go func() { @@ -220,8 +229,6 @@ func (s *APIServer) Serve() error { select { case err := <-errChan: return err - case sig := <-sigChan: - logrus.Infof("APIServer terminated by signal %v", sig) } return nil -- cgit v1.2.3-54-g00ecf From 83e6e4ccdd925fa25500cff9e4b631b2c5d157cb Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 12 Oct 2020 15:10:52 -0400 Subject: Enable masking stop signals within container creation 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 #7941 Signed-off-by: Matthew Heon --- libpod/runtime.go | 13 +++++++++++++ libpod/runtime_ctr.go | 5 +++++ libpod/shutdown/handler.go | 8 ++++++++ pkg/api/server/server.go | 10 ++++------ pkg/domain/infra/abi/terminal/sigproxy_linux.go | 5 +++++ 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/libpod/runtime.go b/libpod/runtime.go index 7da8b181f..1118264f0 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -17,6 +17,7 @@ import ( "github.com/containers/podman/v2/libpod/events" "github.com/containers/podman/v2/libpod/image" "github.com/containers/podman/v2/libpod/lock" + "github.com/containers/podman/v2/libpod/shutdown" "github.com/containers/podman/v2/pkg/cgroups" "github.com/containers/podman/v2/pkg/registries" "github.com/containers/podman/v2/pkg/rootless" @@ -174,9 +175,21 @@ func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...R } } + if err := shutdown.Start(); err != nil { + return nil, errors.Wrapf(err, "error starting shutdown signal handler") + } + if err := makeRuntime(ctx, runtime); err != nil { return nil, err } + + if err := shutdown.Register("libpod", func() error { + os.Exit(1) + return nil + }); err != nil { + logrus.Errorf("Error registering shutdown handler for libpod: %v", err) + } + return runtime, nil } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 51b4c5f03..de73a9ff3 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -12,6 +12,7 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/libpod/events" + "github.com/containers/podman/v2/libpod/shutdown" "github.com/containers/podman/v2/pkg/cgroups" "github.com/containers/podman/v2/pkg/rootless" "github.com/containers/storage" @@ -149,6 +150,10 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai return nil, err } + // Inhibit shutdown until creation succeeds + shutdown.Inhibit() + defer shutdown.Uninhibit() + // Allocate a lock for the container lock, err := r.lockManager.AllocateLock() if err != nil { diff --git a/libpod/shutdown/handler.go b/libpod/shutdown/handler.go index 79f236ab6..7abaf065b 100644 --- a/libpod/shutdown/handler.go +++ b/libpod/shutdown/handler.go @@ -84,6 +84,10 @@ func Uninhibit() { // Register registers a function that will be executed when Podman is terminated // by a signal. func Register(name string, handler func() error) error { + if handlers == nil { + handlers = make(map[string]func() error) + } + if _, ok := handlers[name]; ok { return errors.Errorf("handler with name %s already exists", name) } @@ -95,6 +99,10 @@ func Register(name string, handler func() error) error { // Unregister un-registers a given shutdown handler. func Unregister(name string) error { + if handlers == nil { + handlers = make(map[string]func() error) + } + if _, ok := handlers[name]; !ok { return errors.Errorf("no handler with name %s found", name) } diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index cc5b45a65..24ad1874b 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -190,6 +190,9 @@ func (s *APIServer) Serve() error { }); err != nil { return err } + // Unregister the libpod handler, which just calls exit(1). + // Ignore errors if it doesn't exist. + _ = shutdown.Unregister("libpod") errChan := make(chan error, 1) @@ -226,12 +229,7 @@ func (s *APIServer) Serve() error { errChan <- nil }() - select { - case err := <-errChan: - return err - } - - return nil + return <-errChan } // Shutdown is a clean shutdown waiting on existing clients diff --git a/pkg/domain/infra/abi/terminal/sigproxy_linux.go b/pkg/domain/infra/abi/terminal/sigproxy_linux.go index f484e926c..0c586cf5c 100644 --- a/pkg/domain/infra/abi/terminal/sigproxy_linux.go +++ b/pkg/domain/infra/abi/terminal/sigproxy_linux.go @@ -5,12 +5,17 @@ import ( "syscall" "github.com/containers/podman/v2/libpod" + "github.com/containers/podman/v2/libpod/shutdown" "github.com/containers/podman/v2/pkg/signal" "github.com/sirupsen/logrus" ) // ProxySignals ... func ProxySignals(ctr *libpod.Container) { + // Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going + // to the container now. + shutdown.Stop() + sigBuffer := make(chan os.Signal, 128) signal.CatchAll(sigBuffer) -- cgit v1.2.3-54-g00ecf From f58d2f5e75b4982774509847c18b39f4a50fd5be Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 13 Oct 2020 14:00:35 -0400 Subject: Enforce LIFO ordering for shutdown handlers 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 --- libpod/runtime.go | 2 +- libpod/shutdown/handler.go | 42 ++++++++++++++++++++++++++++++------------ pkg/api/server/server.go | 5 +---- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/libpod/runtime.go b/libpod/runtime.go index 1118264f0..ccd920ab0 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -183,7 +183,7 @@ func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...R return nil, err } - if err := shutdown.Register("libpod", func() error { + if err := shutdown.Register("libpod", func(sig os.Signal) error { os.Exit(1) return nil }); err != nil { diff --git a/libpod/shutdown/handler.go b/libpod/shutdown/handler.go index 7abaf065b..87538dec9 100644 --- a/libpod/shutdown/handler.go +++ b/libpod/shutdown/handler.go @@ -11,17 +11,20 @@ import ( ) var ( - stopped bool - sigChan chan os.Signal - cancelChan chan bool - handlers map[string]func() error + stopped bool + sigChan chan os.Signal + cancelChan chan bool + // Definitions of all on-shutdown handlers + handlers map[string]func(os.Signal) error + // Ordering that on-shutdown handlers will be invoked. + handlerOrder []string shutdownInhibit sync.RWMutex ) // 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 { + if sigChan != nil { // Already running, do nothing. return nil } @@ -43,9 +46,14 @@ func Start() error { case sig := <-sigChan: logrus.Infof("Received shutdown signal %v, terminating!", sig) shutdownInhibit.Lock() - for name, handler := range handlers { + for _, name := range handlerOrder { + handler, ok := handlers[name] + if !ok { + logrus.Errorf("Shutdown handler %s definition not found!", name) + continue + } logrus.Infof("Invoking shutdown handler %s", name) - if err := handler(); err != nil { + if err := handler(sig); err != nil { logrus.Errorf("Error running shutdown handler %s: %v", name, err) } } @@ -82,10 +90,11 @@ func Uninhibit() { } // Register registers a function that will be executed when Podman is terminated -// by a signal. -func Register(name string, handler func() error) error { +// by a signal. Handlers are invoked LIFO - the last handler registered is the +// first run. +func Register(name string, handler func(os.Signal) error) error { if handlers == nil { - handlers = make(map[string]func() error) + handlers = make(map[string]func(os.Signal) error) } if _, ok := handlers[name]; ok { @@ -93,6 +102,7 @@ func Register(name string, handler func() error) error { } handlers[name] = handler + handlerOrder = append([]string{name}, handlerOrder...) return nil } @@ -100,14 +110,22 @@ func Register(name string, handler func() error) error { // Unregister un-registers a given shutdown handler. func Unregister(name string) error { if handlers == nil { - handlers = make(map[string]func() error) + return nil } if _, ok := handlers[name]; !ok { - return errors.Errorf("no handler with name %s found", name) + return nil } delete(handlers, name) + newOrder := []string{} + for _, checkName := range handlerOrder { + if checkName != name { + newOrder = append(newOrder, checkName) + } + } + handlerOrder = newOrder + return nil } diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index 24ad1874b..64008767b 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -185,14 +185,11 @@ func (s *APIServer) Serve() error { if err := shutdown.Start(); err != nil { return err } - if err := shutdown.Register("server", func() error { + if err := shutdown.Register("server", func(sig os.Signal) error { return s.Shutdown() }); err != nil { return err } - // Unregister the libpod handler, which just calls exit(1). - // Ignore errors if it doesn't exist. - _ = shutdown.Unregister("libpod") errChan := make(chan error, 1) -- cgit v1.2.3-54-g00ecf