From 1f49f555af0709ea7c12becdb750ba60a00eaf1d Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 19 Dec 2017 15:42:30 -0500 Subject: Plumb through the --stop-timeout signal handling podman run/create have the ability to set the stop timeout flag. We need to stop it in the database. Also Allowing negative time for stop timeout makes no sense, so switching to timeout of uint, allows user to specify huge timeout values. Signed-off-by: Daniel J Walsh Closes: #158 Approved by: TomSweeneyRedHat --- cmd/podman/common.go | 1 + cmd/podman/create.go | 4 ++-- cmd/podman/spec.go | 2 +- cmd/podman/stop.go | 17 +++++++++-------- libpod/container.go | 10 +++++++++- libpod/oci.go | 2 +- libpod/options.go | 13 +++++++++++++ libpod/runtime.go | 2 +- libpod/runtime_ctr.go | 7 +++++-- libpod/sql_state.go | 5 +++-- libpod/sql_state_internal.go | 4 ++++ libpod/sql_state_test.go | 1 + 12 files changed, 50 insertions(+), 18 deletions(-) diff --git a/cmd/podman/common.go b/cmd/podman/common.go index 57e2ff717..e6c2645c9 100644 --- a/cmd/podman/common.go +++ b/cmd/podman/common.go @@ -393,6 +393,7 @@ var createFlags = []cli.Flag{ cli.IntFlag{ Name: "stop-timeout", Usage: "Timeout (in seconds) to stop a container. Default is 10", + Value: libpod.CtrRemoveTimeout, }, cli.StringSliceFlag{ Name: "storage-opt", diff --git a/cmd/podman/create.go b/cmd/podman/create.go index d3e14742b..8b64a1cb0 100644 --- a/cmd/podman/create.go +++ b/cmd/podman/create.go @@ -110,7 +110,7 @@ type createConfig struct { ShmDir string SigProxy bool //sig-proxy StopSignal syscall.Signal // stop-signal - StopTimeout int64 // stop-timeout + StopTimeout uint // stop-timeout StorageOpts []string //storage-opt Sysctl map[string]string //sysctl Tmpfs []string // tmpfs @@ -494,7 +494,7 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er ShmDir: shmDir, SigProxy: c.Bool("sig-proxy"), StopSignal: stopSignal, - StopTimeout: c.Int64("stop-timeout"), + StopTimeout: c.Uint("stop-timeout"), StorageOpts: c.StringSlice("storage-opt"), Sysctl: sysctl, Tmpfs: c.StringSlice("tmpfs"), diff --git a/cmd/podman/spec.go b/cmd/podman/spec.go index dc6e24c24..037b18950 100644 --- a/cmd/podman/spec.go +++ b/cmd/podman/spec.go @@ -559,7 +559,7 @@ func (c *createConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, er // TODO should not happen if --net=host options = append(options, libpod.WithNetNS([]ocicni.PortMapping{})) options = append(options, libpod.WithStopSignal(c.StopSignal)) - + options = append(options, libpod.WithStopTimeout(c.StopTimeout)) return options, nil } diff --git a/cmd/podman/stop.go b/cmd/podman/stop.go index 3b1ffbba5..f85512d47 100644 --- a/cmd/podman/stop.go +++ b/cmd/podman/stop.go @@ -6,17 +6,15 @@ import ( "github.com/pkg/errors" "github.com/projectatomic/libpod/libpod" - "github.com/sirupsen/logrus" "github.com/urfave/cli" ) var ( - defaultTimeout int64 = 10 - stopFlags = []cli.Flag{ - cli.Int64Flag{ + stopFlags = []cli.Flag{ + cli.UintFlag{ Name: "timeout, t", Usage: "Seconds to wait for stop before killing the container", - Value: defaultTimeout, + Value: libpod.CtrRemoveTimeout, }, cli.BoolFlag{ Name: "all, a", @@ -43,7 +41,6 @@ var ( func stopCmd(c *cli.Context) error { args := c.Args() - stopTimeout := c.Int64("timeout") if c.Bool("all") && len(args) > 0 { return errors.Errorf("no arguments are needed with -a") } @@ -60,8 +57,6 @@ func stopCmd(c *cli.Context) error { } defer runtime.Shutdown(false) - logrus.Debugf("Stopping containers with timeout %d", stopTimeout) - var filterFuncs []libpod.ContainerFilter var containers []*libpod.Container var lastError error @@ -91,6 +86,12 @@ func stopCmd(c *cli.Context) error { } for _, ctr := range containers { + var stopTimeout uint + if c.IsSet("timeout") { + stopTimeout = c.Uint("timeout") + } else { + stopTimeout = ctr.StopTimeout() + } if err := ctr.Stop(stopTimeout); err != nil { if lastError != nil { fmt.Fprintln(os.Stderr, lastError) diff --git a/libpod/container.go b/libpod/container.go index dc22c9c61..158bf7529 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -146,6 +146,8 @@ type ContainerConfig struct { Mounts []string `json:"mounts,omitempty"` // StopSignal is the signal that will be used to stop the container StopSignal uint `json:"stopSignal,omitempty"` + // StopTimeout is the signal that will be used to stop the container + StopTimeout uint `json:"stopTimeout,omitempty"` // Shared namespaces with container SharedNamespaceCtr *string `json:"shareNamespacesWith,omitempty"` SharedNamespaceMap map[string]string `json:"sharedNamespaces"` @@ -671,9 +673,10 @@ func (c *Container) Start() error { // to stop the container, and if it has not stopped after the given timeout (in // seconds), uses SIGKILL to attempt to forcibly stop the container. // If timeout is 0, SIGKILL will be used immediately -func (c *Container) Stop(timeout int64) error { +func (c *Container) Stop(timeout uint) error { c.lock.Lock() defer c.lock.Unlock() + logrus.Debugf("Stopping ctr %s with timeout %d", c.ID(), timeout) if err := c.syncContainer(); err != nil { return err @@ -1140,3 +1143,8 @@ func (c *Container) copyHostFileToRundir(sourcePath string) (string, error) { } return destFileName, nil } + +// StopTimeout returns a stop timeout field for this container +func (c *Container) StopTimeout() uint { + return c.config.StopTimeout +} diff --git a/libpod/oci.go b/libpod/oci.go index 49bf95bc5..c122071e3 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -401,7 +401,7 @@ func (r *OCIRuntime) killContainer(ctr *Container, signal uint) error { // immediately kill with SIGKILL // Does not set finished time for container, assumes you will run updateStatus // after to pull the exit code -func (r *OCIRuntime) stopContainer(ctr *Container, timeout int64) error { +func (r *OCIRuntime) stopContainer(ctr *Container, timeout uint) error { // Ping the container to see if it's alive // If it's not, it's already stopped, return err := unix.Kill(ctr.state.PID, 0) diff --git a/libpod/options.go b/libpod/options.go index a0894c0e3..cbce1f767 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -415,6 +415,19 @@ func WithStopSignal(signal syscall.Signal) CtrCreateOption { } } +// WithStopTimeout sets the time to after initial stop signal is sent to container, before sending the kill signal +func WithStopTimeout(timeout uint) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return ErrCtrFinalized + } + + ctr.config.StopTimeout = timeout + + return nil + } +} + // WithNetNS indicates that the container should be given a new network // namespace with a minimal configuration // An optional array of port mappings can be provided diff --git a/libpod/runtime.go b/libpod/runtime.go index 91c6c7683..59d7f7e3c 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -277,7 +277,7 @@ func (r *Runtime) Shutdown(force bool) error { logrus.Errorf("Error retrieving containers from database: %v", err) } else { for _, ctr := range ctrs { - if err := ctr.Stop(ctrRemoveTimeout); err != nil { + if err := ctr.Stop(CtrRemoveTimeout); err != nil { logrus.Errorf("Error stopping container %s: %v", ctr.ID(), err) } } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 320821b38..1f2b8945e 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -9,7 +9,9 @@ import ( "github.com/sirupsen/logrus" ) -const ctrRemoveTimeout = 10 +// CtrRemoveTimeout is the default number of seconds to wait after stopping a container +// before sending the kill signal +const CtrRemoveTimeout = 10 // Contains the public Runtime API for containers @@ -34,6 +36,7 @@ func (r *Runtime) NewContainer(spec *spec.Spec, options ...CtrCreateOption) (c * if err != nil { return nil, err } + ctr.config.StopTimeout = CtrRemoveTimeout for _, option := range options { if err := option(ctr); err != nil { @@ -122,7 +125,7 @@ func (r *Runtime) removeContainer(c *Container, force bool) error { // Check that the container's in a good state to be removed if c.state.State == ContainerStateRunning && force { - if err := r.ociRuntime.stopContainer(c, ctrRemoveTimeout); err != nil { + if err := r.ociRuntime.stopContainer(c, c.StopTimeout()); err != nil { return errors.Wrapf(err, "cannot remove container %s as it could not be stopped", c.ID()) } diff --git a/libpod/sql_state.go b/libpod/sql_state.go index a93a025dc..ec6818bcf 100644 --- a/libpod/sql_state.go +++ b/libpod/sql_state.go @@ -15,7 +15,7 @@ import ( // DBSchema is the current DB schema version // Increments every time a change is made to the database's tables -const DBSchema = 4 +const DBSchema = 5 // SQLState is a state implementation backed by a persistent SQLite3 database type SQLState struct { @@ -271,7 +271,7 @@ func (s *SQLState) HasContainer(id string) (bool, error) { func (s *SQLState) AddContainer(ctr *Container) (err error) { const ( addCtr = `INSERT INTO containers VALUES ( - ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? + ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? );` addCtrState = `INSERT INTO containerState VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? @@ -332,6 +332,7 @@ func (s *SQLState) AddContainer(ctr *Container) (err error) { boolToSQL(ctr.config.Stdin), string(labelsJSON), ctr.config.StopSignal, + ctr.config.StopTimeout, timeToSQL(ctr.config.CreatedTime), ctr.config.RootfsImageID, ctr.config.RootfsImageName, diff --git a/libpod/sql_state_internal.go b/libpod/sql_state_internal.go index e1ecc8ea6..8513216fa 100644 --- a/libpod/sql_state_internal.go +++ b/libpod/sql_state_internal.go @@ -176,6 +176,7 @@ func prepareDB(db *sql.DB) (err error) { Stdin INTEGER NOT NULL, LabelsJSON TEXT NOT NULL, StopSignal INTEGER NOT NULL, + StopTimeout INTEGER NOT NULL, CreatedTime TEXT NOT NULL, RootfsImageID TEXT NOT NULL, RootfsImageName TEXT NOT NULL, @@ -284,6 +285,7 @@ func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, lockDir stdin int labelsJSON string stopSignal uint + stopTimeout uint createdTimeString string rootfsImageID string rootfsImageName string @@ -313,6 +315,7 @@ func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, lockDir &stdin, &labelsJSON, &stopSignal, + &stopTimeout, &createdTimeString, &rootfsImageID, &rootfsImageName, @@ -351,6 +354,7 @@ func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, lockDir ctr.config.StaticDir = staticDir ctr.config.Stdin = boolFromSQL(stdin) ctr.config.StopSignal = stopSignal + ctr.config.StopTimeout = stopTimeout ctr.state.State = ContainerState(state) ctr.state.ConfigPath = configPath diff --git a/libpod/sql_state_test.go b/libpod/sql_state_test.go index 0bb0cd517..e990df6c9 100644 --- a/libpod/sql_state_test.go +++ b/libpod/sql_state_test.go @@ -26,6 +26,7 @@ func getTestContainer(id, name, locksDir string) (*Container, error) { Stdin: true, Labels: make(map[string]string), StopSignal: 0, + StopTimeout: 0, CreatedTime: time.Now(), }, state: &containerRuntimeInfo{ -- cgit v1.2.3-54-g00ecf