Browse code
Fix regression in stats API endpoint where stream query param default is true
Signed-off-by: Antonio Murdaca <me@runcom.ninja>
Antonio Murdaca authored on 2015/05/23 23:09:39Showing 4 changed files
- api/server/form.go
- api/server/form_test.go
- api/server/server.go
- integration-cli/docker_api_containers_test.go
| ... | ... |
@@ -11,6 +11,15 @@ func boolValue(r *http.Request, k string) bool {
|
| 11 | 11 |
return !(s == "" || s == "0" || s == "no" || s == "false" || s == "none") |
| 12 | 12 |
} |
| 13 | 13 |
|
| 14 |
+// boolValueOrDefault returns the default bool passed if the query param is |
|
| 15 |
+// missing, otherwise it's just a proxy to boolValue above |
|
| 16 |
+func boolValueOrDefault(r *http.Request, k string, d bool) bool {
|
|
| 17 |
+ if _, ok := r.Form[k]; !ok {
|
|
| 18 |
+ return d |
|
| 19 |
+ } |
|
| 20 |
+ return boolValue(r, k) |
|
| 21 |
+} |
|
| 22 |
+ |
|
| 14 | 23 |
func int64ValueOrZero(r *http.Request, k string) int64 {
|
| 15 | 24 |
val, err := strconv.ParseInt(r.FormValue(k), 10, 64) |
| 16 | 25 |
if err != nil {
|
| ... | ... |
@@ -33,6 +33,21 @@ func TestBoolValue(t *testing.T) {
|
| 33 | 33 |
} |
| 34 | 34 |
} |
| 35 | 35 |
|
| 36 |
+func TestBoolValueOrDefault(t *testing.T) {
|
|
| 37 |
+ r, _ := http.NewRequest("GET", "", nil)
|
|
| 38 |
+ if !boolValueOrDefault(r, "queryparam", true) {
|
|
| 39 |
+ t.Fatal("Expected to get true default value, got false")
|
|
| 40 |
+ } |
|
| 41 |
+ |
|
| 42 |
+ v := url.Values{}
|
|
| 43 |
+ v.Set("param", "")
|
|
| 44 |
+ r, _ = http.NewRequest("GET", "", nil)
|
|
| 45 |
+ r.Form = v |
|
| 46 |
+ if boolValueOrDefault(r, "param", true) {
|
|
| 47 |
+ t.Fatal("Expected not to get true")
|
|
| 48 |
+ } |
|
| 49 |
+} |
|
| 50 |
+ |
|
| 36 | 51 |
func TestInt64ValueOrZero(t *testing.T) {
|
| 37 | 52 |
cases := map[string]int64{
|
| 38 | 53 |
"": 0, |
| ... | ... |
@@ -551,7 +551,7 @@ func (s *Server) getContainersStats(version version.Version, w http.ResponseWrit |
| 551 | 551 |
return fmt.Errorf("Missing parameter")
|
| 552 | 552 |
} |
| 553 | 553 |
|
| 554 |
- return s.daemon.ContainerStats(vars["name"], boolValue(r, "stream"), ioutils.NewWriteFlusher(w)) |
|
| 554 |
+ return s.daemon.ContainerStats(vars["name"], boolValueOrDefault(r, "stream", true), ioutils.NewWriteFlusher(w)) |
|
| 555 | 555 |
} |
| 556 | 556 |
|
| 557 | 557 |
func (s *Server) getContainersLogs(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
|
| ... | ... |
@@ -254,7 +254,7 @@ func (s *DockerSuite) TestGetContainerStats(c *check.C) {
|
| 254 | 254 |
} |
| 255 | 255 |
} |
| 256 | 256 |
|
| 257 |
-func (s *DockerSuite) TestContainerStatsRmRunning(c *check.C) {
|
|
| 257 |
+func (s *DockerSuite) TestGetContainerStatsRmRunning(c *check.C) {
|
|
| 258 | 258 |
out, _ := dockerCmd(c, "run", "-d", "busybox", "top") |
| 259 | 259 |
id := strings.TrimSpace(out) |
| 260 | 260 |
|
| ... | ... |
@@ -290,6 +290,89 @@ func (s *DockerSuite) TestContainerStatsRmRunning(c *check.C) {
|
| 290 | 290 |
c.Assert(err, check.Not(check.IsNil)) |
| 291 | 291 |
} |
| 292 | 292 |
|
| 293 |
+// regression test for gh13421 |
|
| 294 |
+// previous test was just checking one stat entry so it didn't fail (stats with |
|
| 295 |
+// stream false always return one stat) |
|
| 296 |
+func (s *DockerSuite) TestGetContainerStatsStream(c *check.C) {
|
|
| 297 |
+ name := "statscontainer" |
|
| 298 |
+ runCmd := exec.Command(dockerBinary, "run", "-d", "--name", name, "busybox", "top") |
|
| 299 |
+ _, err := runCommand(runCmd) |
|
| 300 |
+ c.Assert(err, check.IsNil) |
|
| 301 |
+ |
|
| 302 |
+ type b struct {
|
|
| 303 |
+ status int |
|
| 304 |
+ body []byte |
|
| 305 |
+ err error |
|
| 306 |
+ } |
|
| 307 |
+ bc := make(chan b, 1) |
|
| 308 |
+ go func() {
|
|
| 309 |
+ status, body, err := sockRequest("GET", "/containers/"+name+"/stats", nil)
|
|
| 310 |
+ bc <- b{status, body, err}
|
|
| 311 |
+ }() |
|
| 312 |
+ |
|
| 313 |
+ // allow some time to stream the stats from the container |
|
| 314 |
+ time.Sleep(4 * time.Second) |
|
| 315 |
+ if _, err := runCommand(exec.Command(dockerBinary, "rm", "-f", name)); err != nil {
|
|
| 316 |
+ c.Fatal(err) |
|
| 317 |
+ } |
|
| 318 |
+ |
|
| 319 |
+ // collect the results from the stats stream or timeout and fail |
|
| 320 |
+ // if the stream was not disconnected. |
|
| 321 |
+ select {
|
|
| 322 |
+ case <-time.After(2 * time.Second): |
|
| 323 |
+ c.Fatal("stream was not closed after container was removed")
|
|
| 324 |
+ case sr := <-bc: |
|
| 325 |
+ c.Assert(sr.err, check.IsNil) |
|
| 326 |
+ c.Assert(sr.status, check.Equals, http.StatusOK) |
|
| 327 |
+ |
|
| 328 |
+ s := string(sr.body) |
|
| 329 |
+ // count occurrences of "read" of types.Stats |
|
| 330 |
+ if l := strings.Count(s, "read"); l < 2 {
|
|
| 331 |
+ c.Fatalf("Expected more than one stat streamed, got %d", l)
|
|
| 332 |
+ } |
|
| 333 |
+ } |
|
| 334 |
+} |
|
| 335 |
+ |
|
| 336 |
+func (s *DockerSuite) TestGetContainerStatsNoStream(c *check.C) {
|
|
| 337 |
+ name := "statscontainer" |
|
| 338 |
+ runCmd := exec.Command(dockerBinary, "run", "-d", "--name", name, "busybox", "top") |
|
| 339 |
+ _, err := runCommand(runCmd) |
|
| 340 |
+ c.Assert(err, check.IsNil) |
|
| 341 |
+ |
|
| 342 |
+ type b struct {
|
|
| 343 |
+ status int |
|
| 344 |
+ body []byte |
|
| 345 |
+ err error |
|
| 346 |
+ } |
|
| 347 |
+ bc := make(chan b, 1) |
|
| 348 |
+ go func() {
|
|
| 349 |
+ status, body, err := sockRequest("GET", "/containers/"+name+"/stats?stream=0", nil)
|
|
| 350 |
+ bc <- b{status, body, err}
|
|
| 351 |
+ }() |
|
| 352 |
+ |
|
| 353 |
+ // allow some time to stream the stats from the container |
|
| 354 |
+ time.Sleep(4 * time.Second) |
|
| 355 |
+ if _, err := runCommand(exec.Command(dockerBinary, "rm", "-f", name)); err != nil {
|
|
| 356 |
+ c.Fatal(err) |
|
| 357 |
+ } |
|
| 358 |
+ |
|
| 359 |
+ // collect the results from the stats stream or timeout and fail |
|
| 360 |
+ // if the stream was not disconnected. |
|
| 361 |
+ select {
|
|
| 362 |
+ case <-time.After(2 * time.Second): |
|
| 363 |
+ c.Fatal("stream was not closed after container was removed")
|
|
| 364 |
+ case sr := <-bc: |
|
| 365 |
+ c.Assert(sr.err, check.IsNil) |
|
| 366 |
+ c.Assert(sr.status, check.Equals, http.StatusOK) |
|
| 367 |
+ |
|
| 368 |
+ s := string(sr.body) |
|
| 369 |
+ // count occurrences of "read" of types.Stats |
|
| 370 |
+ if l := strings.Count(s, "read"); l != 1 {
|
|
| 371 |
+ c.Fatalf("Expected only one stat streamed, got %d", l)
|
|
| 372 |
+ } |
|
| 373 |
+ } |
|
| 374 |
+} |
|
| 375 |
+ |
|
| 293 | 376 |
func (s *DockerSuite) TestGetStoppedContainerStats(c *check.C) {
|
| 294 | 377 |
// TODO: this test does nothing because we are c.Assert'ing in goroutine |
| 295 | 378 |
var ( |