From 8ed5b685f2f9dab63d50672eb20d66930c0dc096 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert-Andr=C3=A9=20Mauchin?= <zebob.m@gmail.com>
Date: Sun, 9 Oct 2022 15:09:05 +0200
Subject: [PATCH 2/3] Revert "client: ContainerStop(), ContainerRestart():
support stop-signal"
This reverts commit e8fa708ae5c460ebeec1a88a3ec3615272efaa4f.
---
client/container_restart.go | 14 +++++---------
client/container_restart_test.go | 19 ++++++-------------
client/container_stop.go | 14 +++++---------
client/container_stop_test.go | 19 ++++++-------------
client/interface.go | 5 +++--
integration-cli/docker_api_containers_test.go | 18 +++++++-----------
integration/container/pause_test.go | 3 +--
integration/container/rename_test.go | 2 +-
integration/container/stop_linux_test.go | 4 ++--
integration/container/stop_test.go | 3 +--
integration/container/stop_windows_test.go | 4 ++--
integration/container/wait_test.go | 3 +--
12 files changed, 40 insertions(+), 68 deletions(-)
diff --git a/client/container_restart.go b/client/container_restart.go
index 1e0ad99981..aa0d6485de 100644
--- a/client/container_restart.go
+++ b/client/container_restart.go
@@ -3,22 +3,18 @@ package client // import "github.com/docker/docker/client"
import (
"context"
"net/url"
- "strconv"
+ "time"
- "github.com/docker/docker/api/types/container"
- "github.com/docker/docker/api/types/versions"
+ timetypes "github.com/docker/docker/api/types/time"
)
// ContainerRestart stops and starts a container again.
// It makes the daemon wait for the container to be up again for
// a specific amount of time, given the timeout.
-func (cli *Client) ContainerRestart(ctx context.Context, containerID string, options container.StopOptions) error {
+func (cli *Client) ContainerRestart(ctx context.Context, containerID string, timeout *time.Duration) error {
query := url.Values{}
- if options.Timeout != nil {
- query.Set("t", strconv.Itoa(*options.Timeout))
- }
- if options.Signal != "" && versions.GreaterThanOrEqualTo(cli.version, "1.42") {
- query.Set("signal", options.Signal)
+ if timeout != nil {
+ query.Set("t", timetypes.DurationToSecondsString(*timeout))
}
resp, err := cli.post(ctx, "/containers/"+containerID+"/restart", query, nil, nil)
ensureReaderClosed(resp)
diff --git a/client/container_restart_test.go b/client/container_restart_test.go
index 8c66525edb..085da26b70 100644
--- a/client/container_restart_test.go
+++ b/client/container_restart_test.go
@@ -8,8 +8,8 @@ import (
"net/http"
"strings"
"testing"
+ "time"
- "github.com/docker/docker/api/types/container"
"github.com/docker/docker/errdefs"
)
@@ -17,23 +17,20 @@ func TestContainerRestartError(t *testing.T) {
client := &Client{
client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
}
- err := client.ContainerRestart(context.Background(), "nothing", container.StopOptions{})
+ timeout := 0 * time.Second
+ err := client.ContainerRestart(context.Background(), "nothing", &timeout)
if !errdefs.IsSystem(err) {
t.Fatalf("expected a Server Error, got %[1]T: %[1]v", err)
}
}
func TestContainerRestart(t *testing.T) {
- const expectedURL = "/v1.42/containers/container_id/restart"
+ expectedURL := "/containers/container_id/restart"
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
if !strings.HasPrefix(req.URL.Path, expectedURL) {
return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL)
}
- s := req.URL.Query().Get("signal")
- if s != "SIGKILL" {
- return nil, fmt.Errorf("signal not set in URL query. Expected 'SIGKILL', got '%s'", s)
- }
t := req.URL.Query().Get("t")
if t != "100" {
return nil, fmt.Errorf("t (timeout) not set in URL query properly. Expected '100', got %s", t)
@@ -43,13 +40,9 @@ func TestContainerRestart(t *testing.T) {
Body: io.NopCloser(bytes.NewReader([]byte(""))),
}, nil
}),
- version: "1.42",
}
- timeout := 100
- err := client.ContainerRestart(context.Background(), "container_id", container.StopOptions{
- Signal: "SIGKILL",
- Timeout: &timeout,
- })
+ timeout := 100 * time.Second
+ err := client.ContainerRestart(context.Background(), "container_id", &timeout)
if err != nil {
t.Fatal(err)
}
diff --git a/client/container_stop.go b/client/container_stop.go
index 2a43ce2274..629d7ab64c 100644
--- a/client/container_stop.go
+++ b/client/container_stop.go
@@ -3,10 +3,9 @@ package client // import "github.com/docker/docker/client"
import (
"context"
"net/url"
- "strconv"
+ "time"
- "github.com/docker/docker/api/types/container"
- "github.com/docker/docker/api/types/versions"
+ timetypes "github.com/docker/docker/api/types/time"
)
// ContainerStop stops a container. In case the container fails to stop
@@ -16,13 +15,10 @@ import (
// If the timeout is nil, the container's StopTimeout value is used, if set,
// otherwise the engine default. A negative timeout value can be specified,
// meaning no timeout, i.e. no forceful termination is performed.
-func (cli *Client) ContainerStop(ctx context.Context, containerID string, options container.StopOptions) error {
+func (cli *Client) ContainerStop(ctx context.Context, containerID string, timeout *time.Duration) error {
query := url.Values{}
- if options.Timeout != nil {
- query.Set("t", strconv.Itoa(*options.Timeout))
- }
- if options.Signal != "" && versions.GreaterThanOrEqualTo(cli.version, "1.42") {
- query.Set("signal", options.Signal)
+ if timeout != nil {
+ query.Set("t", timetypes.DurationToSecondsString(*timeout))
}
resp, err := cli.post(ctx, "/containers/"+containerID+"/stop", query, nil, nil)
ensureReaderClosed(resp)
diff --git a/client/container_stop_test.go b/client/container_stop_test.go
index ec4cfc0d3c..f5df1805cb 100644
--- a/client/container_stop_test.go
+++ b/client/container_stop_test.go
@@ -8,8 +8,8 @@ import (
"net/http"
"strings"
"testing"
+ "time"
- "github.com/docker/docker/api/types/container"
"github.com/docker/docker/errdefs"
)
@@ -17,23 +17,20 @@ func TestContainerStopError(t *testing.T) {
client := &Client{
client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
}
- err := client.ContainerStop(context.Background(), "nothing", container.StopOptions{})
+ timeout := 0 * time.Second
+ err := client.ContainerStop(context.Background(), "nothing", &timeout)
if !errdefs.IsSystem(err) {
t.Fatalf("expected a Server Error, got %[1]T: %[1]v", err)
}
}
func TestContainerStop(t *testing.T) {
- const expectedURL = "/v1.42/containers/container_id/stop"
+ expectedURL := "/containers/container_id/stop"
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
if !strings.HasPrefix(req.URL.Path, expectedURL) {
return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL)
}
- s := req.URL.Query().Get("signal")
- if s != "SIGKILL" {
- return nil, fmt.Errorf("signal not set in URL query. Expected 'SIGKILL', got '%s'", s)
- }
t := req.URL.Query().Get("t")
if t != "100" {
return nil, fmt.Errorf("t (timeout) not set in URL query properly. Expected '100', got %s", t)
@@ -43,13 +40,9 @@ func TestContainerStop(t *testing.T) {
Body: io.NopCloser(bytes.NewReader([]byte(""))),
}, nil
}),
- version: "1.42",
}
- timeout := 100
- err := client.ContainerStop(context.Background(), "container_id", container.StopOptions{
- Signal: "SIGKILL",
- Timeout: &timeout,
- })
+ timeout := 100 * time.Second
+ err := client.ContainerStop(context.Background(), "container_id", &timeout)
if err != nil {
t.Fatal(err)
}
diff --git a/client/interface.go b/client/interface.go
index e9c1ed722e..73934b2361 100644
--- a/client/interface.go
+++ b/client/interface.go
@@ -5,6 +5,7 @@ import (
"io"
"net"
"net/http"
+ "time"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
@@ -64,12 +65,12 @@ type ContainerAPIClient interface {
ContainerRemove(ctx context.Context, container string, options types.ContainerRemoveOptions) error
ContainerRename(ctx context.Context, container, newContainerName string) error
ContainerResize(ctx context.Context, container string, options types.ResizeOptions) error
- ContainerRestart(ctx context.Context, container string, options container.StopOptions) error
+ ContainerRestart(ctx context.Context, container string, timeout *time.Duration) error
ContainerStatPath(ctx context.Context, container, path string) (types.ContainerPathStat, error)
ContainerStats(ctx context.Context, container string, stream bool) (types.ContainerStats, error)
ContainerStatsOneShot(ctx context.Context, container string) (types.ContainerStats, error)
ContainerStart(ctx context.Context, container string, options types.ContainerStartOptions) error
- ContainerStop(ctx context.Context, container string, options container.StopOptions) error
+ ContainerStop(ctx context.Context, container string, timeout *time.Duration) error
ContainerTop(ctx context.Context, container string, arguments []string) (container.ContainerTopOKBody, error)
ContainerUnpause(ctx context.Context, container string) error
ContainerUpdate(ctx context.Context, container string, updateConfig container.UpdateConfig) (container.ContainerUpdateOKBody, error)
diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go
index b5e6b18e67..e58814f540 100644
--- a/integration-cli/docker_api_containers_test.go
+++ b/integration-cli/docker_api_containers_test.go
@@ -913,8 +913,8 @@ func (s *DockerSuite) TestContainerAPIRestart(c *testing.T) {
assert.NilError(c, err)
defer cli.Close()
- timeout := 1
- err = cli.ContainerRestart(context.Background(), name, container.StopOptions{Timeout: &timeout})
+ timeout := 1 * time.Second
+ err = cli.ContainerRestart(context.Background(), name, &timeout)
assert.NilError(c, err)
assert.Assert(c, waitInspect(name, "{{ .State.Restarting }} {{ .State.Running }}", "false true", 15*time.Second) == nil)
@@ -930,7 +930,7 @@ func (s *DockerSuite) TestContainerAPIRestartNotimeoutParam(c *testing.T) {
assert.NilError(c, err)
defer cli.Close()
- err = cli.ContainerRestart(context.Background(), name, container.StopOptions{})
+ err = cli.ContainerRestart(context.Background(), name, nil)
assert.NilError(c, err)
assert.Assert(c, waitInspect(name, "{{ .State.Restarting }} {{ .State.Running }}", "false true", 15*time.Second) == nil)
@@ -965,23 +965,19 @@ func (s *DockerSuite) TestContainerAPIStart(c *testing.T) {
func (s *DockerSuite) TestContainerAPIStop(c *testing.T) {
name := "test-api-stop"
runSleepingContainer(c, "-i", "--name", name)
- timeout := 30
+ timeout := 30 * time.Second
cli, err := client.NewClientWithOpts(client.FromEnv)
assert.NilError(c, err)
defer cli.Close()
- err = cli.ContainerStop(context.Background(), name, container.StopOptions{
- Timeout: &timeout,
- })
+ err = cli.ContainerStop(context.Background(), name, &timeout)
assert.NilError(c, err)
assert.Assert(c, waitInspect(name, "{{ .State.Running }}", "false", 60*time.Second) == nil)
// second call to start should give 304
// maybe add ContainerStartWithRaw to test it
- err = cli.ContainerStop(context.Background(), name, container.StopOptions{
- Timeout: &timeout,
- })
+ err = cli.ContainerStop(context.Background(), name, &timeout)
assert.NilError(c, err)
}
@@ -1259,7 +1255,7 @@ func (s *DockerSuite) TestContainerAPIPostContainerStop(c *testing.T) {
assert.NilError(c, err)
defer cli.Close()
- err = cli.ContainerStop(context.Background(), containerID, container.StopOptions{})
+ err = cli.ContainerStop(context.Background(), containerID, nil)
assert.NilError(c, err)
assert.Assert(c, waitInspect(containerID, "{{ .State.Running }}", "false", 60*time.Second) == nil)
}
diff --git a/integration/container/pause_test.go b/integration/container/pause_test.go
index e34eeee337..63e7fe3a6e 100644
--- a/integration/container/pause_test.go
+++ b/integration/container/pause_test.go
@@ -8,7 +8,6 @@ import (
containerderrdefs "github.com/containerd/containerd/errdefs"
"github.com/docker/docker/api/types"
- containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/events"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/versions"
@@ -81,7 +80,7 @@ func TestPauseStopPausedContainer(t *testing.T) {
err := client.ContainerPause(ctx, cID)
assert.NilError(t, err)
- err = client.ContainerStop(ctx, cID, containertypes.StopOptions{})
+ err = client.ContainerStop(ctx, cID, nil)
assert.NilError(t, err)
poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond))
diff --git a/integration/container/rename_test.go b/integration/container/rename_test.go
index 689a37fc32..b2ad4902ec 100644
--- a/integration/container/rename_test.go
+++ b/integration/container/rename_test.go
@@ -143,7 +143,7 @@ func TestRenameAnonymousContainer(t *testing.T) {
assert.NilError(t, err)
// Stop/Start the container to get registered
// FIXME(vdemeester) this is a really weird behavior as it fails otherwise
- err = client.ContainerStop(ctx, container1Name, containertypes.StopOptions{})
+ err = client.ContainerStop(ctx, container1Name, nil)
assert.NilError(t, err)
err = client.ContainerStart(ctx, container1Name, types.ContainerStartOptions{})
assert.NilError(t, err)
diff --git a/integration/container/stop_linux_test.go b/integration/container/stop_linux_test.go
index 0535ce7787..9ce11f37d1 100644
--- a/integration/container/stop_linux_test.go
+++ b/integration/container/stop_linux_test.go
@@ -9,7 +9,6 @@ import (
"time"
"github.com/docker/docker/api/types"
- containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/integration/internal/container"
"gotest.tools/v3/assert"
"gotest.tools/v3/icmd"
@@ -57,7 +56,8 @@ func TestStopContainerWithTimeout(t *testing.T) {
t.Parallel()
id := container.Run(ctx, t, client, testCmd)
- err := client.ContainerStop(ctx, id, containertypes.StopOptions{Timeout: &d.timeout})
+ timeout := time.Duration(d.timeout) * time.Second
+ err := client.ContainerStop(ctx, id, &timeout)
assert.NilError(t, err)
poll.WaitOn(t, container.IsStopped(ctx, client, id),
diff --git a/integration/container/stop_test.go b/integration/container/stop_test.go
index b026527e88..7e12996e33 100644
--- a/integration/container/stop_test.go
+++ b/integration/container/stop_test.go
@@ -5,7 +5,6 @@ import (
"testing"
"time"
- containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/integration/internal/container"
"gotest.tools/v3/assert"
"gotest.tools/v3/poll"
@@ -30,7 +29,7 @@ func TestStopContainerWithRestartPolicyAlways(t *testing.T) {
}
for _, name := range names {
- err := client.ContainerStop(ctx, name, containertypes.StopOptions{})
+ err := client.ContainerStop(ctx, name, nil)
assert.NilError(t, err)
}
diff --git a/integration/container/stop_windows_test.go b/integration/container/stop_windows_test.go
index 65683822e9..a20ba5e6d9 100644
--- a/integration/container/stop_windows_test.go
+++ b/integration/container/stop_windows_test.go
@@ -6,7 +6,6 @@ import (
"testing"
"time"
- containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/integration/internal/container"
"gotest.tools/v3/assert"
"gotest.tools/v3/poll"
@@ -54,7 +53,8 @@ func TestStopContainerWithTimeout(t *testing.T) {
t.Parallel()
id := container.Run(ctx, t, client, testCmd)
- err := client.ContainerStop(ctx, id, containertypes.StopOptions{Timeout: &d.timeout})
+ timeout := time.Duration(d.timeout) * time.Second
+ err := client.ContainerStop(ctx, id, &timeout)
assert.NilError(t, err)
poll.WaitOn(t, container.IsStopped(ctx, client, id),
diff --git a/integration/container/wait_test.go b/integration/container/wait_test.go
index 990868fd35..44e2c81143 100644
--- a/integration/container/wait_test.go
+++ b/integration/container/wait_test.go
@@ -5,7 +5,6 @@ import (
"testing"
"time"
- containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/integration/internal/container"
"github.com/docker/docker/testutil/request"
"gotest.tools/v3/assert"
@@ -87,7 +86,7 @@ func TestWaitBlocked(t *testing.T) {
waitResC, errC := cli.ContainerWait(ctx, containerID, "")
- err := cli.ContainerStop(ctx, containerID, containertypes.StopOptions{})
+ err := cli.ContainerStop(ctx, containerID, nil)
assert.NilError(t, err)
select {
--
2.37.3