[24.0 backport] daemon: daemon.containerRestart: don't cancel restart on context cancel

[thaJeztah]

• backport of daemon: daemon.containerRestart: don't cancel restart on context cancel #46688
• depends on [24.0 backport] daemon/c8d: Use non cancellable context in defers #46686
• fixes Restart container from inside #46682
• relates to imageservice: Add context to various methods #44365
• relates to don't cancel container stop when cancelling context #45738

commit def549c (#44365) passed through the context to the daemon.ContainerStart function. As a result, restarting containers no longer is an atomic operation, because a context cancellation could interrupt the restart (between "stopping" and "(re)starting"), resulting in the container being stopped, but not restarted.

Restarting a container, or more factually; making a successful request on the /containers/{id]/restart endpoint, should be an atomic operation.

This patch uses a context.WithoutCancel for restart requests.

It's worth noting that daemon.containerStop already uses context.WithoutCancel, so in that function, we'll be wrapping the context twice, but this should likely not cause issues (just redundant for this code-path).

Before this patch, starting a container that bind-mounts the docker socket, then restarting itself from within the container would cancel the restart operation. The container would be stopped, but not started after that:

docker run -dit --name myself -v /var/run/docker.sock:/var/run/docker.sock docker:cli sh
docker exec myself sh -c 'docker restart myself'

docker ps -a
CONTAINER ID   IMAGE         COMMAND                  CREATED          STATUS                       PORTS     NAMES
3a2a741c65ff   docker:cli    "docker-entrypoint.s…"   26 seconds ago   Exited (128) 7 seconds ago             myself

With this patch: the stop still cancels the exec, but does not cancel the restart operation, and the container is started again:

docker run -dit --name myself -v /var/run/docker.sock:/var/run/docker.sock docker:cli sh
docker exec myself sh -c 'docker restart myself'
docker ps
CONTAINER ID   IMAGE        COMMAND                  CREATED              STATUS         PORTS     NAMES
4393a01f7c75   docker:cli   "docker-entrypoint.s…"   About a minute ago   Up 4 seconds             myself

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[thaJeztah]

Some changes compared to the PR in master, due to things not yet being in the 24.0 branch;

git diff
diff --git a/integration/container/restart_test.go b/integration/container/restart_test.go
index cf4e0c4975..bceb605c34 100644
--- a/integration/container/restart_test.go
+++ b/integration/container/restart_test.go
@@ -9,7 +9,6 @@ import (

        "github.com/docker/docker/api/types"
        "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/client"
        testContainer "github.com/docker/docker/integration/internal/container"
@@ -219,11 +218,10 @@ func TestContainerWithAutoRemoveCanBeRestarted(t *testing.T) {
 //
 // Regression test for https://github.com/moby/moby/discussions/46682
 func TestContainerRestartWithCancelledRequest(t *testing.T) {
-       ctx := setupTest(t)
+       ctx := context.TODO()
+       defer setupTest(t)()
        apiClient := testEnv.APIClient()

-       testutil.StartSpan(ctx, t)
-
        // Create a container that ignores SIGTERM and doesn't stop immediately,
        // giving us time to cancel the request.
        //
@@ -233,7 +231,7 @@ func TestContainerRestartWithCancelledRequest(t *testing.T) {
        // taking place.
        cID := testContainer.Run(ctx, t, apiClient, testContainer.WithCmd("sh", "-c", "trap 'echo received TERM' TERM; while true; do usleep 10; done"))
        defer func() {
-               err := apiClient.ContainerRemove(ctx, cID, container.RemoveOptions{Force: true})
+               err := apiClient.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true})
                if t.Failed() && err != nil {
                        t.Logf("Cleaning up test container failed with error: %v", err)
                }
@@ -243,7 +241,7 @@ func TestContainerRestartWithCancelledRequest(t *testing.T) {
        messages, errs := apiClient.Events(ctx, types.EventsOptions{
                Filters: filters.NewArgs(
                        filters.Arg("container", cID),
-                       filters.Arg("event", string(events.ActionRestart)),
+                       filters.Arg("event", "restart"),
                ),
        })

@@ -270,7 +268,7 @@ func TestContainerRestartWithCancelledRequest(t *testing.T) {
        select {
        case m := <-messages:
                assert.Check(t, is.Equal(m.Actor.ID, cID))
-               assert.Check(t, is.Equal(m.Action, events.ActionRestart))
+               assert.Check(t, is.Equal(m.Action, "restart"))
        case err := <-errs:
                assert.NilError(t, err)
        case <-time.After(restartTimeout):