From 6d3ef495ae00abe5e147addf1b41f643b3c5a229 Mon Sep 17 00:00:00 2001 From: zhuangqh Date: Sun, 14 Apr 2019 19:58:37 +0800 Subject: [PATCH] fix: allow to stats not-running container just return a empty stats data for them Signed-off-by: zhuangqh --- cli/stats_helpers.go | 21 ++++++++----- cri/v1alpha2/cri_utils.go | 15 +++------ daemon/mgr/container_stats.go | 59 +++++++++++++++++++---------------- test/cli_stats_test.go | 12 ------- 4 files changed, 50 insertions(+), 57 deletions(-) diff --git a/cli/stats_helpers.go b/cli/stats_helpers.go index c37e0889d..85bb53b4c 100644 --- a/cli/stats_helpers.go +++ b/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"]) } diff --git a/cri/v1alpha2/cri_utils.go b/cri/v1alpha2/cri_utils.go index c22bde102..3b36f2693 100644 --- a/cri/v1alpha2/cri_utils.go +++ b/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}, } } diff --git a/daemon/mgr/container_stats.go b/daemon/mgr/container_stats.go index 933e5eb70..da0cdfae3 100644 --- a/daemon/mgr/container_stats.go +++ b/daemon/mgr/container_stats.go @@ -32,14 +32,12 @@ 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 @@ -47,33 +45,33 @@ func (mgr *ContainerManager) StreamStats(ctx context.Context, name string, confi 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) + logrus.Debugf("failed to get network stats from container %s: %v", name, err) } 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 +87,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 +110,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 +128,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{ diff --git a/test/cli_stats_test.go b/test/cli_stats_test.go index bfc6b5947..057cb58c1 100644 --- a/test/cli_stats_test.go +++ b/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) - } -}