From e66d2108911a2fad016205bdd6bf181f7e822c1c Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Thu, 26 May 2016 13:34:48 -0700 Subject: [PATCH 1/2] Add config parameter to change per-container stop timeout during daemon shutdown This fix tries to add a flag `--stop-timeout` to specify the timeout value (in seconds) for the container to stop before SIGKILL is issued. If stop timeout is not specified then the default timeout (10s) is used. Additional test cases have been added to cover the change. This fix is related to #22471. Another pull request will add `--shutdown-timeout` to daemon for #22471. Signed-off-by: Yong Tang --- container/container.go | 13 ++++++++++ container/container_unit_test.go | 24 +++++++++++++++++++ daemon/daemon.go | 8 +++---- docs/reference/api/docker_remote_api.md | 1 + docs/reference/api/docker_remote_api_v1.25.md | 5 +++- docs/reference/commandline/create.md | 1 + docs/reference/commandline/run.md | 6 +++++ integration-cli/docker_cli_create_test.go | 15 ++++++++++++ man/docker-create.1.md | 4 ++++ man/docker-run.1.md | 4 ++++ runconfig/opts/parse.go | 5 ++++ 11 files changed, 81 insertions(+), 5 deletions(-) diff --git a/container/container.go b/container/container.go index 5ec598cb0d..59bcaa8962 100644 --- a/container/container.go +++ b/container/container.go @@ -44,6 +44,11 @@ import ( const configFileName = "config.v2.json" +const ( + // defaultStopTimeout is the timeout (in seconds) for the syscall signal used to stop a container. + defaultStopTimeout = 10 +) + var ( errInvalidEndpoint = fmt.Errorf("invalid endpoint while building port map info") errInvalidNetwork = fmt.Errorf("invalid network settings while building port map info") @@ -578,6 +583,14 @@ func (container *Container) StopSignal() int { return int(stopSignal) } +// StopTimeout returns the timeout (in seconds) used to stop the container. +func (container *Container) StopTimeout() int { + if container.Config.StopTimeout != nil { + return *container.Config.StopTimeout + } + return defaultStopTimeout +} + // InitDNSHostConfig ensures that the dns fields are never nil. // New containers don't ever have those fields nil, // but pre created containers can still have those nil values. diff --git a/container/container_unit_test.go b/container/container_unit_test.go index f14dc12e97..73bdf0fe13 100644 --- a/container/container_unit_test.go +++ b/container/container_unit_test.go @@ -34,3 +34,27 @@ func TestContainerStopSignal(t *testing.T) { t.Fatalf("Expected 9, got %v", s) } } + +func TestContainerStopTimeout(t *testing.T) { + c := &Container{ + CommonContainer: CommonContainer{ + Config: &container.Config{}, + }, + } + + s := c.StopTimeout() + if s != defaultStopTimeout { + t.Fatalf("Expected %v, got %v", defaultStopTimeout, s) + } + + stopTimeout := 15 + c = &Container{ + CommonContainer: CommonContainer{ + Config: &container.Config{StopTimeout: &stopTimeout}, + }, + } + s = c.StopSignal() + if s != 15 { + t.Fatalf("Expected 15, got %v", s) + } +} diff --git a/daemon/daemon.go b/daemon/daemon.go index b09c054285..218c9031cc 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -708,8 +708,8 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { if err := daemon.containerUnpause(c); err != nil { return fmt.Errorf("Failed to unpause container %s with error: %v", c.ID, err) } - if _, err := c.WaitStop(10 * time.Second); err != nil { - logrus.Debugf("container %s failed to exit in 10 seconds of SIGTERM, sending SIGKILL to force", c.ID) + if _, err := c.WaitStop(time.Duration(c.StopTimeout()) * time.Second); err != nil { + logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, c.StopTimeout()) sig, ok := signal.SignalMap["KILL"] if !ok { return fmt.Errorf("System does not support SIGKILL") @@ -721,8 +721,8 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { return err } } - // If container failed to exit in 10 seconds of SIGTERM, then using the force - if err := daemon.containerStop(c, 10); err != nil { + // If container failed to exit in c.StopTimeout() seconds of SIGTERM, then using the force + if err := daemon.containerStop(c, c.StopTimeout()); err != nil { return fmt.Errorf("Failed to stop container %s with error: %v", c.ID, err) } diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md index 741110d730..1b6617c45a 100644 --- a/docs/reference/api/docker_remote_api.md +++ b/docs/reference/api/docker_remote_api.md @@ -128,6 +128,7 @@ This section lists each version from latest to oldest. Each listing includes a * `DELETE /containers/(name)` endpoint now returns an error of `removal of container name is already in progress` with status code of 400, when container name is in a state of removal in progress. * `GET /containers/json` now supports a `is-task` filter to filter containers that are tasks (part of a service in swarm mode). +* `POST /containers/create` now takes `StopTimeout` field. ### v1.24 API changes diff --git a/docs/reference/api/docker_remote_api_v1.25.md b/docs/reference/api/docker_remote_api_v1.25.md index 10f8acedb5..b1b716cbc6 100644 --- a/docs/reference/api/docker_remote_api_v1.25.md +++ b/docs/reference/api/docker_remote_api_v1.25.md @@ -284,6 +284,7 @@ Create a container "22/tcp": {} }, "StopSignal": "SIGTERM", + "StopTimeout": 10, "HostConfig": { "Binds": ["/tmp:/tmp"], "Links": ["redis3:redis"], @@ -391,6 +392,7 @@ Create a container - **ExposedPorts** - An object mapping ports to an empty object in the form of: `"ExposedPorts": { "/: {}" }` - **StopSignal** - Signal to stop a container as a string or unsigned integer. `SIGTERM` by default. +- **StopTimeout** - Timeout (in seconds) to stop a container. 10 by default. - **HostConfig** - **Binds** – A list of volume bindings for this container. Each volume binding is a string in one of these forms: + `host-src:container-dest` to bind-mount a host path into the @@ -580,7 +582,8 @@ Return low-level information on the container `id` "/volumes/data": {} }, "WorkingDir": "", - "StopSignal": "SIGTERM" + "StopSignal": "SIGTERM", + "StopTimeout": 10 }, "Created": "2015-01-06T15:47:31.485331387Z", "Driver": "devicemapper", diff --git a/docs/reference/commandline/create.md b/docs/reference/commandline/create.md index d6c584d079..247e67f656 100644 --- a/docs/reference/commandline/create.md +++ b/docs/reference/commandline/create.md @@ -94,6 +94,7 @@ Options: Unit is optional and can be `b` (bytes), `k` (kilobytes), `m` (megabytes), or `g` (gigabytes). If you omit the unit, the system uses bytes. --stop-signal string Signal to stop a container, SIGTERM by default (default "SIGTERM") + --stop-timeout=10 Timeout (in seconds) to stop a container --storage-opt value Storage driver options for the container (default []) --sysctl value Sysctl options (default map[]) --tmpfs value Mount a tmpfs directory (default []) diff --git a/docs/reference/commandline/run.md b/docs/reference/commandline/run.md index e22cbeafc4..17b0c1e94e 100644 --- a/docs/reference/commandline/run.md +++ b/docs/reference/commandline/run.md @@ -101,6 +101,7 @@ Options: or `g` (gigabytes). If you omit the unit, the system uses bytes. --sig-proxy Proxy received signals to the process (default true) --stop-signal string Signal to stop a container, SIGTERM by default (default "SIGTERM") + --stop-timeout=10 Timeout (in seconds) to stop a container --storage-opt value Storage driver options for the container (default []) --sysctl value Sysctl options (default map[]) --tmpfs value Mount a tmpfs directory (default []) @@ -620,6 +621,11 @@ or a signal name in the format SIGNAME, for instance SIGKILL. On Windows, this flag can be used to specify the `credentialspec` option. The `credentialspec` must be in the format `file://spec.txt` or `registry://keyname`. +### Stop container with timeout (--stop-timeout) + +The `--stop-timeout` flag sets the the timeout (in seconds) that a pre-defined (see `--stop-signal`) system call +signal that will be sent to the container to exit. After timeout elapses the container will be killed with SIGKILL. + ### Specify isolation technology for container (--isolation) This option is useful in situations where you are running Docker containers on diff --git a/integration-cli/docker_cli_create_test.go b/integration-cli/docker_cli_create_test.go index 21d3fdf831..515a340976 100644 --- a/integration-cli/docker_cli_create_test.go +++ b/integration-cli/docker_cli_create_test.go @@ -496,3 +496,18 @@ exec "$@"`, out, _ = dockerCmd(c, "start", "-a", id) c.Assert(strings.TrimSpace(out), check.Equals, "foo") } + +// #22471 +func (s *DockerSuite) TestCreateStopTimeout(c *check.C) { + name1 := "test_create_stop_timeout_1" + dockerCmd(c, "create", "--name", name1, "--stop-timeout", "15", "busybox") + + res := inspectFieldJSON(c, name1, "Config.StopTimeout") + c.Assert(res, checker.Contains, "15") + + name2 := "test_create_stop_timeout_2" + dockerCmd(c, "create", "--name", name2, "busybox") + + res = inspectFieldJSON(c, name2, "Config.StopTimeout") + c.Assert(res, checker.Contains, "null") +} diff --git a/man/docker-create.1.md b/man/docker-create.1.md index 64365fac74..ece8a91f5f 100644 --- a/man/docker-create.1.md +++ b/man/docker-create.1.md @@ -68,6 +68,7 @@ docker-create - Create a new container [**--security-opt**[=*[]*]] [**--storage-opt**[=*[]*]] [**--stop-signal**[=*SIGNAL*]] +[**--stop-timeout**[=*TIMEOUT*]] [**--shm-size**[=*[]*]] [**--sysctl**[=*[]*]] [**-t**|**--tty**] @@ -352,6 +353,9 @@ unit, `b` is used. Set LIMIT to `-1` to enable unlimited swap. **--stop-signal**=*SIGTERM* Signal to stop a container. Default is SIGTERM. +**--stop-timeout**=*10* + Timeout (in seconds) to stop a container. Default is 10. + **--sysctl**=SYSCTL Configure namespaced kernel parameters at runtime diff --git a/man/docker-run.1.md b/man/docker-run.1.md index 89166710c4..51df3df153 100644 --- a/man/docker-run.1.md +++ b/man/docker-run.1.md @@ -70,6 +70,7 @@ docker-run - Run a command in a new container [**--security-opt**[=*[]*]] [**--storage-opt**[=*[]*]] [**--stop-signal**[=*SIGNAL*]] +[**--stop-timeout**[=*TIMEOUT*]] [**--shm-size**[=*[]*]] [**--sig-proxy**[=*true*]] [**--sysctl**[=*[]*]] @@ -502,6 +503,9 @@ incompatible with any restart policy other than `none`. **--stop-signal**=*SIGTERM* Signal to stop a container. Default is SIGTERM. +**--stop-timeout**=*10* + Timeout (in seconds) to stop a container. Default is 10. + **--shm-size**="" Size of `/dev/shm`. The format is ``. `number` must be greater than `0`. Unit is optional and can be `b` (bytes), `k` (kilobytes), `m`(megabytes), or `g` (gigabytes). diff --git a/runconfig/opts/parse.go b/runconfig/opts/parse.go index 5b99ce6736..81240b8290 100644 --- a/runconfig/opts/parse.go +++ b/runconfig/opts/parse.go @@ -106,6 +106,7 @@ type ContainerOptions struct { init bool initPath string credentialSpec string + stopTimeout int Image string Args []string @@ -145,6 +146,7 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions { ulimits: NewUlimitOpt(nil), volumes: opts.NewListOpts(nil), volumesFrom: opts.NewListOpts(nil), + stopSignal: flags.String("stop-signal", signal.DefaultStopSignal, fmt.Sprintf("Signal to stop a container, %v by default", signal.DefaultStopSignal)), } // General purpose flags @@ -558,6 +560,9 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c if flags.Changed("stop-signal") { config.StopSignal = copts.stopSignal } + if flags.Changed("stop-timeout") { + config.StopTimeout = copts.flStopTimeout + } hostConfig := &container.HostConfig{ Binds: binds, From cc703784f39fb3f585ec685cf7aeda6410648446 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 6 Jun 2016 20:29:05 -0700 Subject: [PATCH 2/2] Update `docker stop` and `docker restart` to allow not specifying timeout and use the one specified at container creation time. Signed-off-by: Yong Tang --- api/server/router/container/backend.go | 4 +-- .../router/container/container_routes.go | 20 +++++++++-- cli/command/container/restart.go | 13 ++++++-- cli/command/container/stop.go | 13 ++++++-- cmd/dockerd/daemon.go | 12 +++++-- container/container.go | 6 ++-- container/container_unit_test.go | 4 +-- daemon/cluster/executor/backend.go | 2 +- daemon/cluster/executor/container/adapter.go | 7 ++-- daemon/daemon.go | 33 ++++++++++++++++--- daemon/restart.go | 9 +++-- daemon/stop.go | 8 +++-- runconfig/opts/parse.go | 6 ++-- 13 files changed, 103 insertions(+), 34 deletions(-) diff --git a/api/server/router/container/backend.go b/api/server/router/container/backend.go index 55f2c9aa23..2e8968d2e1 100644 --- a/api/server/router/container/backend.go +++ b/api/server/router/container/backend.go @@ -37,10 +37,10 @@ type stateBackend interface { ContainerPause(name string) error ContainerRename(oldName, newName string) error ContainerResize(name string, height, width int) error - ContainerRestart(name string, seconds int) error + ContainerRestart(name string, seconds *int) error ContainerRm(name string, config *types.ContainerRmConfig) error ContainerStart(name string, hostConfig *container.HostConfig, validateHostname bool, checkpoint string) error - ContainerStop(name string, seconds int) error + ContainerStop(name string, seconds *int) error ContainerUnpause(name string) error ContainerUpdate(name string, hostConfig *container.HostConfig, validateHostname bool) (types.ContainerUpdateResponse, error) ContainerWait(name string, timeout time.Duration) (int, error) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 481745082d..001ad3a3ee 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -169,7 +169,14 @@ func (s *containerRouter) postContainersStop(ctx context.Context, w http.Respons return err } - seconds, _ := strconv.Atoi(r.Form.Get("t")) + var seconds *int + if tmpSeconds := r.Form.Get("t"); tmpSeconds != "" { + valSeconds, err := strconv.Atoi(tmpSeconds) + if err != nil { + return err + } + seconds = &valSeconds + } if err := s.backend.ContainerStop(vars["name"], seconds); err != nil { return err @@ -223,9 +230,16 @@ func (s *containerRouter) postContainersRestart(ctx context.Context, w http.Resp return err } - timeout, _ := strconv.Atoi(r.Form.Get("t")) + var seconds *int + if tmpSeconds := r.Form.Get("t"); tmpSeconds != "" { + valSeconds, err := strconv.Atoi(tmpSeconds) + if err != nil { + return err + } + seconds = &valSeconds + } - if err := s.backend.ContainerRestart(vars["name"], timeout); err != nil { + if err := s.backend.ContainerRestart(vars["name"], seconds); err != nil { return err } diff --git a/cli/command/container/restart.go b/cli/command/container/restart.go index e370ef4010..fc3ba93c84 100644 --- a/cli/command/container/restart.go +++ b/cli/command/container/restart.go @@ -13,7 +13,8 @@ import ( ) type restartOptions struct { - nSeconds int + nSeconds int + nSecondsChanged bool containers []string } @@ -28,6 +29,7 @@ func NewRestartCommand(dockerCli *command.DockerCli) *cobra.Command { Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.containers = args + opts.nSecondsChanged = cmd.Flags().Changed("time") return runRestart(dockerCli, &opts) }, } @@ -40,9 +42,14 @@ func NewRestartCommand(dockerCli *command.DockerCli) *cobra.Command { func runRestart(dockerCli *command.DockerCli, opts *restartOptions) error { ctx := context.Background() var errs []string + var timeout *time.Duration + if opts.nSecondsChanged { + timeoutValue := time.Duration(opts.nSeconds) * time.Second + timeout = &timeoutValue + } + for _, name := range opts.containers { - timeout := time.Duration(opts.nSeconds) * time.Second - if err := dockerCli.Client().ContainerRestart(ctx, name, &timeout); err != nil { + if err := dockerCli.Client().ContainerRestart(ctx, name, timeout); err != nil { errs = append(errs, err.Error()) } else { fmt.Fprintf(dockerCli.Out(), "%s\n", name) diff --git a/cli/command/container/stop.go b/cli/command/container/stop.go index 2f22fd09a4..c68ede5368 100644 --- a/cli/command/container/stop.go +++ b/cli/command/container/stop.go @@ -13,7 +13,8 @@ import ( ) type stopOptions struct { - time int + time int + timeChanged bool containers []string } @@ -28,6 +29,7 @@ func NewStopCommand(dockerCli *command.DockerCli) *cobra.Command { Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.containers = args + opts.timeChanged = cmd.Flags().Changed("time") return runStop(dockerCli, &opts) }, } @@ -39,12 +41,17 @@ func NewStopCommand(dockerCli *command.DockerCli) *cobra.Command { func runStop(dockerCli *command.DockerCli, opts *stopOptions) error { ctx := context.Background() - timeout := time.Duration(opts.time) * time.Second + + var timeout *time.Duration + if opts.timeChanged { + timeoutValue := time.Duration(opts.time) * time.Second + timeout = &timeoutValue + } var errs []string errChan := parallelOperation(ctx, opts.containers, func(ctx context.Context, id string) error { - return dockerCli.Client().ContainerStop(ctx, id, &timeout) + return dockerCli.Client().ContainerStop(ctx, id, timeout) }) for _, container := range opts.containers { if err := <-errChan; err != nil { diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index f90f93835c..d85cde45c9 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -296,7 +296,7 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) { // Wait for serve API to complete errAPI := <-serveAPIWait c.Cleanup() - shutdownDaemon(d, 15) + shutdownDaemon(d) containerdRemote.Cleanup() if errAPI != nil { return fmt.Errorf("Shutting down due to ServeAPI error: %v", errAPI) @@ -342,16 +342,22 @@ func (cli *DaemonCli) stop() { // shutdownDaemon just wraps daemon.Shutdown() to handle a timeout in case // d.Shutdown() is waiting too long to kill container or worst it's // blocked there -func shutdownDaemon(d *daemon.Daemon, timeout time.Duration) { +func shutdownDaemon(d *daemon.Daemon) { + shutdownTimeout := d.ShutdownTimeout() ch := make(chan struct{}) go func() { d.Shutdown() close(ch) }() + if shutdownTimeout < 0 { + <-ch + logrus.Debug("Clean shutdown succeeded") + return + } select { case <-ch: logrus.Debug("Clean shutdown succeeded") - case <-time.After(timeout * time.Second): + case <-time.After(time.Duration(shutdownTimeout) * time.Second): logrus.Error("Force shutdown daemon") } } diff --git a/container/container.go b/container/container.go index 59bcaa8962..0043c7091c 100644 --- a/container/container.go +++ b/container/container.go @@ -45,8 +45,8 @@ import ( const configFileName = "config.v2.json" const ( - // defaultStopTimeout is the timeout (in seconds) for the syscall signal used to stop a container. - defaultStopTimeout = 10 + // DefaultStopTimeout is the timeout (in seconds) for the syscall signal used to stop a container. + DefaultStopTimeout = 10 ) var ( @@ -588,7 +588,7 @@ func (container *Container) StopTimeout() int { if container.Config.StopTimeout != nil { return *container.Config.StopTimeout } - return defaultStopTimeout + return DefaultStopTimeout } // InitDNSHostConfig ensures that the dns fields are never nil. diff --git a/container/container_unit_test.go b/container/container_unit_test.go index 73bdf0fe13..f301f25bbe 100644 --- a/container/container_unit_test.go +++ b/container/container_unit_test.go @@ -43,8 +43,8 @@ func TestContainerStopTimeout(t *testing.T) { } s := c.StopTimeout() - if s != defaultStopTimeout { - t.Fatalf("Expected %v, got %v", defaultStopTimeout, s) + if s != DefaultStopTimeout { + t.Fatalf("Expected %v, got %v", DefaultStopTimeout, s) } stopTimeout := 15 diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index d8a7646e4d..fb7c822225 100644 --- a/daemon/cluster/executor/backend.go +++ b/daemon/cluster/executor/backend.go @@ -25,7 +25,7 @@ type Backend interface { PullImage(ctx context.Context, image, tag string, metaHeaders map[string][]string, authConfig *types.AuthConfig, outStream io.Writer) error CreateManagedContainer(config types.ContainerCreateConfig, validateHostname bool) (types.ContainerCreateResponse, error) ContainerStart(name string, hostConfig *container.HostConfig, validateHostname bool, checkpoint string) error - ContainerStop(name string, seconds int) error + ContainerStop(name string, seconds *int) error ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error UpdateContainerServiceConfig(containerName string, serviceConfig *clustertypes.ServiceConfig) error ContainerInspectCurrent(name string, size bool) (*types.ContainerJSON, error) diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 774333e8a3..56f8c4067e 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -279,11 +279,12 @@ func (c *containerAdapter) wait(ctx context.Context) error { } func (c *containerAdapter) shutdown(ctx context.Context) error { - // Default stop grace period to 10s. - stopgrace := 10 + // Default stop grace period to nil (daemon will use the stopTimeout of the container) + var stopgrace *int spec := c.container.spec() if spec.StopGracePeriod != nil { - stopgrace = int(spec.StopGracePeriod.Seconds) + stopgraceValue := int(spec.StopGracePeriod.Seconds) + stopgrace = &stopgraceValue } return c.backend.ContainerStop(c.container.name(), stopgrace) } diff --git a/daemon/daemon.go b/daemon/daemon.go index 218c9031cc..adf5e443ae 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -692,6 +692,8 @@ func NewDaemon(config *Config, registryService registry.Service, containerdRemot } func (daemon *Daemon) shutdownContainer(c *container.Container) error { + stopTimeout := c.StopTimeout() + // TODO(windows): Handle docker restart with paused containers if c.IsPaused() { // To terminate a process in freezer cgroup, we should send @@ -708,8 +710,8 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { if err := daemon.containerUnpause(c); err != nil { return fmt.Errorf("Failed to unpause container %s with error: %v", c.ID, err) } - if _, err := c.WaitStop(time.Duration(c.StopTimeout()) * time.Second); err != nil { - logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, c.StopTimeout()) + if _, err := c.WaitStop(time.Duration(stopTimeout) * time.Second); err != nil { + logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, stopTimeout) sig, ok := signal.SignalMap["KILL"] if !ok { return fmt.Errorf("System does not support SIGKILL") @@ -721,8 +723,8 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { return err } } - // If container failed to exit in c.StopTimeout() seconds of SIGTERM, then using the force - if err := daemon.containerStop(c, c.StopTimeout()); err != nil { + // If container failed to exit in stopTimeout seconds of SIGTERM, then using the force + if err := daemon.containerStop(c, stopTimeout); err != nil { return fmt.Errorf("Failed to stop container %s with error: %v", c.ID, err) } @@ -730,6 +732,29 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { return nil } +// ShutdownTimeout returns the shutdown timeout based on the max stopTimeout of the containers +func (daemon *Daemon) ShutdownTimeout() int { + // By default we use container.DefaultStopTimeout + 5s, which is 15s. + // TODO (yongtang): Will need to allow shutdown-timeout once #23036 is in place. + graceTimeout := 5 + shutdownTimeout := container.DefaultStopTimeout + graceTimeout + if daemon.containers != nil { + for _, c := range daemon.containers.List() { + if shutdownTimeout >= 0 { + stopTimeout := c.StopTimeout() + if stopTimeout < 0 { + shutdownTimeout = -1 + } else { + if stopTimeout+graceTimeout > shutdownTimeout { + shutdownTimeout = stopTimeout + graceTimeout + } + } + } + } + } + return shutdownTimeout +} + // Shutdown stops the daemon. func (daemon *Daemon) Shutdown() error { daemon.shutdown = true diff --git a/daemon/restart.go b/daemon/restart.go index 1172f6c82e..cdf21af293 100644 --- a/daemon/restart.go +++ b/daemon/restart.go @@ -13,15 +13,20 @@ import ( // timeout, ContainerRestart will wait forever until a graceful // stop. Returns an error if the container cannot be found, or if // there is an underlying error at any stage of the restart. -func (daemon *Daemon) ContainerRestart(name string, seconds int) error { +func (daemon *Daemon) ContainerRestart(name string, seconds *int) error { container, err := daemon.GetContainer(name) if err != nil { return err } - if err := daemon.containerRestart(container, seconds); err != nil { + if seconds == nil { + stopTimeout := container.StopTimeout() + seconds = &stopTimeout + } + if err := daemon.containerRestart(container, *seconds); err != nil { return fmt.Errorf("Cannot restart container %s: %v", name, err) } return nil + } // containerRestart attempts to gracefully stop and then start the diff --git a/daemon/stop.go b/daemon/stop.go index 90b8898b24..aa7b3820c8 100644 --- a/daemon/stop.go +++ b/daemon/stop.go @@ -16,7 +16,7 @@ import ( // will wait for a graceful termination. An error is returned if the // container is not found, is already stopped, or if there is a // problem stopping the container. -func (daemon *Daemon) ContainerStop(name string, seconds int) error { +func (daemon *Daemon) ContainerStop(name string, seconds *int) error { container, err := daemon.GetContainer(name) if err != nil { return err @@ -25,7 +25,11 @@ func (daemon *Daemon) ContainerStop(name string, seconds int) error { err := fmt.Errorf("Container %s is already stopped", name) return errors.NewErrorWithStatusCode(err, http.StatusNotModified) } - if err := daemon.containerStop(container, seconds); err != nil { + if seconds == nil { + stopTimeout := container.StopTimeout() + seconds = &stopTimeout + } + if err := daemon.containerStop(container, *seconds); err != nil { return fmt.Errorf("Cannot stop container %s: %v", name, err) } return nil diff --git a/runconfig/opts/parse.go b/runconfig/opts/parse.go index 81240b8290..e4221dee2d 100644 --- a/runconfig/opts/parse.go +++ b/runconfig/opts/parse.go @@ -94,6 +94,7 @@ type ContainerOptions struct { cgroupParent string volumeDriver string stopSignal string + stopTimeout int isolation string shmSize string noHealthcheck bool @@ -106,7 +107,6 @@ type ContainerOptions struct { init bool initPath string credentialSpec string - stopTimeout int Image string Args []string @@ -146,7 +146,6 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions { ulimits: NewUlimitOpt(nil), volumes: opts.NewListOpts(nil), volumesFrom: opts.NewListOpts(nil), - stopSignal: flags.String("stop-signal", signal.DefaultStopSignal, fmt.Sprintf("Signal to stop a container, %v by default", signal.DefaultStopSignal)), } // General purpose flags @@ -163,6 +162,7 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions { flags.BoolVar(&copts.readonlyRootfs, "read-only", false, "Mount the container's root filesystem as read only") flags.StringVar(&copts.restartPolicy, "restart", "no", "Restart policy to apply when a container exits") flags.StringVar(&copts.stopSignal, "stop-signal", signal.DefaultStopSignal, fmt.Sprintf("Signal to stop a container, %v by default", signal.DefaultStopSignal)) + flags.IntVar(&copts.stopTimeout, "stop-timeout", 0, "Timeout (in seconds) to stop a container") flags.Var(copts.sysctls, "sysctl", "Sysctl options") flags.BoolVarP(&copts.tty, "tty", "t", false, "Allocate a pseudo-TTY") flags.Var(copts.ulimits, "ulimit", "Ulimit options") @@ -561,7 +561,7 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c config.StopSignal = copts.stopSignal } if flags.Changed("stop-timeout") { - config.StopTimeout = copts.flStopTimeout + config.StopTimeout = &copts.stopTimeout } hostConfig := &container.HostConfig{