From df8b34595bd43aecbdc7bcabacb58f737f6d4946 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 27 Sep 2024 16:26:33 +0200 Subject: [PATCH] cli/command/container: stop, restart: rename "--time" to "--timeout" This renames the `--time` flag as used on `docker stop` and `docker restart` to `--timeout`, bringing it in line with other uses for this property, such as `--stop-timeout` on `docker run`. The `--time` option is deprecated and hidden, but will be kept for backward compatibility, as these options existed for a long time. Signed-off-by: Sebastiaan van Stijn --- cli/command/container/restart.go | 11 +++++++++-- cli/command/container/restart_test.go | 11 +++++++++++ cli/command/container/stop.go | 11 +++++++++-- cli/command/container/stop_test.go | 11 +++++++++++ contrib/completion/bash/docker | 8 ++++---- contrib/completion/zsh/_docker | 4 ++-- docs/deprecated.md | 11 +++++++++++ docs/reference/commandline/container_restart.md | 14 +++++++------- docs/reference/commandline/container_stop.md | 14 +++++++------- docs/reference/commandline/restart.md | 8 ++++---- docs/reference/commandline/stop.md | 8 ++++---- 11 files changed, 79 insertions(+), 32 deletions(-) diff --git a/cli/command/container/restart.go b/cli/command/container/restart.go index 950bb81e04..5460ff5228 100644 --- a/cli/command/container/restart.go +++ b/cli/command/container/restart.go @@ -30,8 +30,11 @@ func NewRestartCommand(dockerCli command.Cli) *cobra.Command { Short: "Restart one or more containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("time") && cmd.Flags().Changed("timeout") { + return errors.New("conflicting options: cannot specify both --timeout and --time") + } opts.containers = args - opts.timeoutChanged = cmd.Flags().Changed("time") + opts.timeoutChanged = cmd.Flags().Changed("timeout") || cmd.Flags().Changed("time") return runRestart(cmd.Context(), dockerCli, &opts) }, Annotations: map[string]string{ @@ -42,7 +45,11 @@ func NewRestartCommand(dockerCli command.Cli) *cobra.Command { flags := cmd.Flags() flags.StringVarP(&opts.signal, "signal", "s", "", "Signal to send to the container") - flags.IntVarP(&opts.timeout, "time", "t", 0, "Seconds to wait before killing the container") + flags.IntVarP(&opts.timeout, "timeout", "t", 0, "Seconds to wait before killing the container") + + // The --time option is deprecated, but kept for backward compatibility. + flags.IntVar(&opts.timeout, "time", 0, "Seconds to wait before killing the container (deprecated: use --timeout)") + _ = flags.MarkDeprecated("time", "use --timeout instead") _ = cmd.RegisterFlagCompletionFunc("signal", completeSignals) diff --git a/cli/command/container/restart_test.go b/cli/command/container/restart_test.go index 8a5a69c1bc..d1b6ed43ea 100644 --- a/cli/command/container/restart_test.go +++ b/cli/command/container/restart_test.go @@ -40,12 +40,23 @@ func TestRestart(t *testing.T) { expectedOpts: container.StopOptions{Timeout: func(to int) *int { return &to }(2)}, restarted: []string{"container-1"}, }, + { + name: "with --timeout", + args: []string{"--timeout", "2", "container-1"}, + expectedOpts: container.StopOptions{Timeout: func(to int) *int { return &to }(2)}, + restarted: []string{"container-1"}, + }, { name: "with --time", args: []string{"--time", "2", "container-1"}, expectedOpts: container.StopOptions{Timeout: func(to int) *int { return &to }(2)}, restarted: []string{"container-1"}, }, + { + name: "conflicting options", + args: []string{"--timeout", "2", "--time", "2", "container-1"}, + expectedErr: "conflicting options: cannot specify both --timeout and --time", + }, } { tc := tc t.Run(tc.name, func(t *testing.T) { diff --git a/cli/command/container/stop.go b/cli/command/container/stop.go index c99a7c456d..af24f43caf 100644 --- a/cli/command/container/stop.go +++ b/cli/command/container/stop.go @@ -30,8 +30,11 @@ func NewStopCommand(dockerCli command.Cli) *cobra.Command { Short: "Stop one or more running containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("time") && cmd.Flags().Changed("timeout") { + return errors.New("conflicting options: cannot specify both --timeout and --time") + } opts.containers = args - opts.timeoutChanged = cmd.Flags().Changed("time") + opts.timeoutChanged = cmd.Flags().Changed("timeout") || cmd.Flags().Changed("time") return runStop(cmd.Context(), dockerCli, &opts) }, Annotations: map[string]string{ @@ -42,7 +45,11 @@ func NewStopCommand(dockerCli command.Cli) *cobra.Command { flags := cmd.Flags() flags.StringVarP(&opts.signal, "signal", "s", "", "Signal to send to the container") - flags.IntVarP(&opts.timeout, "time", "t", 0, "Seconds to wait before killing the container") + flags.IntVarP(&opts.timeout, "timeout", "t", 0, "Seconds to wait before killing the container") + + // The --time option is deprecated, but kept for backward compatibility. + flags.IntVar(&opts.timeout, "time", 0, "Seconds to wait before killing the container (deprecated: use --timeout)") + _ = flags.MarkDeprecated("time", "use --timeout instead") _ = cmd.RegisterFlagCompletionFunc("signal", completeSignals) diff --git a/cli/command/container/stop_test.go b/cli/command/container/stop_test.go index 44893d68ab..1fad3cd05e 100644 --- a/cli/command/container/stop_test.go +++ b/cli/command/container/stop_test.go @@ -40,12 +40,23 @@ func TestStop(t *testing.T) { expectedOpts: container.StopOptions{Timeout: func(to int) *int { return &to }(2)}, stopped: []string{"container-1"}, }, + { + name: "with --timeout", + args: []string{"--timeout", "2", "container-1"}, + expectedOpts: container.StopOptions{Timeout: func(to int) *int { return &to }(2)}, + stopped: []string{"container-1"}, + }, { name: "with --time", args: []string{"--time", "2", "container-1"}, expectedOpts: container.StopOptions{Timeout: func(to int) *int { return &to }(2)}, stopped: []string{"container-1"}, }, + { + name: "conflicting options", + args: []string{"--timeout", "2", "--time", "2", "container-1"}, + expectedErr: "conflicting options: cannot specify both --timeout and --time", + }, } { tc := tc t.Run(tc.name, func(t *testing.T) { diff --git a/contrib/completion/bash/docker b/contrib/completion/bash/docker index a934ecbbb6..68b7956054 100644 --- a/contrib/completion/bash/docker +++ b/contrib/completion/bash/docker @@ -1844,14 +1844,14 @@ _docker_container_rename() { _docker_container_restart() { case "$prev" in - --time|-t) + --timeout|--time|-t) return ;; esac case "$cur" in -*) - COMPREPLY=( $( compgen -W "--help --time -t" -- "$cur" ) ) + COMPREPLY=( $( compgen -W "--help --timeout -t" -- "$cur" ) ) ;; *) __docker_complete_containers_all @@ -2256,14 +2256,14 @@ _docker_container_stats() { _docker_container_stop() { case "$prev" in - --time|-t) + --timeout|--time|-t) return ;; esac case "$cur" in -*) - COMPREPLY=( $( compgen -W "--help --time -t" -- "$cur" ) ) + COMPREPLY=( $( compgen -W "--help --timeout -t" -- "$cur" ) ) ;; *) __docker_complete_containers_stoppable diff --git a/contrib/completion/zsh/_docker b/contrib/completion/zsh/_docker index 4236c225e3..f4d538a3df 100644 --- a/contrib/completion/zsh/_docker +++ b/contrib/completion/zsh/_docker @@ -836,7 +836,7 @@ __docker_container_subcommand() { (restart) _arguments $(__docker_arguments) \ $opts_help \ - "($help -t --time)"{-t=,--time=}"[Number of seconds to try to stop for before killing the container]:seconds to before killing:(1 5 10 30 60)" \ + "($help -t --timeout)"{-t=,--timeout=}"[Number of seconds to try to stop for before killing the container]:seconds to before killing:(1 5 10 30 60)" \ "($help -)*:containers:__docker_complete_containers" && ret=0 ;; (rm) @@ -915,7 +915,7 @@ __docker_container_subcommand() { (stop) _arguments $(__docker_arguments) \ $opts_help \ - "($help -t --time)"{-t=,--time=}"[Number of seconds to try to stop for before killing the container]:seconds to before killing:(1 5 10 30 60)" \ + "($help -t --timeout)"{-t=,--timeout=}"[Number of seconds to try to stop for before killing the container]:seconds to before killing:(1 5 10 30 60)" \ "($help -)*:containers:__docker_complete_running_containers" && ret=0 ;; (top) diff --git a/docs/deprecated.md b/docs/deprecated.md index 735ff4f862..9c99fad610 100644 --- a/docs/deprecated.md +++ b/docs/deprecated.md @@ -53,6 +53,7 @@ The following table provides an overview of the current status of deprecated fea | Status | Feature | Deprecated | Remove | |------------|------------------------------------------------------------------------------------------------------------------------------------|------------|--------| +| Deprecated | [`--time` option on `docker stop` and `docker restart`](#--time-option-on-docker-stop-and-docker-restart) | v28.0 | - | | Deprecated | [Non-standard fields in image inspect](#non-standard-fields-in-image-inspect) | v27.0 | v28.0 | | Removed | [API CORS headers](#api-cors-headers) | v27.0 | v28.0 | | Deprecated | [Graphdriver plugins (experimental)](#graphdriver-plugins-experimental) | v27.0 | v28.0 | @@ -118,6 +119,16 @@ The following table provides an overview of the current status of deprecated fea | Removed | [`--run` flag on `docker commit`](#--run-flag-on-docker-commit) | v0.10 | v1.13 | | Removed | [Three arguments form in `docker import`](#three-arguments-form-in-docker-import) | v0.6.7 | v1.12 | +### `--time` option on `docker stop` and `docker restart` + +**Deprecated in Release: v28.0** + +The `--time` option for the `docker stop`, `docker container stop`, `docker restart`, +and `docker container restart` commands has been renamed to `--timeout` for +consistency with other uses of timeout options. The `--time` option is now +deprecated and hidden, but remains functional for backward compatibility. +Users are encouraged to migrate to using the `--timeout` option instead. + ### Non-standard fields in image inspect **Deprecated in Release: v27.0** diff --git a/docs/reference/commandline/container_restart.md b/docs/reference/commandline/container_restart.md index 804fa072bf..9a988599b7 100644 --- a/docs/reference/commandline/container_restart.md +++ b/docs/reference/commandline/container_restart.md @@ -9,10 +9,10 @@ Restart one or more containers ### Options -| Name | Type | Default | Description | -|:---------------------------------------|:---------|:--------|:---------------------------------------------| -| [`-s`](#signal), [`--signal`](#signal) | `string` | | Signal to send to the container | -| [`-t`](#time), [`--time`](#time) | `int` | `0` | Seconds to wait before killing the container | +| Name | Type | Default | Description | +|:------------------------------------------|:---------|:--------|:---------------------------------------------| +| [`-s`](#signal), [`--signal`](#signal) | `string` | | Signal to send to the container | +| [`-t`](#timeout), [`--timeout`](#timeout) | `int` | `0` | Seconds to wait before killing the container | @@ -39,14 +39,14 @@ Dockerfile instruction when building the image, or configured using the option when creating the container. If no signal is configured for the container, `SIGTERM` is used as default. -### Stop container with timeout (-t, --timeout) +### Stop container with timeout (-t, --timeout) -The `--time` flag sets the number of seconds to wait for the container +The `--timeout` flag sets the number of seconds to wait for the container to stop after sending the pre-defined (see [`--signal`]{#signal)) system call signal. If the container does not exit after the timeout elapses, it's forcibly killed with a `SIGKILL` signal. -If you set `--time` to `-1`, no timeout is applied, and the daemon +If you set `--timeout` to `-1`, no timeout is applied, and the daemon waits indefinitely for the container to exit. The default timeout can be specified using the [`--stop-timeout`](https://docs.docker.com/reference/cli/docker/container/run/#stop-timeout) diff --git a/docs/reference/commandline/container_stop.md b/docs/reference/commandline/container_stop.md index 78f0325381..61fc1cabad 100644 --- a/docs/reference/commandline/container_stop.md +++ b/docs/reference/commandline/container_stop.md @@ -9,10 +9,10 @@ Stop one or more running containers ### Options -| Name | Type | Default | Description | -|:---------------------------------------|:---------|:--------|:---------------------------------------------| -| [`-s`](#signal), [`--signal`](#signal) | `string` | | Signal to send to the container | -| [`-t`](#time), [`--time`](#time) | `int` | `0` | Seconds to wait before killing the container | +| Name | Type | Default | Description | +|:------------------------------------------|:---------|:--------|:---------------------------------------------| +| [`-s`](#signal), [`--signal`](#signal) | `string` | | Signal to send to the container | +| [`-t`](#timeout), [`--timeout`](#timeout) | `int` | `0` | Seconds to wait before killing the container | @@ -45,14 +45,14 @@ Dockerfile instruction when building the image, or configured using the option when creating the container. If no signal is configured for the container, `SIGTERM` is used as default. -### Stop container with timeout (-t, --timeout) +### Stop container with timeout (-t, --timeout) -The `--time` flag sets the number of seconds to wait for the container +The `--timeout` flag sets the number of seconds to wait for the container to stop after sending the pre-defined (see [`--signal`]{#signal)) system call signal. If the container does not exit after the timeout elapses, it's forcibly killed with a `SIGKILL` signal. -If you set `--time` to `-1`, no timeout is applied, and the daemon +If you set `--timeout` to `-1`, no timeout is applied, and the daemon waits indefinitely for the container to exit. The default timeout can be specified using the [`--stop-timeout`](https://docs.docker.com/reference/cli/docker/container/run/#stop-timeout) diff --git a/docs/reference/commandline/restart.md b/docs/reference/commandline/restart.md index 2c6681cd2a..7392e25b24 100644 --- a/docs/reference/commandline/restart.md +++ b/docs/reference/commandline/restart.md @@ -9,10 +9,10 @@ Restart one or more containers ### Options -| Name | Type | Default | Description | -|:-----------------|:---------|:--------|:---------------------------------------------| -| `-s`, `--signal` | `string` | | Signal to send to the container | -| `-t`, `--time` | `int` | `0` | Seconds to wait before killing the container | +| Name | Type | Default | Description | +|:------------------|:---------|:--------|:---------------------------------------------| +| `-s`, `--signal` | `string` | | Signal to send to the container | +| `-t`, `--timeout` | `int` | `0` | Seconds to wait before killing the container | diff --git a/docs/reference/commandline/stop.md b/docs/reference/commandline/stop.md index 16934813c5..166e962c85 100644 --- a/docs/reference/commandline/stop.md +++ b/docs/reference/commandline/stop.md @@ -9,10 +9,10 @@ Stop one or more running containers ### Options -| Name | Type | Default | Description | -|:-----------------|:---------|:--------|:---------------------------------------------| -| `-s`, `--signal` | `string` | | Signal to send to the container | -| `-t`, `--time` | `int` | `0` | Seconds to wait before killing the container | +| Name | Type | Default | Description | +|:------------------|:---------|:--------|:---------------------------------------------| +| `-s`, `--signal` | `string` | | Signal to send to the container | +| `-t`, `--timeout` | `int` | `0` | Seconds to wait before killing the container |