fix: ctx should cancel image pull on run

This patch fixes the context cancellation
behaviour for the `runContainer` function,
specifically the `createContainer` function
introduced in this commit 991b1303da.

It delays stripping the `cancel` from the context
passed into the `runContainer` function so that
the `createContainer` function can be cancelled
gracefully by a SIGTERM/SIGINT.

This is especially true when the requested image
does not exist and `docker run` needs to `pull`
the image before creating the container.

Although this patch does gracefully cancel
the `runContainer` function it does not address
the root cause. Some functions in the call path
are not context aware, such as `pullImage`.

Future work would still be necessary to ensure
a consistent behaviour in the CLI.

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
This commit is contained in:
Alano Terblanche
2024-11-22 14:13:40 +01:00
parent 682cf57d73
commit 30a73ff19c
4 changed files with 93 additions and 9 deletions

View File

@ -2,7 +2,9 @@ package container
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net"
"syscall"
@ -16,7 +18,9 @@ import (
"github.com/docker/cli/internal/test/notary"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/image"
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/pkg/jsonmessage"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/spf13/pflag"
"gotest.tools/v3/assert"
@ -217,6 +221,88 @@ func TestRunAttachTermination(t *testing.T) {
}
}
func TestRunPullTermination(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
attachCh := make(chan struct{})
fakeCLI := test.NewFakeCli(&fakeClient{
createContainerFunc: func(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig,
platform *specs.Platform, containerName string,
) (container.CreateResponse, error) {
select {
case <-ctx.Done():
return container.CreateResponse{}, ctx.Err()
default:
}
return container.CreateResponse{}, fakeNotFound{}
},
containerAttachFunc: func(ctx context.Context, containerID string, options container.AttachOptions) (types.HijackedResponse, error) {
return types.HijackedResponse{}, errors.New("shouldn't try to attach to a container")
},
imageCreateFunc: func(ctx context.Context, parentReference string, options image.CreateOptions) (io.ReadCloser, error) {
server, client := net.Pipe()
t.Cleanup(func() {
_ = server.Close()
})
go func() {
enc := json.NewEncoder(server)
for i := 0; i < 100; i++ {
select {
case <-ctx.Done():
assert.NilError(t, server.Close(), "failed to close imageCreateFunc server")
return
default:
}
assert.NilError(t, enc.Encode(jsonmessage.JSONMessage{
Status: "Downloading",
ID: fmt.Sprintf("id-%d", i),
TimeNano: time.Now().UnixNano(),
Time: time.Now().Unix(),
Progress: &jsonmessage.JSONProgress{
Current: int64(i),
Total: 100,
Start: 0,
},
}))
time.Sleep(100 * time.Millisecond)
}
}()
attachCh <- struct{}{}
return client, nil
},
Version: "1.30",
})
cmd := NewRunCommand(fakeCLI)
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
cmd.SetArgs([]string{"foobar:latest"})
cmdErrC := make(chan error, 1)
go func() {
cmdErrC <- cmd.ExecuteContext(ctx)
}()
select {
case <-time.After(5 * time.Second):
t.Fatal("imageCreateFunc was not called before the timeout")
case <-attachCh:
}
cancel()
select {
case cmdErr := <-cmdErrC:
assert.Equal(t, cmdErr, cli.StatusError{
StatusCode: 125,
Status: "docker: context canceled\n\nRun 'docker run --help' for more information",
})
case <-time.After(10 * time.Second):
t.Fatal("cmd did not return before the timeout")
}
}
func TestRunCommandWithContentTrustErrors(t *testing.T) {
testCases := []struct {
name string