From 7a89e897eadd5a385c39cde645faa60e6c3c0042 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 19 Jul 2023 14:34:07 +0200 Subject: [PATCH] cli/command/container: waitExitOrRemoved: take APIClient as argument It only needs the API client, not the whole DockerCLI. Signed-off-by: Sebastiaan van Stijn --- cli/command/container/run.go | 2 +- cli/command/container/start.go | 2 +- cli/command/container/utils.go | 18 +++++++++--------- cli/command/container/utils_test.go | 7 +++---- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/cli/command/container/run.go b/cli/command/container/run.go index b171450c91..9d0e3d742d 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -187,7 +187,7 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio defer closeFn() } - statusChan := waitExitOrRemoved(ctx, dockerCli, containerID, copts.autoRemove) + statusChan := waitExitOrRemoved(ctx, dockerCli.Client(), containerID, copts.autoRemove) // start the container if err := client.ContainerStart(ctx, containerID, types.ContainerStartOptions{}); err != nil { diff --git a/cli/command/container/start.go b/cli/command/container/start.go index 0a0b992ca8..febe0b8fc9 100644 --- a/cli/command/container/start.go +++ b/cli/command/container/start.go @@ -145,7 +145,7 @@ func RunStart(dockerCli command.Cli, opts *StartOptions) error { // 3. We should open a channel for receiving status code of the container // no matter it's detached, removed on daemon side(--rm) or exit normally. - statusChan := waitExitOrRemoved(ctx, dockerCli, c.ID, c.HostConfig.AutoRemove) + statusChan := waitExitOrRemoved(ctx, dockerCli.Client(), c.ID, c.HostConfig.AutoRemove) // 4. Start the container. err = dockerCli.Client().ContainerStart(ctx, c.ID, types.ContainerStartOptions{ diff --git a/cli/command/container/utils.go b/cli/command/container/utils.go index f329261417..0989c10b10 100644 --- a/cli/command/container/utils.go +++ b/cli/command/container/utils.go @@ -4,16 +4,16 @@ import ( "context" "strconv" - "github.com/docker/cli/cli/command" "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/api/types/versions" + "github.com/docker/docker/client" "github.com/sirupsen/logrus" ) -func waitExitOrRemoved(ctx context.Context, dockerCli command.Cli, containerID string, waitRemove bool) <-chan int { +func waitExitOrRemoved(ctx context.Context, apiClient client.APIClient, containerID string, waitRemove bool) <-chan int { if len(containerID) == 0 { // containerID can never be empty panic("Internal Error: waitExitOrRemoved needs a containerID as parameter") @@ -22,8 +22,8 @@ func waitExitOrRemoved(ctx context.Context, dockerCli command.Cli, containerID s // Older versions used the Events API, and even older versions did not // support server-side removal. This legacyWaitExitOrRemoved method // preserves that old behavior and any issues it may have. - if versions.LessThan(dockerCli.Client().ClientVersion(), "1.30") { - return legacyWaitExitOrRemoved(ctx, dockerCli, containerID, waitRemove) + if versions.LessThan(apiClient.ClientVersion(), "1.30") { + return legacyWaitExitOrRemoved(ctx, apiClient, containerID, waitRemove) } condition := container.WaitConditionNextExit @@ -31,7 +31,7 @@ func waitExitOrRemoved(ctx context.Context, dockerCli command.Cli, containerID s condition = container.WaitConditionRemoved } - resultC, errC := dockerCli.Client().ContainerWait(ctx, containerID, condition) + resultC, errC := apiClient.ContainerWait(ctx, containerID, condition) statusC := make(chan int) go func() { @@ -52,7 +52,7 @@ func waitExitOrRemoved(ctx context.Context, dockerCli command.Cli, containerID s return statusC } -func legacyWaitExitOrRemoved(ctx context.Context, dockerCli command.Cli, containerID string, waitRemove bool) <-chan int { +func legacyWaitExitOrRemoved(ctx context.Context, apiClient client.APIClient, containerID string, waitRemove bool) <-chan int { var removeErr error statusChan := make(chan int) exitCode := 125 @@ -65,7 +65,7 @@ func legacyWaitExitOrRemoved(ctx context.Context, dockerCli command.Cli, contain Filters: f, } eventCtx, cancel := context.WithCancel(ctx) - eventq, errq := dockerCli.Client().Events(eventCtx, options) + eventq, errq := apiClient.Events(eventCtx, options) eventProcessor := func(e events.Message) bool { stopProcessing := false @@ -84,9 +84,9 @@ func legacyWaitExitOrRemoved(ctx context.Context, dockerCli command.Cli, contain } else { // If we are talking to an older daemon, `AutoRemove` is not supported. // We need to fall back to the old behavior, which is client-side removal - if versions.LessThan(dockerCli.Client().ClientVersion(), "1.25") { + if versions.LessThan(apiClient.ClientVersion(), "1.25") { go func() { - removeErr = dockerCli.Client().ContainerRemove(ctx, containerID, types.ContainerRemoveOptions{RemoveVolumes: true}) + removeErr = apiClient.ContainerRemove(ctx, containerID, types.ContainerRemoveOptions{RemoveVolumes: true}) if removeErr != nil { logrus.Errorf("error removing container: %v", removeErr) cancel() // cancel the event Q diff --git a/cli/command/container/utils_test.go b/cli/command/container/utils_test.go index 295b347882..7903113c56 100644 --- a/cli/command/container/utils_test.go +++ b/cli/command/container/utils_test.go @@ -2,13 +2,12 @@ package container import ( "context" + "fmt" "strings" "testing" - "github.com/docker/cli/internal/test" "github.com/docker/docker/api" "github.com/docker/docker/api/types/container" - "github.com/pkg/errors" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -24,7 +23,7 @@ func waitFn(cid string) (<-chan container.WaitResponse, <-chan error) { res.StatusCode = 42 resC <- res case strings.Contains(cid, "non-existent"): - err := errors.Errorf("no such container: %v", cid) + err := fmt.Errorf("no such container: %v", cid) errC <- err case strings.Contains(cid, "wait-error"): res.Error = &container.WaitExitError{Message: "removal failed"} @@ -61,7 +60,7 @@ func TestWaitExitOrRemoved(t *testing.T) { }, } - client := test.NewFakeCli(&fakeClient{waitFunc: waitFn, Version: api.DefaultVersion}) + client := &fakeClient{waitFunc: waitFn, Version: api.DefaultVersion} for _, testcase := range testcases { statusC := waitExitOrRemoved(context.Background(), client, testcase.cid, true) exitCode := <-statusC