From 13364cd431578abb5d233f0279337c0b122071ed Mon Sep 17 00:00:00 2001 From: Shijiang Wei Date: Sun, 11 Oct 2015 23:39:44 +0800 Subject: [PATCH 1/2] fix the flaws in the test of the attach API Signed-off-by: Shijiang Wei --- integration-cli/docker_api_attach_test.go | 188 ++++++++-------------- 1 file changed, 71 insertions(+), 117 deletions(-) diff --git a/integration-cli/docker_api_attach_test.go b/integration-cli/docker_api_attach_test.go index d2ca4ba29d..283a0b53eb 100644 --- a/integration-cli/docker_api_attach_test.go +++ b/integration-cli/docker_api_attach_test.go @@ -1,10 +1,11 @@ package main import ( + "bufio" "bytes" "io" + "net" "net/http" - "net/http/httputil" "strings" "time" @@ -99,128 +100,81 @@ func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *check.C) { func (s *DockerSuite) TestPostContainersAttach(c *check.C) { testRequires(c, DaemonIsLinux) - out, _ := dockerCmd(c, "run", "-dit", "busybox", "cat") - r, w := io.Pipe() - defer r.Close() - defer w.Close() - - conn, err := sockConn(time.Duration(10 * time.Second)) - c.Assert(err, check.IsNil) - - containerID := strings.TrimSpace(out) - - req, err := http.NewRequest("POST", "/containers/"+containerID+"/attach?stream=1&stdin=1&stdout=1&stderr=1", bytes.NewReader([]byte{})) - c.Assert(err, check.IsNil) - - client := httputil.NewClientConn(conn, nil) - defer client.Close() - - // Do POST attach request - resp, err := client.Do(req) - c.Assert(resp.StatusCode, check.Equals, http.StatusOK) - // If we check the err, we get a ErrPersistEOF = &http.ProtocolError{ErrorString: "persistent connection closed"} - // This means that the remote requested this be the last request serviced, is this okay? - - // Test read and write to the attached container - expected := []byte("hello") - actual := make([]byte, len(expected)) - - outChan := make(chan error) - go func() { - _, err := r.Read(actual) - outChan <- err - close(outChan) - }() - - inChan := make(chan error) - go func() { - _, err := w.Write(expected) - inChan <- err - close(inChan) - }() - - select { - case err := <-inChan: + expectSuccess := func(conn net.Conn, br *bufio.Reader, stream string, tty bool) { + defer conn.Close() + expected := []byte("success") + _, err := conn.Write(expected) c.Assert(err, check.IsNil) - case <-time.After(5 * time.Second): - c.Fatal("Timeout writing to stdout") - } - select { - case err := <-outChan: + conn.SetReadDeadline(time.Now().Add(time.Second)) + lenHeader := 0 + if !tty { + lenHeader = 8 + } + actual := make([]byte, len(expected)+lenHeader) + _, err = io.ReadFull(br, actual) c.Assert(err, check.IsNil) - case <-time.After(5 * time.Second): - c.Fatal("Timeout reading from stdin") + if !tty { + fdMap := map[string]byte{ + "stdin": 0, + "stdout": 1, + "stderr": 2, + } + c.Assert(actual[0], check.Equals, fdMap[stream]) + } + c.Assert(actual[lenHeader:], check.DeepEquals, expected, check.Commentf("Attach didn't return the expected data from %s", stream)) } - if !bytes.Equal(expected, actual) { - c.Fatal("Expected output to match input") + expectTimeout := func(conn net.Conn, br *bufio.Reader, stream string) { + defer conn.Close() + _, err := conn.Write([]byte{'t'}) + c.Assert(err, check.IsNil) + + conn.SetReadDeadline(time.Now().Add(time.Second)) + actual := make([]byte, 1) + _, err = io.ReadFull(br, actual) + opErr, ok := err.(*net.OpError) + c.Assert(ok, check.Equals, true, check.Commentf("Error is expected to be *net.OpError, got %v", err)) + c.Assert(opErr.Timeout(), check.Equals, true, check.Commentf("Read from %s is expected to timeout", stream)) } - resp.Body.Close() -} - -func (s *DockerSuite) TestPostContainersAttachStderr(c *check.C) { - testRequires(c, DaemonIsLinux) - out, _ := dockerCmd(c, "run", "-dit", "busybox", "/bin/sh", "-c", "cat >&2") - - r, w := io.Pipe() - defer r.Close() - defer w.Close() - - conn, err := sockConn(time.Duration(10 * time.Second)) - c.Assert(err, check.IsNil) - - containerID := strings.TrimSpace(out) - - req, err := http.NewRequest("POST", "/containers/"+containerID+"/attach?stream=1&stdin=1&stdout=1&stderr=1", bytes.NewReader([]byte{})) - c.Assert(err, check.IsNil) - - client := httputil.NewClientConn(conn, nil) - defer client.Close() - - // Do POST attach request - resp, err := client.Do(req) - c.Assert(resp.StatusCode, check.Equals, http.StatusOK) - // If we check the err, we get a ErrPersistEOF = &http.ProtocolError{ErrorString: "persistent connection closed"} - // This means that the remote requested this be the last request serviced, is this okay? - - // Test read and write to the attached container - expected := []byte("hello") - actual := make([]byte, len(expected)) - - outChan := make(chan error) - go func() { - _, err := r.Read(actual) - outChan <- err - close(outChan) - }() - - inChan := make(chan error) - go func() { - _, err := w.Write(expected) - inChan <- err - close(inChan) - }() - - select { - case err := <-inChan: - c.Assert(err, check.IsNil) - case <-time.After(5 * time.Second): - c.Fatal("Timeout writing to stdout") - } - - select { - case err := <-outChan: - c.Assert(err, check.IsNil) - case <-time.After(5 * time.Second): - c.Fatal("Timeout reading from stdin") - } - - if !bytes.Equal(expected, actual) { - c.Fatal("Expected output to match input") - } - - resp.Body.Close() + // Create a container that only emits stdout. + cid, _ := dockerCmd(c, "run", "-di", "busybox", "cat") + cid = strings.TrimSpace(cid) + // Attach to the container's stdout stream. + conn, br, err := sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain") + c.Assert(err, check.IsNil) + // Check if the data from stdout can be received. + expectSuccess(conn, br, "stdout", false) + // Attach to the container's stderr stream. + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain") + c.Assert(err, check.IsNil) + // Since the container only emits stdout, attaching to stderr should return nothing. + expectTimeout(conn, br, "stdout") + + // Test the simlar functions of the stderr stream. + cid, _ = dockerCmd(c, "run", "-di", "busybox", "/bin/sh", "-c", "cat >&2") + cid = strings.TrimSpace(cid) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain") + c.Assert(err, check.IsNil) + expectSuccess(conn, br, "stderr", false) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain") + c.Assert(err, check.IsNil) + expectTimeout(conn, br, "stderr") + + // Test with tty. + cid, _ = dockerCmd(c, "run", "-dit", "busybox", "/bin/sh", "-c", "cat >&2") + cid = strings.TrimSpace(cid) + // Attach to stdout only. + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain") + c.Assert(err, check.IsNil) + expectSuccess(conn, br, "stdout", true) + + // Attach without stdout stream. + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain") + c.Assert(err, check.IsNil) + // Nothing should be received because both the stdout and stderr of the container will be + // sent to the client as stdout when tty is enabled. + expectTimeout(conn, br, "stdout") } From 4f6f46a11d9a8588d1e21f5cb543bb9c7181fec3 Mon Sep 17 00:00:00 2001 From: Shijiang Wei Date: Thu, 15 Oct 2015 15:22:38 +0800 Subject: [PATCH 2/2] refacter attach API tests to use checkers Signed-off-by: Shijiang Wei --- integration-cli/docker_api_attach_test.go | 68 +++++++++-------------- 1 file changed, 26 insertions(+), 42 deletions(-) diff --git a/integration-cli/docker_api_attach_test.go b/integration-cli/docker_api_attach_test.go index 283a0b53eb..32bf4b7f33 100644 --- a/integration-cli/docker_api_attach_test.go +++ b/integration-cli/docker_api_attach_test.go @@ -2,13 +2,13 @@ package main import ( "bufio" - "bytes" "io" "net" "net/http" "strings" "time" + "github.com/docker/docker/pkg/integration/checker" "github.com/go-check/check" "golang.org/x/net/websocket" ) @@ -18,23 +18,17 @@ func (s *DockerSuite) TestGetContainersAttachWebsocket(c *check.C) { out, _ := dockerCmd(c, "run", "-dit", "busybox", "cat") rwc, err := sockConn(time.Duration(10 * time.Second)) - if err != nil { - c.Fatal(err) - } + c.Assert(err, checker.IsNil) cleanedContainerID := strings.TrimSpace(out) config, err := websocket.NewConfig( "/containers/"+cleanedContainerID+"/attach/ws?stream=1&stdin=1&stdout=1&stderr=1", "http://localhost", ) - if err != nil { - c.Fatal(err) - } + c.Assert(err, checker.IsNil) ws, err := websocket.NewClient(config, rwc) - if err != nil { - c.Fatal(err) - } + c.Assert(err, checker.IsNil) defer ws.Close() expected := []byte("hello") @@ -56,46 +50,36 @@ func (s *DockerSuite) TestGetContainersAttachWebsocket(c *check.C) { select { case err := <-inChan: - if err != nil { - c.Fatal(err) - } + c.Assert(err, checker.IsNil) case <-time.After(5 * time.Second): c.Fatal("Timeout writing to ws") } select { case err := <-outChan: - if err != nil { - c.Fatal(err) - } + c.Assert(err, checker.IsNil) case <-time.After(5 * time.Second): c.Fatal("Timeout reading from ws") } - if !bytes.Equal(expected, actual) { - c.Fatal("Expected output on websocket to match input") - } + c.Assert(actual, checker.DeepEquals, expected, check.Commentf("Websocket didn't return the expected data")) } // regression gh14320 func (s *DockerSuite) TestPostContainersAttachContainerNotFound(c *check.C) { status, body, err := sockRequest("POST", "/containers/doesnotexist/attach", nil) - c.Assert(status, check.Equals, http.StatusNotFound) - c.Assert(err, check.IsNil) + c.Assert(status, checker.Equals, http.StatusNotFound) + c.Assert(err, checker.IsNil) expected := "no such id: doesnotexist\n" - if !strings.Contains(string(body), expected) { - c.Fatalf("Expected response body to contain %q", expected) - } + c.Assert(string(body), checker.Contains, expected) } func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *check.C) { status, body, err := sockRequest("GET", "/containers/doesnotexist/attach/ws", nil) - c.Assert(status, check.Equals, http.StatusNotFound) - c.Assert(err, check.IsNil) + c.Assert(status, checker.Equals, http.StatusNotFound) + c.Assert(err, checker.IsNil) expected := "no such id: doesnotexist\n" - if !strings.Contains(string(body), expected) { - c.Fatalf("Expected response body to contain %q", expected) - } + c.Assert(string(body), checker.Contains, expected) } func (s *DockerSuite) TestPostContainersAttach(c *check.C) { @@ -105,7 +89,7 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { defer conn.Close() expected := []byte("success") _, err := conn.Write(expected) - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) conn.SetReadDeadline(time.Now().Add(time.Second)) lenHeader := 0 @@ -114,29 +98,29 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { } actual := make([]byte, len(expected)+lenHeader) _, err = io.ReadFull(br, actual) - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) if !tty { fdMap := map[string]byte{ "stdin": 0, "stdout": 1, "stderr": 2, } - c.Assert(actual[0], check.Equals, fdMap[stream]) + c.Assert(actual[0], checker.Equals, fdMap[stream]) } - c.Assert(actual[lenHeader:], check.DeepEquals, expected, check.Commentf("Attach didn't return the expected data from %s", stream)) + c.Assert(actual[lenHeader:], checker.DeepEquals, expected, check.Commentf("Attach didn't return the expected data from %s", stream)) } expectTimeout := func(conn net.Conn, br *bufio.Reader, stream string) { defer conn.Close() _, err := conn.Write([]byte{'t'}) - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) conn.SetReadDeadline(time.Now().Add(time.Second)) actual := make([]byte, 1) _, err = io.ReadFull(br, actual) opErr, ok := err.(*net.OpError) - c.Assert(ok, check.Equals, true, check.Commentf("Error is expected to be *net.OpError, got %v", err)) - c.Assert(opErr.Timeout(), check.Equals, true, check.Commentf("Read from %s is expected to timeout", stream)) + c.Assert(ok, checker.Equals, true, check.Commentf("Error is expected to be *net.OpError, got %v", err)) + c.Assert(opErr.Timeout(), checker.Equals, true, check.Commentf("Read from %s is expected to timeout", stream)) } // Create a container that only emits stdout. @@ -144,12 +128,12 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { cid = strings.TrimSpace(cid) // Attach to the container's stdout stream. conn, br, err := sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain") - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) // Check if the data from stdout can be received. expectSuccess(conn, br, "stdout", false) // Attach to the container's stderr stream. conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain") - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) // Since the container only emits stdout, attaching to stderr should return nothing. expectTimeout(conn, br, "stdout") @@ -157,10 +141,10 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { cid, _ = dockerCmd(c, "run", "-di", "busybox", "/bin/sh", "-c", "cat >&2") cid = strings.TrimSpace(cid) conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain") - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) expectSuccess(conn, br, "stderr", false) conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain") - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) expectTimeout(conn, br, "stderr") // Test with tty. @@ -168,12 +152,12 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { cid = strings.TrimSpace(cid) // Attach to stdout only. conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain") - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) expectSuccess(conn, br, "stdout", true) // Attach without stdout stream. conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain") - c.Assert(err, check.IsNil) + c.Assert(err, checker.IsNil) // Nothing should be received because both the stdout and stderr of the container will be // sent to the client as stdout when tty is enabled. expectTimeout(conn, br, "stdout")