don't cancel container stop when cancelling context

Commit 90de570cfa passed through the request
context to daemon.ContainerStop(). As a result, cancelling the context would
cancel the "graceful" stop of the container, and would proceed with forcefully
killing the container.

This patch partially reverts the changes from 90de570cfa
and breaks the context to prevent cancelling the context from cancelling the stop.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit fc94ed0a86)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/stop.go

@@ -36,7 +36,13 @@ func (daemon *Daemon) ContainerStop(ctx context.Context, name string, options co
 }
 
 // containerStop sends a stop signal, waits, sends a kill signal.
-func (daemon *Daemon) containerStop(ctx context.Context, ctr *container.Container, options containertypes.StopOptions) (retErr error) {
+func (daemon *Daemon) containerStop(_ context.Context, ctr *container.Container, options containertypes.StopOptions) (retErr error) {
+	// Deliberately using a local context here, because cancelling the
+	// request should not cancel the stop.
+	//
+	// TODO(thaJeztah): pass context, and use context.WithoutCancel() once available: https://github.com/golang/go/issues/40221
+	ctx := context.Background()
+
 	if !ctr.IsRunning() {
 		return nil
 	}

integration/container/stop_linux_test.go

@@ -1,8 +1,10 @@
 package container // import "github.com/docker/docker/integration/container"
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"strconv"
 	"strings"
 	"testing"
@@ -10,8 +12,12 @@ import (
 
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/client"
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/integration/internal/container"
+	"github.com/docker/docker/pkg/stdcopy"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/icmd"
 	"gotest.tools/v3/poll"
 	"gotest.tools/v3/skip"
@@ -98,3 +104,77 @@ func TestDeleteDevicemapper(t *testing.T) {
 	err = client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{})
 	assert.NilError(t, err)
 }
+
+// TestStopContainerWithTimeoutCancel checks that ContainerStop is not cancelled
+// if the request is cancelled.
+// See issue https://github.com/moby/moby/issues/45731
+func TestStopContainerWithTimeoutCancel(t *testing.T) {
+	t.Parallel()
+	defer setupTest(t)()
+	apiClient := testEnv.APIClient()
+	t.Cleanup(func() { _ = apiClient.Close() })
+
+	ctx := context.Background()
+	id := container.Run(ctx, t, apiClient,
+		container.WithCmd("sh", "-c", "trap 'echo received TERM' TERM; while true; do usleep 10; done"),
+	)
+	poll.WaitOn(t, container.IsInState(ctx, apiClient, id, "running"))
+
+	ctxCancel, cancel := context.WithCancel(context.Background())
+	t.Cleanup(cancel)
+	const stopTimeout = 3
+
+	stoppedCh := make(chan error)
+	go func() {
+		sto := stopTimeout
+		stoppedCh <- apiClient.ContainerStop(ctxCancel, id, containertypes.StopOptions{Timeout: &sto})
+	}()
+
+	poll.WaitOn(t, logsContains(ctx, apiClient, id, "received TERM"))
+
+	// Cancel the context once we verified the container was signaled, and check
+	// that the container is not killed immediately
+	cancel()
+
+	select {
+	case stoppedErr := <-stoppedCh:
+		assert.Check(t, is.ErrorType(stoppedErr, errdefs.IsCancelled))
+	case <-time.After(5 * time.Second):
+		t.Fatal("timeout waiting for stop request to be cancelled")
+	}
+	inspect, err := apiClient.ContainerInspect(ctx, id)
+	assert.Check(t, err)
+	assert.Check(t, inspect.State.Running)
+
+	// container should be stopped after stopTimeout is reached. The daemon.containerStop
+	// code is rather convoluted, and waits another 2 seconds for the container to
+	// terminate after signaling it;
+	// https://github.com/moby/moby/blob/97455cc31ffa08078db6591f018256ed59c35bbc/daemon/stop.go#L101-L112
+	//
+	// Adding 3 seconds to the specified stopTimeout to take this into account,
+	// and add another second margin to try to avoid flakiness.
+	poll.WaitOn(t, container.IsStopped(ctx, apiClient, id), poll.WithTimeout((3+stopTimeout)*time.Second))
+}
+
+// logsContains verifies the container contains the given text in the log's stdout.
+func logsContains(ctx context.Context, client client.APIClient, containerID string, logString string) func(log poll.LogT) poll.Result {
+	return func(log poll.LogT) poll.Result {
+		logs, err := client.ContainerLogs(ctx, containerID, types.ContainerLogsOptions{
+			ShowStdout: true,
+		})
+		if err != nil {
+			return poll.Error(err)
+		}
+		defer logs.Close()
+
+		var stdout bytes.Buffer
+		_, err = stdcopy.StdCopy(&stdout, io.Discard, logs)
+		if err != nil {
+			return poll.Error(err)
+		}
+		if strings.Contains(stdout.String(), logString) {
+			return poll.Success()
+		}
+		return poll.Continue("waiting for logstring '%s' in container", logString)
+	}
+}