From e66d2108911a2fad016205bdd6bf181f7e822c1c Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Thu, 26 May 2016 13:34:48 -0700 Subject: [PATCH] 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,