From ec97f414657189f2ce95d6b5bda26aa9d206b705 Mon Sep 17 00:00:00 2001
From: Antonio Murdaca <me@runcom.ninja>
Date: Sat, 23 May 2015 16:09:39 +0200
Subject: [PATCH] Fix regression in stats API endpoint where stream query param
default is true
Signed-off-by: Antonio Murdaca <me@runcom.ninja>
---
api/server/form.go | 9 ++
api/server/form_test.go | 15 ++++
api/server/server.go | 2 +-
integration-cli/docker_api_containers_test.go | 85 ++++++++++++++++++-
4 files changed, 109 insertions(+), 2 deletions(-)
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 (