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 <dwalsh@redhat.com>

Closes: #158
Approved by: TomSweeneyRedHat
This commit is contained in:
Daniel J Walsh
2017-12-19 15:42:30 -05:00
committed by Atomic Bot
parent 3607fcb553
commit 1f49f555af
12 changed files with 50 additions and 18 deletions

View File

@ -393,6 +393,7 @@ var createFlags = []cli.Flag{
cli.IntFlag{ cli.IntFlag{
Name: "stop-timeout", Name: "stop-timeout",
Usage: "Timeout (in seconds) to stop a container. Default is 10", Usage: "Timeout (in seconds) to stop a container. Default is 10",
Value: libpod.CtrRemoveTimeout,
}, },
cli.StringSliceFlag{ cli.StringSliceFlag{
Name: "storage-opt", Name: "storage-opt",

View File

@ -110,7 +110,7 @@ type createConfig struct {
ShmDir string ShmDir string
SigProxy bool //sig-proxy SigProxy bool //sig-proxy
StopSignal syscall.Signal // stop-signal StopSignal syscall.Signal // stop-signal
StopTimeout int64 // stop-timeout StopTimeout uint // stop-timeout
StorageOpts []string //storage-opt StorageOpts []string //storage-opt
Sysctl map[string]string //sysctl Sysctl map[string]string //sysctl
Tmpfs []string // tmpfs Tmpfs []string // tmpfs
@ -494,7 +494,7 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er
ShmDir: shmDir, ShmDir: shmDir,
SigProxy: c.Bool("sig-proxy"), SigProxy: c.Bool("sig-proxy"),
StopSignal: stopSignal, StopSignal: stopSignal,
StopTimeout: c.Int64("stop-timeout"), StopTimeout: c.Uint("stop-timeout"),
StorageOpts: c.StringSlice("storage-opt"), StorageOpts: c.StringSlice("storage-opt"),
Sysctl: sysctl, Sysctl: sysctl,
Tmpfs: c.StringSlice("tmpfs"), Tmpfs: c.StringSlice("tmpfs"),

View File

@ -559,7 +559,7 @@ func (c *createConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, er
// TODO should not happen if --net=host // TODO should not happen if --net=host
options = append(options, libpod.WithNetNS([]ocicni.PortMapping{})) options = append(options, libpod.WithNetNS([]ocicni.PortMapping{}))
options = append(options, libpod.WithStopSignal(c.StopSignal)) options = append(options, libpod.WithStopSignal(c.StopSignal))
options = append(options, libpod.WithStopTimeout(c.StopTimeout))
return options, nil return options, nil
} }

View File

@ -6,17 +6,15 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/projectatomic/libpod/libpod" "github.com/projectatomic/libpod/libpod"
"github.com/sirupsen/logrus"
"github.com/urfave/cli" "github.com/urfave/cli"
) )
var ( var (
defaultTimeout int64 = 10
stopFlags = []cli.Flag{ stopFlags = []cli.Flag{
cli.Int64Flag{ cli.UintFlag{
Name: "timeout, t", Name: "timeout, t",
Usage: "Seconds to wait for stop before killing the container", Usage: "Seconds to wait for stop before killing the container",
Value: defaultTimeout, Value: libpod.CtrRemoveTimeout,
}, },
cli.BoolFlag{ cli.BoolFlag{
Name: "all, a", Name: "all, a",
@ -43,7 +41,6 @@ var (
func stopCmd(c *cli.Context) error { func stopCmd(c *cli.Context) error {
args := c.Args() args := c.Args()
stopTimeout := c.Int64("timeout")
if c.Bool("all") && len(args) > 0 { if c.Bool("all") && len(args) > 0 {
return errors.Errorf("no arguments are needed with -a") return errors.Errorf("no arguments are needed with -a")
} }
@ -60,8 +57,6 @@ func stopCmd(c *cli.Context) error {
} }
defer runtime.Shutdown(false) defer runtime.Shutdown(false)
logrus.Debugf("Stopping containers with timeout %d", stopTimeout)
var filterFuncs []libpod.ContainerFilter var filterFuncs []libpod.ContainerFilter
var containers []*libpod.Container var containers []*libpod.Container
var lastError error var lastError error
@ -91,6 +86,12 @@ func stopCmd(c *cli.Context) error {
} }
for _, ctr := range containers { 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 err := ctr.Stop(stopTimeout); err != nil {
if lastError != nil { if lastError != nil {
fmt.Fprintln(os.Stderr, lastError) fmt.Fprintln(os.Stderr, lastError)

View File

@ -146,6 +146,8 @@ type ContainerConfig struct {
Mounts []string `json:"mounts,omitempty"` Mounts []string `json:"mounts,omitempty"`
// StopSignal is the signal that will be used to stop the container // StopSignal is the signal that will be used to stop the container
StopSignal uint `json:"stopSignal,omitempty"` 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 // Shared namespaces with container
SharedNamespaceCtr *string `json:"shareNamespacesWith,omitempty"` SharedNamespaceCtr *string `json:"shareNamespacesWith,omitempty"`
SharedNamespaceMap map[string]string `json:"sharedNamespaces"` 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 // 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. // seconds), uses SIGKILL to attempt to forcibly stop the container.
// If timeout is 0, SIGKILL will be used immediately // 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() c.lock.Lock()
defer c.lock.Unlock() defer c.lock.Unlock()
logrus.Debugf("Stopping ctr %s with timeout %d", c.ID(), timeout)
if err := c.syncContainer(); err != nil { if err := c.syncContainer(); err != nil {
return err return err
@ -1140,3 +1143,8 @@ func (c *Container) copyHostFileToRundir(sourcePath string) (string, error) {
} }
return destFileName, nil return destFileName, nil
} }
// StopTimeout returns a stop timeout field for this container
func (c *Container) StopTimeout() uint {
return c.config.StopTimeout
}

View File

@ -401,7 +401,7 @@ func (r *OCIRuntime) killContainer(ctr *Container, signal uint) error {
// immediately kill with SIGKILL // immediately kill with SIGKILL
// Does not set finished time for container, assumes you will run updateStatus // Does not set finished time for container, assumes you will run updateStatus
// after to pull the exit code // 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 // Ping the container to see if it's alive
// If it's not, it's already stopped, return // If it's not, it's already stopped, return
err := unix.Kill(ctr.state.PID, 0) err := unix.Kill(ctr.state.PID, 0)

View File

@ -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 // WithNetNS indicates that the container should be given a new network
// namespace with a minimal configuration // namespace with a minimal configuration
// An optional array of port mappings can be provided // An optional array of port mappings can be provided

View File

@ -277,7 +277,7 @@ func (r *Runtime) Shutdown(force bool) error {
logrus.Errorf("Error retrieving containers from database: %v", err) logrus.Errorf("Error retrieving containers from database: %v", err)
} else { } else {
for _, ctr := range ctrs { 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) logrus.Errorf("Error stopping container %s: %v", ctr.ID(), err)
} }
} }

View File

@ -9,7 +9,9 @@ import (
"github.com/sirupsen/logrus" "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 // Contains the public Runtime API for containers
@ -34,6 +36,7 @@ func (r *Runtime) NewContainer(spec *spec.Spec, options ...CtrCreateOption) (c *
if err != nil { if err != nil {
return nil, err return nil, err
} }
ctr.config.StopTimeout = CtrRemoveTimeout
for _, option := range options { for _, option := range options {
if err := option(ctr); err != nil { 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 // Check that the container's in a good state to be removed
if c.state.State == ContainerStateRunning && force { 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()) return errors.Wrapf(err, "cannot remove container %s as it could not be stopped", c.ID())
} }

View File

@ -15,7 +15,7 @@ import (
// DBSchema is the current DB schema version // DBSchema is the current DB schema version
// Increments every time a change is made to the database's tables // 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 // SQLState is a state implementation backed by a persistent SQLite3 database
type SQLState struct { type SQLState struct {
@ -271,7 +271,7 @@ func (s *SQLState) HasContainer(id string) (bool, error) {
func (s *SQLState) AddContainer(ctr *Container) (err error) { func (s *SQLState) AddContainer(ctr *Container) (err error) {
const ( const (
addCtr = `INSERT INTO containers VALUES ( addCtr = `INSERT INTO containers VALUES (
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
);` );`
addCtrState = `INSERT INTO containerState VALUES ( addCtrState = `INSERT INTO containerState VALUES (
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
@ -332,6 +332,7 @@ func (s *SQLState) AddContainer(ctr *Container) (err error) {
boolToSQL(ctr.config.Stdin), boolToSQL(ctr.config.Stdin),
string(labelsJSON), string(labelsJSON),
ctr.config.StopSignal, ctr.config.StopSignal,
ctr.config.StopTimeout,
timeToSQL(ctr.config.CreatedTime), timeToSQL(ctr.config.CreatedTime),
ctr.config.RootfsImageID, ctr.config.RootfsImageID,
ctr.config.RootfsImageName, ctr.config.RootfsImageName,

View File

@ -176,6 +176,7 @@ func prepareDB(db *sql.DB) (err error) {
Stdin INTEGER NOT NULL, Stdin INTEGER NOT NULL,
LabelsJSON TEXT NOT NULL, LabelsJSON TEXT NOT NULL,
StopSignal INTEGER NOT NULL, StopSignal INTEGER NOT NULL,
StopTimeout INTEGER NOT NULL,
CreatedTime TEXT NOT NULL, CreatedTime TEXT NOT NULL,
RootfsImageID TEXT NOT NULL, RootfsImageID TEXT NOT NULL,
RootfsImageName TEXT NOT NULL, RootfsImageName TEXT NOT NULL,
@ -284,6 +285,7 @@ func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, lockDir
stdin int stdin int
labelsJSON string labelsJSON string
stopSignal uint stopSignal uint
stopTimeout uint
createdTimeString string createdTimeString string
rootfsImageID string rootfsImageID string
rootfsImageName string rootfsImageName string
@ -313,6 +315,7 @@ func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, lockDir
&stdin, &stdin,
&labelsJSON, &labelsJSON,
&stopSignal, &stopSignal,
&stopTimeout,
&createdTimeString, &createdTimeString,
&rootfsImageID, &rootfsImageID,
&rootfsImageName, &rootfsImageName,
@ -351,6 +354,7 @@ func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, lockDir
ctr.config.StaticDir = staticDir ctr.config.StaticDir = staticDir
ctr.config.Stdin = boolFromSQL(stdin) ctr.config.Stdin = boolFromSQL(stdin)
ctr.config.StopSignal = stopSignal ctr.config.StopSignal = stopSignal
ctr.config.StopTimeout = stopTimeout
ctr.state.State = ContainerState(state) ctr.state.State = ContainerState(state)
ctr.state.ConfigPath = configPath ctr.state.ConfigPath = configPath

View File

@ -26,6 +26,7 @@ func getTestContainer(id, name, locksDir string) (*Container, error) {
Stdin: true, Stdin: true,
Labels: make(map[string]string), Labels: make(map[string]string),
StopSignal: 0, StopSignal: 0,
StopTimeout: 0,
CreatedTime: time.Now(), CreatedTime: time.Now(),
}, },
state: &containerRuntimeInfo{ state: &containerRuntimeInfo{