Merge pull request #51323 from thaJeztah/container_wait_nits

client: ContainerWait: touch-up GoDoc, and remove legacy code, and use singular for channels (ContainerWaitResult)

client/container_wait.go

@@ -13,34 +13,31 @@ import (
 
 const containerWaitErrorMsgLimit = 2 * 1024 /* Max: 2KiB */
 
-// ContainerWaitOptions holds options for container wait operations.
+// ContainerWaitOptions holds options for [Client.ContainerWait].
 type ContainerWaitOptions struct {
 	Condition container.WaitCondition
 }
 
-// ContainerWaitResult defines the result of a container wait operation.
+// ContainerWaitResult defines the result from the [Client.ContainerWait] method.
 type ContainerWaitResult struct {
-	Results <-chan container.WaitResponse
-	Errors  <-chan error
+	Result <-chan container.WaitResponse
+	Error  <-chan error
 }
 
 // ContainerWait waits until the specified container is in a certain state
-// indicated by the given condition, either "not-running" ([container.WaitConditionNotRunning])
-// (default),  "next-exit" ([container.WaitConditionNextExit]), or "removed".
-// ([container.WaitConditionRemoved]).
+// indicated by the given condition, either;
 //
-// If this client's API version is before 1.30, "condition" is ignored and
-// ContainerWait returns immediately with the two channels, as the server
-// waits as if the condition were "not-running".
+//   - "not-running" ([container.WaitConditionNotRunning]) (default)
+//   - "next-exit" ([container.WaitConditionNextExit])
+//   - "removed" ([container.WaitConditionRemoved])
 //
-// If this client's API version is at least 1.30, ContainerWait blocks until
-// the request has been acknowledged by the server (with a response header),
-// then returns two channels on which the caller can wait for the exit status
-// of the container or an error if there was a problem either beginning the
-// wait request or in getting the response. This allows the caller to
-// synchronize ContainerWait with other calls, such as specifying a
-// "next-exit" condition ([container.WaitConditionNextExit]) before
-// issuing a [Client.ContainerStart] request.
+// ContainerWait blocks until the request has been acknowledged by the server
+// (with a response header), then returns two channels on which the caller can
+// wait for the exit status of the container or an error if there was a problem
+// either beginning the wait request or in getting the response. This allows the
+// caller to synchronize ContainerWait with other calls, such as specifying a
+// "next-exit" condition ([container.WaitConditionNextExit]) before issuing a
+// [Client.ContainerStart] request.
 func (cli *Client) ContainerWait(ctx context.Context, containerID string, options ContainerWaitOptions) ContainerWaitResult {
 	resultC := make(chan container.WaitResponse)
 	errC := make(chan error, 1)
@@ -48,7 +45,7 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, option
 	containerID, err := trimID("container", containerID)
 	if err != nil {
 		errC <- err
-		return ContainerWaitResult{Results: resultC, Errors: errC}
+		return ContainerWaitResult{Result: resultC, Error: errC}
 	}
 
 	query := url.Values{}
@@ -60,7 +57,7 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, option
 	if err != nil {
 		defer ensureReaderClosed(resp)
 		errC <- err
-		return ContainerWaitResult{Results: resultC, Errors: errC}
+		return ContainerWaitResult{Result: resultC, Error: errC}
 	}
 
 	go func() {
@@ -91,31 +88,5 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, option
 		resultC <- res
 	}()
 
-	return ContainerWaitResult{Results: resultC, Errors: errC}
-}
-
-// legacyContainerWait returns immediately and doesn't have an option to wait
-// until the container is removed.
-func (cli *Client) legacyContainerWait(ctx context.Context, containerID string) (<-chan container.WaitResponse, <-chan error) {
-	resultC := make(chan container.WaitResponse)
-	errC := make(chan error)
-
-	go func() {
-		resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", nil, nil, nil)
-		if err != nil {
-			errC <- err
-			return
-		}
-		defer ensureReaderClosed(resp)
-
-		var res container.WaitResponse
-		if err := json.NewDecoder(resp.Body).Decode(&res); err != nil {
-			errC <- err
-			return
-		}
-
-		resultC <- res
-	}()
-
-	return resultC, errC
+	return ContainerWaitResult{Result: resultC, Error: errC}
 }

client/container_wait_test.go

@@ -21,11 +21,11 @@ import (
 func TestContainerWaitError(t *testing.T) {
 	client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error")))
 	assert.NilError(t, err)
-	wait := client.ContainerWait(context.Background(), "nothing", ContainerWaitOptions{})
+	wait := client.ContainerWait(t.Context(), "nothing", ContainerWaitOptions{})
 	select {
-	case result := <-wait.Results:
+	case result := <-wait.Result:
 		t.Fatalf("expected to not get a wait result, got %d", result.StatusCode)
-	case err := <-wait.Errors:
+	case err := <-wait.Error:
 		assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))
 	}
 }
@@ -38,11 +38,11 @@ func TestContainerWaitConnectionError(t *testing.T) {
 	client, err := NewClientWithOpts(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid"))
 	assert.NilError(t, err)
 
-	wait := client.ContainerWait(context.Background(), "nothing", ContainerWaitOptions{})
+	wait := client.ContainerWait(t.Context(), "nothing", ContainerWaitOptions{})
 	select {
-	case result := <-wait.Results:
+	case result := <-wait.Result:
 		t.Fatalf("expected to not get a wait result, got %d", result.StatusCode)
-	case err := <-wait.Errors:
+	case err := <-wait.Error:
 		assert.Check(t, is.ErrorType(err, IsErrConnectionFailed))
 	}
 }
@@ -59,11 +59,11 @@ func TestContainerWait(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	wait := client.ContainerWait(context.Background(), "container_id", ContainerWaitOptions{})
+	wait := client.ContainerWait(t.Context(), "container_id", ContainerWaitOptions{})
 	select {
-	case err := <-wait.Errors:
+	case err := <-wait.Error:
 		assert.NilError(t, err)
-	case result := <-wait.Results:
+	case result := <-wait.Result:
 		assert.Check(t, is.Equal(result.StatusCode, int64(15)))
 	}
 }
@@ -82,11 +82,11 @@ func TestContainerWaitProxyInterrupt(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	wait := client.ContainerWait(context.Background(), "container_id", ContainerWaitOptions{})
+	wait := client.ContainerWait(t.Context(), "container_id", ContainerWaitOptions{})
 	select {
-	case err := <-wait.Errors:
+	case err := <-wait.Error:
 		assert.Check(t, is.ErrorContains(err, expErr))
-	case result := <-wait.Results:
+	case result := <-wait.Result:
 		t.Fatalf("Unexpected result: %v", result)
 	}
 }
@@ -102,12 +102,12 @@ func TestContainerWaitProxyInterruptLong(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	wait := client.ContainerWait(context.Background(), "container_id", ContainerWaitOptions{})
+	wait := client.ContainerWait(t.Context(), "container_id", ContainerWaitOptions{})
 	select {
-	case err := <-wait.Errors:
+	case err := <-wait.Error:
 		// LimitReader limiting isn't exact, because of how the Readers do chunking.
 		assert.Check(t, len(err.Error()) <= containerWaitErrorMsgLimit*2, "Expected error to be limited around %d, actual length: %d", containerWaitErrorMsgLimit, len(err.Error()))
-	case result := <-wait.Results:
+	case result := <-wait.Result:
 		t.Fatalf("Unexpected result: %v", result)
 	}
 }
@@ -124,7 +124,7 @@ func TestContainerWaitErrorHandling(t *testing.T) {
 		{name: "connection reset", rdr: iotest.ErrReader(syscall.ECONNRESET), exp: syscall.ECONNRESET},
 	} {
 		t.Run(test.name, func(t *testing.T) {
-			ctx, cancel := context.WithCancel(context.Background())
+			ctx, cancel := context.WithCancel(t.Context())
 			defer cancel()
 
 			client, err := NewClientWithOpts(WithMockClient(func(req *http.Request) (*http.Response, error) {
@@ -136,10 +136,10 @@ func TestContainerWaitErrorHandling(t *testing.T) {
 			assert.NilError(t, err)
 			wait := client.ContainerWait(ctx, "container_id", ContainerWaitOptions{})
 			select {
-			case err := <-wait.Errors:
+			case err := <-wait.Error:
 				assert.Check(t, is.Equal(err.Error(), test.exp.Error()))
 				return
-			case result := <-wait.Results:
+			case result := <-wait.Result:
 				t.Fatalf("expected to not get a wait result, got %d", result.StatusCode)
 				return
 			}
@@ -154,7 +154,7 @@ func ExampleClient_ContainerWait_withTimeout() {
 
 	client, _ := NewClientWithOpts(FromEnv)
 	wait := client.ContainerWait(ctx, "container_id", ContainerWaitOptions{})
-	if err := <-wait.Errors; err != nil {
+	if err := <-wait.Error; err != nil {
 		log.Fatal(err)
 	}
 }

integration-cli/docker_api_containers_test.go

@@ -823,9 +823,9 @@ func (s *DockerAPISuite) TestContainerAPIWait(c *testing.T) {
 	wait := apiClient.ContainerWait(testutil.GetContext(c), name, client.ContainerWaitOptions{})
 
 	select {
-	case err = <-wait.Errors:
+	case err = <-wait.Error:
 		assert.NilError(c, err)
-	case waitRes := <-wait.Results:
+	case waitRes := <-wait.Result:
 		assert.Equal(c, waitRes.StatusCode, int64(0))
 	}
 }

integration/container/create_test.go

@@ -508,13 +508,13 @@ func TestCreateTmpfsOverrideAnonymousVolume(t *testing.T) {
 	select {
 	case <-timeout.C:
 		t.Fatal("timeout waiting for container to exit")
-	case status := <-wait.Results:
+	case status := <-wait.Result:
 		var errMsg string
 		if status.Error != nil {
 			errMsg = status.Error.Message
 		}
 		assert.Equal(t, int(status.StatusCode), 0, errMsg)
-	case err := <-wait.Errors:
+	case err := <-wait.Error:
 		assert.NilError(t, err)
 	}
 }

integration/container/daemon_linux_test.go

@@ -218,8 +218,8 @@ func TestRestartDaemonWithRestartingContainer(t *testing.T) {
 	defer cancel()
 	wait := apiClient.ContainerWait(ctxTimeout, id, client.ContainerWaitOptions{Condition: containertypes.WaitConditionNextExit})
 	select {
-	case <-wait.Results:
-	case err := <-wait.Errors:
+	case <-wait.Result:
+	case err := <-wait.Error:
 		assert.NilError(t, err)
 	}
 }

integration/container/wait_test.go

@@ -47,9 +47,9 @@ func TestWaitNonBlocked(t *testing.T) {
 
 			wait := cli.ContainerWait(ctx, containerID, client.ContainerWaitOptions{})
 			select {
-			case err := <-wait.Errors:
+			case err := <-wait.Error:
 				assert.NilError(t, err)
-			case waitRes := <-wait.Results:
+			case waitRes := <-wait.Result:
 				assert.Check(t, is.Equal(tc.expectedCode, waitRes.StatusCode))
 			}
 		})
@@ -91,9 +91,9 @@ func TestWaitBlocked(t *testing.T) {
 			assert.NilError(t, err)
 
 			select {
-			case err := <-wait.Errors:
+			case err := <-wait.Error:
 				assert.NilError(t, err)
-			case waitRes := <-wait.Results:
+			case waitRes := <-wait.Result:
 				assert.Check(t, is.Equal(tc.expectedCode, waitRes.StatusCode))
 			case <-time.After(2 * time.Second):
 				t.Fatal("timeout waiting for `docker wait`")
@@ -152,9 +152,9 @@ func TestWaitConditions(t *testing.T) {
 			assert.NilError(t, err)
 			wait := cli.ContainerWait(ctx, containerID, client.ContainerWaitOptions{Condition: tc.waitCond})
 			select {
-			case err := <-wait.Errors:
+			case err := <-wait.Error:
 				t.Fatalf("ContainerWait() err = %v", err)
-			case res := <-wait.Results:
+			case res := <-wait.Result:
 				t.Fatalf("ContainerWait() sent exit code (%v) before ContainerStart()", res)
 			default:
 			}
@@ -166,9 +166,9 @@ func TestWaitConditions(t *testing.T) {
 			assert.NilError(t, err)
 
 			select {
-			case err := <-wait.Errors:
+			case err := <-wait.Error:
 				assert.NilError(t, err)
-			case waitRes := <-wait.Results:
+			case waitRes := <-wait.Result:
 				assert.Check(t, is.Equal(int64(99), waitRes.StatusCode))
 			case <-time.After(StopContainerWindowsPollTimeout):
 				ctr, _ := cli.ContainerInspect(ctx, containerID, client.ContainerInspectOptions{})
@@ -229,11 +229,11 @@ func TestWaitRestartedContainer(t *testing.T) {
 			assert.NilError(t, err)
 
 			select {
-			case err := <-wait.Errors:
+			case err := <-wait.Error:
 				t.Fatalf("Unexpected error: %v", err)
 			case <-time.After(time.Second * 3):
 				t.Fatalf("Wait should end after restart")
-			case waitRes := <-wait.Results:
+			case waitRes := <-wait.Result:
 				expectedCode := int64(5)
 
 				if !isWindowDaemon {

vendor/github.com/moby/moby/client/container_wait.go

@@ -13,34 +13,31 @@ import (
 
 const containerWaitErrorMsgLimit = 2 * 1024 /* Max: 2KiB */
 
-// ContainerWaitOptions holds options for container wait operations.
+// ContainerWaitOptions holds options for [Client.ContainerWait].
 type ContainerWaitOptions struct {
 	Condition container.WaitCondition
 }
 
-// ContainerWaitResult defines the result of a container wait operation.
+// ContainerWaitResult defines the result from the [Client.ContainerWait] method.
 type ContainerWaitResult struct {
-	Results <-chan container.WaitResponse
-	Errors  <-chan error
+	Result <-chan container.WaitResponse
+	Error  <-chan error
 }
 
 // ContainerWait waits until the specified container is in a certain state
-// indicated by the given condition, either "not-running" ([container.WaitConditionNotRunning])
-// (default),  "next-exit" ([container.WaitConditionNextExit]), or "removed".
-// ([container.WaitConditionRemoved]).
+// indicated by the given condition, either;
 //
-// If this client's API version is before 1.30, "condition" is ignored and
-// ContainerWait returns immediately with the two channels, as the server
-// waits as if the condition were "not-running".
+//   - "not-running" ([container.WaitConditionNotRunning]) (default)
+//   - "next-exit" ([container.WaitConditionNextExit])
+//   - "removed" ([container.WaitConditionRemoved])
 //
-// If this client's API version is at least 1.30, ContainerWait blocks until
-// the request has been acknowledged by the server (with a response header),
-// then returns two channels on which the caller can wait for the exit status
-// of the container or an error if there was a problem either beginning the
-// wait request or in getting the response. This allows the caller to
-// synchronize ContainerWait with other calls, such as specifying a
-// "next-exit" condition ([container.WaitConditionNextExit]) before
-// issuing a [Client.ContainerStart] request.
+// ContainerWait blocks until the request has been acknowledged by the server
+// (with a response header), then returns two channels on which the caller can
+// wait for the exit status of the container or an error if there was a problem
+// either beginning the wait request or in getting the response. This allows the
+// caller to synchronize ContainerWait with other calls, such as specifying a
+// "next-exit" condition ([container.WaitConditionNextExit]) before issuing a
+// [Client.ContainerStart] request.
 func (cli *Client) ContainerWait(ctx context.Context, containerID string, options ContainerWaitOptions) ContainerWaitResult {
 	resultC := make(chan container.WaitResponse)
 	errC := make(chan error, 1)
@@ -48,7 +45,7 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, option
 	containerID, err := trimID("container", containerID)
 	if err != nil {
 		errC <- err
-		return ContainerWaitResult{Results: resultC, Errors: errC}
+		return ContainerWaitResult{Result: resultC, Error: errC}
 	}
 
 	query := url.Values{}
@@ -60,7 +57,7 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, option
 	if err != nil {
 		defer ensureReaderClosed(resp)
 		errC <- err
-		return ContainerWaitResult{Results: resultC, Errors: errC}
+		return ContainerWaitResult{Result: resultC, Error: errC}
 	}
 
 	go func() {
@@ -91,31 +88,5 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, option
 		resultC <- res
 	}()
 
-	return ContainerWaitResult{Results: resultC, Errors: errC}
-}
-
-// legacyContainerWait returns immediately and doesn't have an option to wait
-// until the container is removed.
-func (cli *Client) legacyContainerWait(ctx context.Context, containerID string) (<-chan container.WaitResponse, <-chan error) {
-	resultC := make(chan container.WaitResponse)
-	errC := make(chan error)
-
-	go func() {
-		resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", nil, nil, nil)
-		if err != nil {
-			errC <- err
-			return
-		}
-		defer ensureReaderClosed(resp)
-
-		var res container.WaitResponse
-		if err := json.NewDecoder(resp.Body).Decode(&res); err != nil {
-			errC <- err
-			return
-		}
-
-		resultC <- res
-	}()
-
-	return resultC, errC
+	return ContainerWaitResult{Result: resultC, Error: errC}
 }