Merge pull request #13425 from runcom/13421-stats-default-stream

Fix regression in stats API endpoint

api/server/form.go

@@ -11,6 +11,15 @@ func boolValue(r *http.Request, k string) bool {
 	return !(s == "" || s == "0" || s == "no" || s == "false" || s == "none")
 }
 
+// boolValueOrDefault returns the default bool passed if the query param is
+// missing, otherwise it's just a proxy to boolValue above
+func boolValueOrDefault(r *http.Request, k string, d bool) bool {
+	if _, ok := r.Form[k]; !ok {
+		return d
+	}
+	return boolValue(r, k)
+}
+
 func int64ValueOrZero(r *http.Request, k string) int64 {
 	val, err := strconv.ParseInt(r.FormValue(k), 10, 64)
 	if err != nil {

api/server/form_test.go

@@ -33,6 +33,21 @@ func TestBoolValue(t *testing.T) {
 	}
 }
 
+func TestBoolValueOrDefault(t *testing.T) {
+	r, _ := http.NewRequest("GET", "", nil)
+	if !boolValueOrDefault(r, "queryparam", true) {
+		t.Fatal("Expected to get true default value, got false")
+	}
+
+	v := url.Values{}
+	v.Set("param", "")
+	r, _ = http.NewRequest("GET", "", nil)
+	r.Form = v
+	if boolValueOrDefault(r, "param", true) {
+		t.Fatal("Expected not to get true")
+	}
+}
+
 func TestInt64ValueOrZero(t *testing.T) {
 	cases := map[string]int64{
 		"":     0,

api/server/server.go

@@ -551,7 +551,7 @@ func (s *Server) getContainersStats(version version.Version, w http.ResponseWrit
 		return fmt.Errorf("Missing parameter")
 	}
 
-	return s.daemon.ContainerStats(vars["name"], boolValue(r, "stream"), ioutils.NewWriteFlusher(w))
+	return s.daemon.ContainerStats(vars["name"], boolValueOrDefault(r, "stream", true), ioutils.NewWriteFlusher(w))
 }
 
 func (s *Server) getContainersLogs(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {

integration-cli/docker_api_containers_test.go

@@ -254,7 +254,7 @@ func (s *DockerSuite) TestGetContainerStats(c *check.C) {
 	}
 }
 
-func (s *DockerSuite) TestContainerStatsRmRunning(c *check.C) {
+func (s *DockerSuite) TestGetContainerStatsRmRunning(c *check.C) {
 	out, _ := dockerCmd(c, "run", "-d", "busybox", "top")
 	id := strings.TrimSpace(out)
 
@@ -290,6 +290,89 @@ func (s *DockerSuite) TestContainerStatsRmRunning(c *check.C) {
 	c.Assert(err, check.Not(check.IsNil))
 }
 
+// regression test for gh13421
+// previous test was just checking one stat entry so it didn't fail (stats with
+// stream false always return one stat)
+func (s *DockerSuite) TestGetContainerStatsStream(c *check.C) {
+	name := "statscontainer"
+	runCmd := exec.Command(dockerBinary, "run", "-d", "--name", name, "busybox", "top")
+	_, err := runCommand(runCmd)
+	c.Assert(err, check.IsNil)
+
+	type b struct {
+		status int
+		body   []byte
+		err    error
+	}
+	bc := make(chan b, 1)
+	go func() {
+		status, body, err := sockRequest("GET", "/containers/"+name+"/stats", nil)
+		bc <- b{status, body, err}
+	}()
+
+	// allow some time to stream the stats from the container
+	time.Sleep(4 * time.Second)
+	if _, err := runCommand(exec.Command(dockerBinary, "rm", "-f", name)); err != nil {
+		c.Fatal(err)
+	}
+
+	// collect the results from the stats stream or timeout and fail
+	// if the stream was not disconnected.
+	select {
+	case <-time.After(2 * time.Second):
+		c.Fatal("stream was not closed after container was removed")
+	case sr := <-bc:
+		c.Assert(sr.err, check.IsNil)
+		c.Assert(sr.status, check.Equals, http.StatusOK)
+
+		s := string(sr.body)
+		// count occurrences of "read" of types.Stats
+		if l := strings.Count(s, "read"); l < 2 {
+			c.Fatalf("Expected more than one stat streamed, got %d", l)
+		}
+	}
+}
+
+func (s *DockerSuite) TestGetContainerStatsNoStream(c *check.C) {
+	name := "statscontainer"
+	runCmd := exec.Command(dockerBinary, "run", "-d", "--name", name, "busybox", "top")
+	_, err := runCommand(runCmd)
+	c.Assert(err, check.IsNil)
+
+	type b struct {
+		status int
+		body   []byte
+		err    error
+	}
+	bc := make(chan b, 1)
+	go func() {
+		status, body, err := sockRequest("GET", "/containers/"+name+"/stats?stream=0", nil)
+		bc <- b{status, body, err}
+	}()
+
+	// allow some time to stream the stats from the container
+	time.Sleep(4 * time.Second)
+	if _, err := runCommand(exec.Command(dockerBinary, "rm", "-f", name)); err != nil {
+		c.Fatal(err)
+	}
+
+	// collect the results from the stats stream or timeout and fail
+	// if the stream was not disconnected.
+	select {
+	case <-time.After(2 * time.Second):
+		c.Fatal("stream was not closed after container was removed")
+	case sr := <-bc:
+		c.Assert(sr.err, check.IsNil)
+		c.Assert(sr.status, check.Equals, http.StatusOK)
+
+		s := string(sr.body)
+		// count occurrences of "read" of types.Stats
+		if l := strings.Count(s, "read"); l != 1 {
+			c.Fatalf("Expected only one stat streamed, got %d", l)
+		}
+	}
+}
+
 func (s *DockerSuite) TestGetStoppedContainerStats(c *check.C) {
 	// TODO: this test does nothing because we are c.Assert'ing in goroutine
 	var (