diff --git a/api/server/form.go b/api/server/form.go index 75584df065..230d875366 100644 --- a/api/server/form.go +++ b/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 { diff --git a/api/server/form_test.go b/api/server/form_test.go index caa9f1757c..5b3bd718d2 100644 --- a/api/server/form_test.go +++ b/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, diff --git a/api/server/server.go b/api/server/server.go index 1fa629acaa..7bf770f44d 100644 --- a/api/server/server.go +++ b/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 { diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index daec87c497..e95ac43e34 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/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 (