Merge pull request #18710 from TomSweeneyRedHat/dev/tsweeney/pscpufix…

…_v3.0.1-rhel [v3.0.1-rhel] podman stats: calc CPU percentage correctly
containers · Jun 5, 2023 · 9581020 · 9581020
2 parents b501c22 + ca3d490
commit 9581020
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 16 deletions.
diff --git a/libpod/pod.go b/libpod/pod.go
@@ -297,10 +297,6 @@ type PodContainerStats struct {
 
 // GetPodStats returns the stats for each of its containers
 func (p *Pod) GetPodStats(previousContainerStats map[string]*define.ContainerStats) (map[string]*define.ContainerStats, error) {
-	var (
-		ok       bool
-		prevStat *define.ContainerStats
-	)
 	p.lock.Lock()
 	defer p.lock.Unlock()
 
@@ -313,10 +309,7 @@ func (p *Pod) GetPodStats(previousContainerStats map[string]*define.ContainerSta
 	}
 	newContainerStats := make(map[string]*define.ContainerStats)
 	for _, c := range containers {
-		if prevStat, ok = previousContainerStats[c.ID()]; !ok {
-			prevStat = &define.ContainerStats{}
-		}
-		newStats, err := c.GetContainerStats(prevStat)
+		newStats, err := c.GetContainerStats(previousContainerStats[c.ID()])
 		// If the container wasn't running, don't include it
 		// but also suppress the error
 		if err != nil && errors.Cause(err) != define.ErrCtrStateInvalid {

diff --git a/libpod/stats.go b/libpod/stats.go
@@ -12,7 +12,9 @@ import (
 	"github.com/pkg/errors"
 )
 
-// GetContainerStats gets the running stats for a given container
+// GetContainerStats gets the running stats for a given container.
+// The previousStats is used to correctly calculate cpu percentages. You
+// should pass nil if there is no previous stat for this container.
 func (c *Container) GetContainerStats(previousStats *define.ContainerStats) (*define.ContainerStats, error) {
 	stats := new(define.ContainerStats)
 	stats.ContainerID = c.ID()
@@ -34,6 +36,14 @@ func (c *Container) GetContainerStats(previousStats *define.ContainerStats) (*de
 		return stats, define.ErrCtrStateInvalid
 	}
 
+	if previousStats == nil {
+		previousStats = &define.ContainerStats{
+			// if we have no prev stats use the container start time as prev time
+			// otherwise we cannot correctly calculate the CPU percentage
+			SystemNano: uint64(c.state.StartedTime.UnixNano()),
+		}
+	}
+
 	cgroupPath, err := c.cGroupPath()
 	if err != nil {
 		return nil, err

diff --git a/pkg/api/handlers/compat/containers_stats.go b/pkg/api/handlers/compat/containers_stats.go
@@ -50,7 +50,7 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	stats, err := ctnr.GetContainerStats(&define.ContainerStats{})
+	stats, err := ctnr.GetContainerStats(nil)
 	if err != nil {
 		utils.InternalServerError(w, errors.Wrapf(err, "failed to obtain Container %s stats", name))
 		return

diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
@@ -1248,12 +1248,7 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri
 
 			reportStats := []define.ContainerStats{}
 			for _, ctr := range containers {
-				prev, ok := containerStats[ctr.ID()]
-				if !ok {
-					prev = &define.ContainerStats{}
-				}
-
-				stats, err := ctr.GetContainerStats(prev)
+				stats, err := ctr.GetContainerStats(containerStats[ctr.ID()])
 				if err != nil {
 					cause := errors.Cause(err)
 					if queryAll && (cause == define.ErrCtrRemoved || cause == define.ErrNoSuchCtr || cause == define.ErrCtrStateInvalid) {