From e8fa708ae5c460ebeec1a88a3ec3615272efaa4f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 16 Feb 2022 11:36:37 +0100 Subject: [PATCH] client: ContainerStop(), ContainerRestart(): support stop-signal Signed-off-by: Sebastiaan van Stijn --- 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, 68 insertions(+), 40 deletions(-) diff --git a/client/container_restart.go b/client/container_restart.go index aa0d6485de..1e0ad99981 100644 --- a/client/container_restart.go +++ b/client/container_restart.go @@ -3,18 +3,22 @@ package client // import "github.com/docker/docker/client" import ( "context" "net/url" - "time" + "strconv" - timetypes "github.com/docker/docker/api/types/time" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/versions" ) // 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, timeout *time.Duration) error { +func (cli *Client) ContainerRestart(ctx context.Context, containerID string, options container.StopOptions) error { query := url.Values{} - if timeout != nil { - query.Set("t", timetypes.DurationToSecondsString(*timeout)) + 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) } 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 085da26b70..8c66525edb 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,20 +17,23 @@ func TestContainerRestartError(t *testing.T) { client := &Client{ client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - timeout := 0 * time.Second - err := client.ContainerRestart(context.Background(), "nothing", &timeout) + err := client.ContainerRestart(context.Background(), "nothing", container.StopOptions{}) if !errdefs.IsSystem(err) { t.Fatalf("expected a Server Error, got %[1]T: %[1]v", err) } } func TestContainerRestart(t *testing.T) { - expectedURL := "/containers/container_id/restart" + const expectedURL = "/v1.42/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) @@ -40,9 +43,13 @@ func TestContainerRestart(t *testing.T) { Body: io.NopCloser(bytes.NewReader([]byte(""))), }, nil }), + version: "1.42", } - timeout := 100 * time.Second - err := client.ContainerRestart(context.Background(), "container_id", &timeout) + timeout := 100 + err := client.ContainerRestart(context.Background(), "container_id", container.StopOptions{ + Signal: "SIGKILL", + Timeout: &timeout, + }) if err != nil { t.Fatal(err) } diff --git a/client/container_stop.go b/client/container_stop.go index 629d7ab64c..2a43ce2274 100644 --- a/client/container_stop.go +++ b/client/container_stop.go @@ -3,9 +3,10 @@ package client // import "github.com/docker/docker/client" import ( "context" "net/url" - "time" + "strconv" - timetypes "github.com/docker/docker/api/types/time" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/versions" ) // ContainerStop stops a container. In case the container fails to stop @@ -15,10 +16,13 @@ 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, timeout *time.Duration) error { +func (cli *Client) ContainerStop(ctx context.Context, containerID string, options container.StopOptions) error { query := url.Values{} - if timeout != nil { - query.Set("t", timetypes.DurationToSecondsString(*timeout)) + 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) } 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 f5df1805cb..ec4cfc0d3c 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,20 +17,23 @@ func TestContainerStopError(t *testing.T) { client := &Client{ client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - timeout := 0 * time.Second - err := client.ContainerStop(context.Background(), "nothing", &timeout) + err := client.ContainerStop(context.Background(), "nothing", container.StopOptions{}) if !errdefs.IsSystem(err) { t.Fatalf("expected a Server Error, got %[1]T: %[1]v", err) } } func TestContainerStop(t *testing.T) { - expectedURL := "/containers/container_id/stop" + const expectedURL = "/v1.42/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) @@ -40,9 +43,13 @@ func TestContainerStop(t *testing.T) { Body: io.NopCloser(bytes.NewReader([]byte(""))), }, nil }), + version: "1.42", } - timeout := 100 * time.Second - err := client.ContainerStop(context.Background(), "container_id", &timeout) + timeout := 100 + err := client.ContainerStop(context.Background(), "container_id", container.StopOptions{ + Signal: "SIGKILL", + Timeout: &timeout, + }) if err != nil { t.Fatal(err) } diff --git a/client/interface.go b/client/interface.go index fe30e63809..cd177475f9 100644 --- a/client/interface.go +++ b/client/interface.go @@ -5,7 +5,6 @@ import ( "io" "net" "net/http" - "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" @@ -65,12 +64,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, timeout *time.Duration) error + ContainerRestart(ctx context.Context, container string, options container.StopOptions) 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, timeout *time.Duration) error + ContainerStop(ctx context.Context, container string, options container.StopOptions) 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 e58814f540..b5e6b18e67 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 * time.Second - err = cli.ContainerRestart(context.Background(), name, &timeout) + timeout := 1 + err = cli.ContainerRestart(context.Background(), name, container.StopOptions{Timeout: &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, nil) + err = cli.ContainerRestart(context.Background(), name, container.StopOptions{}) assert.NilError(c, err) assert.Assert(c, waitInspect(name, "{{ .State.Restarting }} {{ .State.Running }}", "false true", 15*time.Second) == nil) @@ -965,19 +965,23 @@ 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 * time.Second + timeout := 30 cli, err := client.NewClientWithOpts(client.FromEnv) assert.NilError(c, err) defer cli.Close() - err = cli.ContainerStop(context.Background(), name, &timeout) + err = cli.ContainerStop(context.Background(), name, container.StopOptions{ + Timeout: &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, &timeout) + err = cli.ContainerStop(context.Background(), name, container.StopOptions{ + Timeout: &timeout, + }) assert.NilError(c, err) } @@ -1255,7 +1259,7 @@ func (s *DockerSuite) TestContainerAPIPostContainerStop(c *testing.T) { assert.NilError(c, err) defer cli.Close() - err = cli.ContainerStop(context.Background(), containerID, nil) + err = cli.ContainerStop(context.Background(), containerID, container.StopOptions{}) 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 63e7fe3a6e..e34eeee337 100644 --- a/integration/container/pause_test.go +++ b/integration/container/pause_test.go @@ -8,6 +8,7 @@ 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" @@ -80,7 +81,7 @@ func TestPauseStopPausedContainer(t *testing.T) { err := client.ContainerPause(ctx, cID) assert.NilError(t, err) - err = client.ContainerStop(ctx, cID, nil) + err = client.ContainerStop(ctx, cID, containertypes.StopOptions{}) 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 b2ad4902ec..689a37fc32 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, nil) + err = client.ContainerStop(ctx, container1Name, containertypes.StopOptions{}) 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 9ce11f37d1..0535ce7787 100644 --- a/integration/container/stop_linux_test.go +++ b/integration/container/stop_linux_test.go @@ -9,6 +9,7 @@ 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" @@ -56,8 +57,7 @@ func TestStopContainerWithTimeout(t *testing.T) { t.Parallel() id := container.Run(ctx, t, client, testCmd) - timeout := time.Duration(d.timeout) * time.Second - err := client.ContainerStop(ctx, id, &timeout) + err := client.ContainerStop(ctx, id, containertypes.StopOptions{Timeout: &d.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 7e12996e33..b026527e88 100644 --- a/integration/container/stop_test.go +++ b/integration/container/stop_test.go @@ -5,6 +5,7 @@ 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" @@ -29,7 +30,7 @@ func TestStopContainerWithRestartPolicyAlways(t *testing.T) { } for _, name := range names { - err := client.ContainerStop(ctx, name, nil) + err := client.ContainerStop(ctx, name, containertypes.StopOptions{}) assert.NilError(t, err) } diff --git a/integration/container/stop_windows_test.go b/integration/container/stop_windows_test.go index a20ba5e6d9..65683822e9 100644 --- a/integration/container/stop_windows_test.go +++ b/integration/container/stop_windows_test.go @@ -6,6 +6,7 @@ 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" @@ -53,8 +54,7 @@ func TestStopContainerWithTimeout(t *testing.T) { t.Parallel() id := container.Run(ctx, t, client, testCmd) - timeout := time.Duration(d.timeout) * time.Second - err := client.ContainerStop(ctx, id, &timeout) + err := client.ContainerStop(ctx, id, containertypes.StopOptions{Timeout: &d.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 44e2c81143..990868fd35 100644 --- a/integration/container/wait_test.go +++ b/integration/container/wait_test.go @@ -5,6 +5,7 @@ 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" @@ -86,7 +87,7 @@ func TestWaitBlocked(t *testing.T) { waitResC, errC := cli.ContainerWait(ctx, containerID, "") - err := cli.ContainerStop(ctx, containerID, nil) + err := cli.ContainerStop(ctx, containerID, containertypes.StopOptions{}) assert.NilError(t, err) select {