Skip to content

Commit

Permalink
Merge pull request #5485 from thaJeztah/time_or_timeout
Browse files Browse the repository at this point in the history
cli/command/container: stop, restart: rename "--time" to "--timeout"
  • Loading branch information
thaJeztah authored Sep 30, 2024
2 parents b1ae218 + df8b345 commit 3907414
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 32 deletions.
11 changes: 9 additions & 2 deletions cli/command/container/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)

Expand Down
11 changes: 11 additions & 0 deletions cli/command/container/restart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 9 additions & 2 deletions cli/command/container/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)

Expand Down
11 changes: 11 additions & 0 deletions cli/command/container/stop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions contrib/completion/bash/docker
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions contrib/completion/zsh/_docker
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions docs/deprecated.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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**
Expand Down
14 changes: 7 additions & 7 deletions docs/reference/commandline/container_restart.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |


<!---MARKER_GEN_END-->
Expand All @@ -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.

### <a name="time"></a> Stop container with timeout (-t, --timeout)
### <a name="timeout"></a> 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)
Expand Down
14 changes: 7 additions & 7 deletions docs/reference/commandline/container_stop.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |


<!---MARKER_GEN_END-->
Expand Down Expand Up @@ -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.

### <a name="time"></a> Stop container with timeout (-t, --timeout)
### <a name="timeout"></a> 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)
Expand Down
8 changes: 4 additions & 4 deletions docs/reference/commandline/restart.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |


<!---MARKER_GEN_END-->
Expand Down
8 changes: 4 additions & 4 deletions docs/reference/commandline/stop.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |


<!---MARKER_GEN_END-->
Expand Down

0 comments on commit 3907414

Please sign in to comment.