Merge pull request #20803 from WeiZhang555/empty-stats-no-stream

Bug fix: stats --no-stream always print zero values

api/client/stats.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"strings"
+	"sync"
 	"text/tabwriter"
 	"time"
 
@@ -59,6 +60,9 @@ func (cli *DockerCli) CmdStats(args ...string) error {
 		})
 	}
 
+	// waitFirst is a WaitGroup to wait first stat data's reach for each container
+	waitFirst := &sync.WaitGroup{}
+
 	cStats := stats{}
 	// getContainerList simulates creation event for all previously existing
 	// containers (only used when calling `docker stats` without arguments).
@@ -72,8 +76,10 @@ func (cli *DockerCli) CmdStats(args ...string) error {
 		}
 		for _, container := range cs {
 			s := &containerStats{Name: container.ID[:12]}
-			cStats.add(s)
-			go s.Collect(cli.client, !*noStream)
+			if cStats.add(s) {
+				waitFirst.Add(1)
+				go s.Collect(cli.client, !*noStream, waitFirst)
+			}
 		}
 	}
 
@@ -87,15 +93,19 @@ func (cli *DockerCli) CmdStats(args ...string) error {
 		eh.Handle("create", func(e events.Message) {
 			if *all {
 				s := &containerStats{Name: e.ID[:12]}
-				cStats.add(s)
-				go s.Collect(cli.client, !*noStream)
+				if cStats.add(s) {
+					waitFirst.Add(1)
+					go s.Collect(cli.client, !*noStream, waitFirst)
+				}
 			}
 		})
 
 		eh.Handle("start", func(e events.Message) {
 			s := &containerStats{Name: e.ID[:12]}
-			cStats.add(s)
-			go s.Collect(cli.client, !*noStream)
+			if cStats.add(s) {
+				waitFirst.Add(1)
+				go s.Collect(cli.client, !*noStream, waitFirst)
+			}
 		})
 
 		eh.Handle("die", func(e events.Message) {
@@ -112,14 +122,16 @@ func (cli *DockerCli) CmdStats(args ...string) error {
 
 		// Start a short-lived goroutine to retrieve the initial list of
 		// containers.
-		go getContainerList()
+		getContainerList()
 	} else {
 		// Artificially send creation events for the containers we were asked to
 		// monitor (same code path than we use when monitoring all containers).
 		for _, name := range names {
 			s := &containerStats{Name: name}
-			cStats.add(s)
-			go s.Collect(cli.client, !*noStream)
+			if cStats.add(s) {
+				waitFirst.Add(1)
+				go s.Collect(cli.client, !*noStream, waitFirst)
+			}
 		}
 
 		// We don't expect any asynchronous errors: closeChan can be closed.
@@ -143,6 +155,9 @@ func (cli *DockerCli) CmdStats(args ...string) error {
 		}
 	}
 
+	// before print to screen, make sure each container get at least one valid stat data
+	waitFirst.Wait()
+
 	w := tabwriter.NewWriter(cli.out, 20, 1, 3, ' ', 0)
 	printHeader := func() {
 		if !*noStream {

api/client/stats_helpers.go

@@ -33,12 +33,14 @@ type stats struct {
 	cs []*containerStats
 }
 
-func (s *stats) add(cs *containerStats) {
+func (s *stats) add(cs *containerStats) bool {
 	s.mu.Lock()
+	defer s.mu.Unlock()
 	if _, exists := s.isKnownContainer(cs.Name); !exists {
 		s.cs = append(s.cs, cs)
+		return true
 	}
-	s.mu.Unlock()
+	return false
 }
 
 func (s *stats) remove(id string) {
@@ -58,7 +60,22 @@ func (s *stats) isKnownContainer(cid string) (int, bool) {
 	return -1, false
 }
 
-func (s *containerStats) Collect(cli client.APIClient, streamStats bool) {
+func (s *containerStats) Collect(cli client.APIClient, streamStats bool, waitFirst *sync.WaitGroup) {
+	var (
+		getFirst       bool
+		previousCPU    uint64
+		previousSystem uint64
+		u              = make(chan error, 1)
+	)
+
+	defer func() {
+		// if error happens and we get nothing of stats, release wait group whatever
+		if !getFirst {
+			getFirst = true
+			waitFirst.Done()
+		}
+	}()
+
 	responseBody, err := cli.ContainerStats(context.Background(), s.Name, streamStats)
 	if err != nil {
 		s.mu.Lock()
@@ -68,12 +85,7 @@ func (s *containerStats) Collect(cli client.APIClient, streamStats bool) {
 	}
 	defer responseBody.Close()
 
-	var (
-		previousCPU    uint64
-		previousSystem uint64
-		dec            = json.NewDecoder(responseBody)
-		u              = make(chan error, 1)
-	)
+	dec := json.NewDecoder(responseBody)
 	go func() {
 		for {
 			var v *types.StatsJSON
@@ -125,6 +137,11 @@ func (s *containerStats) Collect(cli client.APIClient, streamStats bool) {
 			s.BlockRead = 0
 			s.BlockWrite = 0
 			s.mu.Unlock()
+			// if this is the first stat you get, release WaitGroup
+			if !getFirst {
+				getFirst = true
+				waitFirst.Done()
+			}
 		case err := <-u:
 			if err != nil {
 				s.mu.Lock()
@@ -132,6 +149,11 @@ func (s *containerStats) Collect(cli client.APIClient, streamStats bool) {
 				s.mu.Unlock()
 				return
 			}
+			// if this is the first stat you get, release WaitGroup
+			if !getFirst {
+				getFirst = true
+				waitFirst.Done()
+			}
 		}
 		if !streamStats {
 			return

integration-cli/docker_cli_stats_test.go

@@ -75,6 +75,18 @@ func (s *DockerSuite) TestStatsAllRunningNoStream(c *check.C) {
 	if strings.Contains(out, id3) {
 		c.Fatalf("Did not expect %s in stats, got %s", id3, out)
 	}
+
+	// check output contains real data, but not all zeros
+	reg, _ := regexp.Compile("[1-9]+")
+	// split output with "\n", outLines[1] is id2's output
+	// outLines[2] is id1's output
+	outLines := strings.Split(out, "\n")
+	// check stat result of id2 contains real data
+	realData := reg.Find([]byte(outLines[1][12:]))
+	c.Assert(realData, checker.NotNil, check.Commentf("stat result are empty: %s", out))
+	// check stat result of id1 contains real data
+	realData = reg.Find([]byte(outLines[2][12:]))
+	c.Assert(realData, checker.NotNil, check.Commentf("stat result are empty: %s", out))
 }
 
 func (s *DockerSuite) TestStatsAllNoStream(c *check.C) {
@@ -93,6 +105,17 @@ func (s *DockerSuite) TestStatsAllNoStream(c *check.C) {
 	if !strings.Contains(out, id1) || !strings.Contains(out, id2) {
 		c.Fatalf("Expected stats output to contain both %s and %s, got %s", id1, id2, out)
 	}
+
+	// check output contains real data, but not all zeros
+	reg, _ := regexp.Compile("[1-9]+")
+	// split output with "\n", outLines[1] is id2's output
+	outLines := strings.Split(out, "\n")
+	// check stat result of id2 contains real data
+	realData := reg.Find([]byte(outLines[1][12:]))
+	c.Assert(realData, checker.NotNil, check.Commentf("stat result of %s is empty: %s", id2, out))
+	// check stat result of id1 contains all zero
+	realData = reg.Find([]byte(outLines[2][12:]))
+	c.Assert(realData, checker.IsNil, check.Commentf("stat result of %s should be empty : %s", id1, out))
 }
 
 func (s *DockerSuite) TestStatsAllNewContainersAdded(c *check.C) {