Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a shutdown signal handler #7999

Merged
merged 3 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions libpod/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
113 changes: 113 additions & 0 deletions libpod/shutdown/handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would appear to be a race on reading/setting stopped

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me just remove that, it's not important that we be able to restart the handler right now.

// 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 handlers == nil {
handlers = make(map[string]func() error)
}

if _, ok := handlers[name]; ok {
return errors.Errorf("handler with name %s already exists", name)
}
Comment on lines +100 to +102
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if _, ok := handlers[name]; ok {
return errors.Errorf("handler with name %s already exists", name)
}

Why not just reset handler to new func?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


handlers[name] = handler

return nil
}

// 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)
}
jwhonce marked this conversation as resolved.
Show resolved Hide resolved

delete(handlers, name)

return nil
}
27 changes: 16 additions & 11 deletions pkg/api/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import (
"net"
"net/http"
"os"
"os/signal"
goRuntime "runtime"
"strings"
"sync"
"syscall"
"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"
Expand Down Expand Up @@ -180,8 +180,20 @@ 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
}
// Unregister the libpod handler, which just calls exit(1).
// Ignore errors if it doesn't exist.
_ = shutdown.Unregister("libpod")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems smelly to remove someone elses handler by name...


errChan := make(chan error, 1)

go func() {
Expand Down Expand Up @@ -217,14 +229,7 @@ func (s *APIServer) Serve() error {
errChan <- nil
}()

select {
case err := <-errChan:
return err
case sig := <-sigChan:
logrus.Infof("APIServer terminated by signal %v", sig)
}

return nil
return <-errChan
}

// Shutdown is a clean shutdown waiting on existing clients
Expand Down
5 changes: 5 additions & 0 deletions pkg/domain/infra/abi/terminal/sigproxy_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down