Merge pull request #696 from mlaventure/attach-use-containerwait

attach: Use ContainerWait instead of Inspect

Author: thaJeztah

cli/command/container/attach.go

@@ -1,12 +1,14 @@
 package container
 
 import (
+	"fmt"
 	"io"
 	"net/http/httputil"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/pkg/errors"
@@ -66,6 +68,9 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 	ctx := context.Background()
 	client := dockerCli.Client()
 
+	// request channel to wait for client
+	resultC, errC := client.ContainerWait(ctx, opts.container, "")
+
 	c, err := inspectContainerAndCheckState(ctx, client, opts.container)
 	if err != nil {
 		return err
@@ -140,7 +145,24 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 	if errAttach != nil {
 		return errAttach
 	}
-	return getExitStatus(ctx, dockerCli.Client(), opts.container)
+
+	return getExitStatus(errC, resultC)
+}
+
+func getExitStatus(errC <-chan error, resultC <-chan container.ContainerWaitOKBody) error {
+	select {
+	case result := <-resultC:
+		if result.Error != nil {
+			return fmt.Errorf(result.Error.Message)
+		}
+		if result.StatusCode != 0 {
+			return cli.StatusError{StatusCode: int(result.StatusCode)}
+		}
+	case err := <-errC:
+		return err
+	}
+
+	return nil
 }
 
 func resizeTTY(ctx context.Context, dockerCli command.Cli, containerID string) {
@@ -157,19 +179,3 @@ func resizeTTY(ctx context.Context, dockerCli command.Cli, containerID string) {
 		logrus.Debugf("Error monitoring TTY size: %s", err)
 	}
 }
-
-func getExitStatus(ctx context.Context, apiclient client.ContainerAPIClient, containerID string) error {
-	container, err := apiclient.ContainerInspect(ctx, containerID)
-	if err != nil {
-		// If we can't connect, then the daemon probably died.
-		if !client.IsErrConnectionFailed(err) {
-			return err
-		}
-		return cli.StatusError{StatusCode: -1}
-	}
-	status := container.State.ExitCode
-	if status != 0 {
-		return cli.StatusError{StatusCode: status}
-	}
-	return nil
-}

cli/command/container/attach_test.go

@@ -1,16 +1,17 @@
 package container
 
 import (
+	"fmt"
 	"io/ioutil"
 	"testing"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/cli/internal/test/testutil"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
-	"golang.org/x/net/context"
 )
 
 func TestNewAttachCommandErrors(t *testing.T) {
@@ -78,40 +79,50 @@ func TestNewAttachCommandErrors(t *testing.T) {
 }
 
 func TestGetExitStatus(t *testing.T) {
-	containerID := "the exec id"
-	expecatedErr := errors.New("unexpected error")
+	var (
+		expectedErr = fmt.Errorf("unexpected error")
+		errC        = make(chan error, 1)
+		resultC     = make(chan container.ContainerWaitOKBody, 1)
+	)
 
 	testcases := []struct {
-		inspectError  error
-		exitCode      int
+		result        *container.ContainerWaitOKBody
+		err           error
 		expectedError error
 	}{
 		{
-			inspectError: nil,
-			exitCode:     0,
+			result: &container.ContainerWaitOKBody{
+				StatusCode: 0,
+			},
+		},
+		{
+			err:           expectedErr,
+			expectedError: expectedErr,
 		},
 		{
-			inspectError:  expecatedErr,
-			expectedError: expecatedErr,
+			result: &container.ContainerWaitOKBody{
+				Error: &container.ContainerWaitOKBodyError{
+					expectedErr.Error(),
+				},
+			},
+			expectedError: expectedErr,
 		},
 		{
-			exitCode:      15,
+			result: &container.ContainerWaitOKBody{
+				StatusCode: 15,
+			},
 			expectedError: cli.StatusError{StatusCode: 15},
 		},
 	}
 
 	for _, testcase := range testcases {
-		client := &fakeClient{
-			inspectFunc: func(id string) (types.ContainerJSON, error) {
-				assert.Equal(t, containerID, id)
-				return types.ContainerJSON{
-					ContainerJSONBase: &types.ContainerJSONBase{
-						State: &types.ContainerState{ExitCode: testcase.exitCode},
-					},
-				}, testcase.inspectError
-			},
+		if testcase.err != nil {
+			errC <- testcase.err
+		}
+		if testcase.result != nil {
+			resultC <- *testcase.result
 		}
-		err := getExitStatus(context.Background(), client, containerID)
+		err := getExitStatus(errC, resultC)
 		assert.Equal(t, testcase.expectedError, err)
 	}
 }

e2e/container/attach_test.go

@@ -0,0 +1,29 @@
+package container
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/gotestyourself/gotestyourself/icmd"
+)
+
+func TestAttachExitCode(t *testing.T) {
+	containerID := runBackgroundContainsWithExitCode(t, 21)
+
+	result := icmd.RunCmd(
+		icmd.Command("docker", "attach", containerID),
+		withStdinNewline)
+
+	result.Assert(t, icmd.Expected{ExitCode: 21})
+}
+
+func runBackgroundContainsWithExitCode(t *testing.T, exitcode int) string {
+	result := icmd.RunCmd(shell(t,
+		"docker run -d -i --rm %s sh -c 'read; exit %d'", alpineImage, exitcode))
+	result.Assert(t, icmd.Success)
+	return strings.TrimSpace(result.Stdout())
+}
+
+func withStdinNewline(cmd *icmd.Cmd) {
+	cmd.Stdin = strings.NewReader("\n")
+}