fix: allow to stats not-running container

just return a empty stats data for them

Signed-off-by: zhuangqh <zhuangqhc@gmail.com>

cli/stats_helpers.go

@@ -170,11 +170,11 @@ func collect(ctx context.Context, s *StatsEntryWithLock, cli client.CommonAPICli
 			}
 
 			// first time PrecpuStats will be nil
-			if v.PrecpuStats != nil {
+			if v.PrecpuStats != nil && v.PrecpuStats.CPUUsage != nil {
 				previousCPU = v.PrecpuStats.CPUUsage.TotalUsage
 				previousSystem = v.PrecpuStats.SyetemCPUUsage
 			}
-			cpuPercent = calculateCPUPercentUnix(previousCPU, previousSystem, v)
+			cpuPercent = calculateCPUPercentUnix(previousCPU, previousSystem, v.CPUStats)
 			blkRead, blkWrite = calculateBlockIO(v.BlkioStats)
 			mem = calculateMemUsageUnixNoCache(v.MemoryStats)
 			memLimit = float64(v.MemoryStats.Limit)
@@ -228,18 +228,22 @@ func collect(ctx context.Context, s *StatsEntryWithLock, cli client.CommonAPICli
 	}
 }
 
-func calculateCPUPercentUnix(previousCPU, previousSystem uint64, v *types.ContainerStats) float64 {
+func calculateCPUPercentUnix(previousCPU, previousSystem uint64, cpuStats *types.CPUStats) float64 {
+	if cpuStats == nil || cpuStats.CPUUsage == nil {
+		return 0.0
+	}
+
 	var (
 		cpuPercent = 0.0
 		// calculate the change for the cpu usage of the container in between readings
-		cpuDelta = float64(v.CPUStats.CPUUsage.TotalUsage) - float64(previousCPU)
+		cpuDelta = float64(cpuStats.CPUUsage.TotalUsage) - float64(previousCPU)
 		// calculate the change for the entire system between readings
-		systemDelta = float64(v.CPUStats.SyetemCPUUsage) - float64(previousSystem)
-		onlineCPUs  = float64(v.CPUStats.OnlineCpus)
+		systemDelta = float64(cpuStats.SyetemCPUUsage) - float64(previousSystem)
+		onlineCPUs  = float64(cpuStats.OnlineCpus)
 	)
 
 	if onlineCPUs == 0.0 {
-		onlineCPUs = float64(len(v.CPUStats.CPUUsage.PercpuUsage))
+		onlineCPUs = float64(len(cpuStats.CPUUsage.PercpuUsage))
 	}
 	if systemDelta > 0.0 && cpuDelta > 0.0 {
 		cpuPercent = (cpuDelta / systemDelta) * onlineCPUs * 100.0
@@ -273,6 +277,9 @@ func calculateNetwork(network map[string]types.NetworkStats) (float64, float64)
 // calculateMemUsageUnixNoCache calculate memory usage of the container.
 // Page cache is intentionally excluded to avoid misinterpretation of the output.
 func calculateMemUsageUnixNoCache(mem *types.MemoryStats) float64 {
+	if mem == nil || len(mem.Stats) == 0 {
+		return 0.0
+	}
 	return float64(mem.Usage - mem.Stats["cache"])
 }
 

cri/v1alpha2/cri_utils.go

@@ -26,8 +26,6 @@ import (
 	"github.com/alibaba/pouch/pkg/randomid"
 	"github.com/alibaba/pouch/pkg/utils"
 
-	"github.com/containerd/cgroups"
-	"github.com/containerd/typeurl"
 	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/go-openapi/strfmt"
 	"github.com/sirupsen/logrus"
@@ -1070,26 +1068,21 @@ func (c *CriManager) getContainerMetrics(ctx context.Context, meta *mgr.Containe
 		Annotations: annotations,
 	}
 
-	stats, _, err := c.ContainerMgr.Stats(ctx, meta.ID)
+	metricsMeta, metrics, err := c.ContainerMgr.Stats(ctx, meta.ID)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get stats of container %q: %v", meta.ID, err)
 	}
 
-	if stats != nil {
-		s, err := typeurl.UnmarshalAny(stats.Data)
-		if err != nil {
-			return nil, fmt.Errorf("failed to extract container metrics: %v", err)
-		}
-		metrics := s.(*cgroups.Metrics)
+	if metricsMeta != nil {
 		if metrics.CPU != nil && metrics.CPU.Usage != nil {
 			cs.Cpu = &runtime.CpuUsage{
-				Timestamp:            stats.Timestamp.UnixNano(),
+				Timestamp:            metricsMeta.Timestamp.UnixNano(),
 				UsageCoreNanoSeconds: &runtime.UInt64Value{Value: metrics.CPU.Usage.Total},
 			}
 		}
 		if metrics.Memory != nil && metrics.Memory.Usage != nil {
 			cs.Memory = &runtime.MemoryUsage{
-				Timestamp:       stats.Timestamp.UnixNano(),
+				Timestamp:       metricsMeta.Timestamp.UnixNano(),
 				WorkingSetBytes: &runtime.UInt64Value{Value: metrics.Memory.Usage.Usage},
 			}
 		}

daemon/mgr/container_stats.go

@@ -32,48 +32,42 @@ func (mgr *ContainerManager) StreamStats(ctx context.Context, name string, confi
 	}
 
 	outStream := config.OutStream
-	if !c.IsRunningOrPaused() {
-		return errors.New("can only stats running or paused container")
-	}
 
 	var preCPUStats *types.CPUStats
 
-	wrapContainerStats := func(timestamp time.Time, metric *cgroups.Metrics) (*types.ContainerStats, error) {
-		stats := toContainerStats(timestamp, c, metric)
+	wrapContainerStats := func(metricMeta *containerdtypes.Metric, metric *cgroups.Metrics) (*types.ContainerStats, error) {
+		stats := toContainerStats(c, metricMeta, metric)
+
 		systemCPUUsage, err := getSystemCPUUsage()
 		if err != nil {
 			return nil, err
 		}
 		stats.PrecpuStats = preCPUStats
 		stats.CPUStats.SyetemCPUUsage = systemCPUUsage
 		preCPUStats = stats.CPUStats
-		networkStat, err := mgr.NetworkMgr.GetNetworkStats(c.NetworkSettings.SandboxID)
-		if err != nil {
-			// --net=none or disconnect from network, the sandbox will be nil
-			logrus.Warnf("sandbox not found, name = %s, err= %v", name, err)
-		}
+
+		networkStat, _ := mgr.NetworkMgr.GetNetworkStats(c.NetworkSettings.SandboxID)
 		stats.Networks = networkStat
 		return stats, nil
 	}
 
-	if c.IsRunningOrPaused() && !config.Stream {
+	// just collect stats data once.
+	if !config.Stream {
 		metrics, stats, err := mgr.Stats(ctx, name)
 		if err != nil {
 			return err
 		}
-		containerStat, err := wrapContainerStats(metrics.Timestamp, stats)
+		containerStat, err := wrapContainerStats(metrics, stats)
 		if err != nil {
 			return errors.Errorf("failed to wrap the containerStat: %v", err)
 		}
 		return json.NewEncoder(outStream).Encode(containerStat)
 	}
 
-	if config.Stream {
-		wf := ioutils.NewWriteFlusher(outStream)
-		defer wf.Close()
-		wf.Flush()
-		outStream = wf
-	}
+	wf := ioutils.NewWriteFlusher(outStream)
+	defer wf.Close()
+	wf.Flush()
+	outStream = wf
 
 	enc := json.NewEncoder(outStream)
 
@@ -89,14 +83,12 @@ func (mgr *ContainerManager) StreamStats(ctx context.Context, name string, confi
 				return err
 			}
 
-			if metrics != nil {
-				containerStat, err := wrapContainerStats(metrics.Timestamp, stats)
-				if err != nil {
-					return errors.Errorf("failed to wrap the containerStat: %v", err)
-				}
-				if err := enc.Encode(containerStat); err != nil {
-					return err
-				}
+			containerStat, err := wrapContainerStats(metrics, stats)
+			if err != nil {
+				return errors.Errorf("failed to wrap the containerStat: %v", err)
+			}
+			if err := enc.Encode(containerStat); err != nil {
+				return err
 			}
 
 			time.Sleep(DefaultStatsInterval)
@@ -114,10 +106,9 @@ func (mgr *ContainerManager) Stats(ctx context.Context, name string) (*container
 	c.Lock()
 	defer c.Unlock()
 
-	// only get metrics when the container is running
-	// return error to help client quick fail
+	// empty stats for not-running container.
 	if !c.IsRunningOrPaused() {
-		return nil, nil, errors.New("can only stats running or paused container")
+		return nil, nil, nil
 	}
 
 	metric, err := mgr.Client.ContainerStats(ctx, c.ID)
@@ -133,9 +124,19 @@ func (mgr *ContainerManager) Stats(ctx context.Context, name string) (*container
 	return metric, v.(*cgroups.Metrics), nil
 }
 
-func toContainerStats(time time.Time, container *Container, metric *cgroups.Metrics) *types.ContainerStats {
+func toContainerStats(container *Container, metricMeta *containerdtypes.Metric, metric *cgroups.Metrics) *types.ContainerStats {
+	if metricMeta == nil {
+		return &types.ContainerStats{
+			ID:          container.ID,
+			Name:        container.Name,
+			PidsStats:   &types.PidsStats{},
+			CPUStats:    &types.CPUStats{},
+			BlkioStats:  &types.BlkioStats{},
+			MemoryStats: &types.MemoryStats{},
+		}
+	}
 	return &types.ContainerStats{
-		Read: strfmt.DateTime(time),
+		Read: strfmt.DateTime(metricMeta.Timestamp),
 		ID:   container.ID,
 		Name: container.Name,
 		PidsStats: &types.PidsStats{

test/cli_stats_test.go

@@ -45,15 +45,3 @@ func (s *PouchStatsSuite) TestStatsNoStream(c *check.C) {
 		c.Fatalf("container name not present in the stats output, %s", res.Stdout())
 	}
 }
-
-func (s *PouchStatsSuite) TestStatsWrongState(c *check.C) {
-	cname := "TestStatsWrongState"
-	command.PouchRun("create", "--name", cname, busyboxImage, "top").Assert(c, icmd.Success)
-	defer DelContainerForceMultyTime(c, cname)
-
-	res := command.PouchRun("stats", "--no-stream", cname)
-	errString := res.Stderr()
-	if !strings.Contains(errString, "can only stats running or paused container") {
-		c.Fatalf("got unexpected error, %s", errString)
-	}
-}