From 66661761d58d444edcc9c29e643fc7d5ce1abb61 Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Wed, 15 Nov 2017 19:18:23 -0800 Subject: [PATCH 1/2] attach: Use ContainerWait instead of Inspect Signed-off-by: Kenfe-Mickael Laventure --- cli/command/container/attach.go | 40 ++++++++++++---------- cli/command/container/attach_test.go | 51 +++++++++++++++++----------- e2e/container/attach_test.go | 17 ++++++++++ 3 files changed, 71 insertions(+), 37 deletions(-) create mode 100644 e2e/container/attach_test.go diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go index 6e4628d95d..dca84d5ddb 100644 --- a/cli/command/container/attach.go +++ b/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 -} diff --git a/cli/command/container/attach_test.go b/cli/command/container/attach_test.go index 1ca775c6d5..1c305aa7a1 100644 --- a/cli/command/container/attach_test.go +++ b/cli/command/container/attach_test.go @@ -1,6 +1,7 @@ package container import ( + "fmt" "io/ioutil" "testing" @@ -8,9 +9,9 @@ import ( "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, + }, }, { - inspectError: expecatedErr, - expectedError: expecatedErr, + err: expectedErr, + expectedError: expectedErr, }, { - exitCode: 15, + result: &container.ContainerWaitOKBody{ + Error: &container.ContainerWaitOKBodyError{ + expectedErr.Error(), + }, + }, + expectedError: expectedErr, + }, + { + 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 } - err := getExitStatus(context.Background(), client, containerID) + if testcase.result != nil { + resultC <- *testcase.result + } + err := getExitStatus(errC, resultC) assert.Equal(t, testcase.expectedError, err) } } diff --git a/e2e/container/attach_test.go b/e2e/container/attach_test.go new file mode 100644 index 0000000000..a2b6188b50 --- /dev/null +++ b/e2e/container/attach_test.go @@ -0,0 +1,17 @@ +package container + +import ( + "testing" + + "github.com/gotestyourself/gotestyourself/icmd" +) + +func TestAttachExitCode(t *testing.T) { + cName := "test-attach-exit-code" + icmd.RunCommand("docker", "run", "-d", "--rm", "--name", cName, + alpineImage, "sh", "-c", "sleep 5 ; exit 21").Assert(t, icmd.Success) + cmd := icmd.Command("docker", "wait", cName) + res := icmd.StartCmd(cmd) + icmd.RunCommand("docker", "attach", cName).Assert(t, icmd.Expected{ExitCode: 21}) + icmd.WaitOnCmd(8, res).Assert(t, icmd.Expected{ExitCode: 0, Out: "21"}) +} From bace33d7a8486371e8a595bd21eb3e4b43db1b23 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 17 Nov 2017 12:45:27 -0500 Subject: [PATCH 2/2] Fix TestAttachExitCode Signed-off-by: Daniel Nephin --- e2e/container/attach_test.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/e2e/container/attach_test.go b/e2e/container/attach_test.go index a2b6188b50..d26cae90ff 100644 --- a/e2e/container/attach_test.go +++ b/e2e/container/attach_test.go @@ -1,17 +1,29 @@ package container import ( + "strings" "testing" "github.com/gotestyourself/gotestyourself/icmd" ) func TestAttachExitCode(t *testing.T) { - cName := "test-attach-exit-code" - icmd.RunCommand("docker", "run", "-d", "--rm", "--name", cName, - alpineImage, "sh", "-c", "sleep 5 ; exit 21").Assert(t, icmd.Success) - cmd := icmd.Command("docker", "wait", cName) - res := icmd.StartCmd(cmd) - icmd.RunCommand("docker", "attach", cName).Assert(t, icmd.Expected{ExitCode: 21}) - icmd.WaitOnCmd(8, res).Assert(t, icmd.Expected{ExitCode: 0, Out: "21"}) + 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") }