Merge pull request #13499 from cpuguy83/fix_stats_unsubscribe

Fix unregister stats on when rm running container

daemon/delete.go

@@ -58,10 +58,6 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error
 
 // Destroy unregisters a container from the daemon and cleanly removes its contents from the filesystem.
 func (daemon *Daemon) rm(container *Container, forceRemove bool) (err error) {
-	// stop collection of stats for the container regardless
-	// if stats are currently getting collected.
-	daemon.statsCollector.stopCollection(container)
-
 	if container.IsRunning() {
 		if !forceRemove {
 			return fmt.Errorf("Conflict, You cannot remove a running container. Stop the container before attempting removal or use -f")
@@ -71,6 +67,10 @@ func (daemon *Daemon) rm(container *Container, forceRemove bool) (err error) {
 		}
 	}
 
+	// stop collection of stats for the container regardless
+	// if stats are currently getting collected.
+	daemon.statsCollector.stopCollection(container)
+
 	element := daemon.containers.Get(container.ID)
 	if element == nil {
 		return fmt.Errorf("Container %v not found - maybe it was already destroyed?", container.ID)

integration-cli/docker_api_containers_test.go

@@ -254,6 +254,42 @@ func (s *DockerSuite) TestGetContainerStats(c *check.C) {
 	}
 }
 
+func (s *DockerSuite) TestContainerStatsRmRunning(c *check.C) {
+	out, _ := dockerCmd(c, "run", "-d", "busybox", "top")
+	id := strings.TrimSpace(out)
+
+	buf := &channelBuffer{make(chan []byte, 1)}
+	defer buf.Close()
+	chErr := make(chan error)
+	go func() {
+		_, body, err := sockRequestRaw("GET", "/containers/"+id+"/stats?stream=1", nil, "application/json")
+		if err != nil {
+			chErr <- err
+		}
+		defer body.Close()
+		_, err = io.Copy(buf, body)
+		chErr <- err
+	}()
+	defer func() {
+		c.Assert(<-chErr, check.IsNil)
+	}()
+
+	b := make([]byte, 32)
+	// make sure we've got some stats
+	_, err := buf.ReadTimeout(b, 2*time.Second)
+	c.Assert(err, check.IsNil)
+
+	// Now remove without `-f` and make sure we are still pulling stats
+	_, err = runCommand(exec.Command(dockerBinary, "rm", id))
+	c.Assert(err, check.Not(check.IsNil), check.Commentf("rm should have failed but didn't"))
+	_, err = buf.ReadTimeout(b, 2*time.Second)
+	c.Assert(err, check.IsNil)
+	dockerCmd(c, "rm", "-f", id)
+
+	_, err = buf.ReadTimeout(b, 2*time.Second)
+	c.Assert(err, check.Not(check.IsNil))
+}
+
 func (s *DockerSuite) TestGetStoppedContainerStats(c *check.C) {
 	// TODO: this test does nothing because we are c.Assert'ing in goroutine
 	var (

integration-cli/utils.go

@@ -316,3 +316,26 @@ func parseCgroupPaths(procCgroupData string) map[string]string {
 	}
 	return cgroupPaths
 }
+
+type channelBuffer struct {
+	c chan []byte
+}
+
+func (c *channelBuffer) Write(b []byte) (int, error) {
+	c.c <- b
+	return len(b), nil
+}
+
+func (c *channelBuffer) Close() error {
+	close(c.c)
+	return nil
+}
+
+func (c *channelBuffer) ReadTimeout(p []byte, n time.Duration) (int, error) {
+	select {
+	case b := <-c.c:
+		return copy(p[0:], b), nil
+	case <-time.After(n):
+		return -1, fmt.Errorf("timeout reading from channel")
+	}
+}