Cumulative stats can't decrease

jsturtevant · jsturtevant · commit c8e5b1eb692d · 2024-09-03T18:45:51.000Z
During removal of the container a stat value might be reported as zero; in this case the caluclation could end up with an extremely large number. If the cumulative stat decreases report zero. Signed-off-by: James Sturtevant <jstur@microsoft.com> (cherry picked from commit f6677a4) Signed-off-by: James Sturtevant <jstur@microsoft.com>
diff --git a/pkg/cri/server/container_stats_list_linux.go b/pkg/cri/server/container_stats_list_linux.go
@@ -123,6 +123,11 @@ func (c *criService) getUsageNanoCores(containerID string, isSandbox bool, curre
 		return 0, nil
 	}
 
+	// can't go backwards, this value might come in as 0 if the container was just removed
+	if currentUsageCoreNanoSeconds < oldStats.UsageCoreNanoSeconds {
+		return 0, nil
+	}
+
 	newUsageNanoCores := uint64(float64(currentUsageCoreNanoSeconds-oldStats.UsageCoreNanoSeconds) /
 		float64(nanoSeconds) * float64(time.Second/time.Nanosecond))
 
diff --git a/pkg/cri/server/container_stats_list_linux_test.go b/pkg/cri/server/container_stats_list_linux_test.go
@@ -28,6 +28,66 @@ import (
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 )
 
+func TestContainerMetricsCPUNanoCoreUsage(t *testing.T) {
+	c := newTestCRIService()
+	timestamp := time.Now()
+	tenSecondAftertimeStamp := timestamp.Add(time.Second * 10)
+
+	for _, test := range []struct {
+		id                          string
+		desc                        string
+		firstCPUValue               uint64
+		secondCPUValue              uint64
+		expectedNanoCoreUsageFirst  uint64
+		expectedNanoCoreUsageSecond uint64
+	}{
+		{
+			id:                          "id1",
+			desc:                        "metrics",
+			firstCPUValue:               50,
+			secondCPUValue:              500,
+			expectedNanoCoreUsageFirst:  0,
+			expectedNanoCoreUsageSecond: 45,
+		},
+		{
+			id:                          "id2",
+			desc:                        "metrics",
+			firstCPUValue:               234235,
+			secondCPUValue:              0,
+			expectedNanoCoreUsageFirst:  0,
+			expectedNanoCoreUsageSecond: 0,
+		},
+	} {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			container, err := containerstore.NewContainer(
+				containerstore.Metadata{ID: test.id},
+			)
+			assert.NoError(t, err)
+			assert.Nil(t, container.Stats)
+			err = c.containerStore.Add(container)
+			assert.NoError(t, err)
+
+			cpuUsage, err := c.getUsageNanoCores(test.id, false, test.firstCPUValue, timestamp)
+			assert.NoError(t, err)
+
+			container, err = c.containerStore.Get(test.id)
+			assert.NoError(t, err)
+			assert.NotNil(t, container.Stats)
+
+			assert.Equal(t, test.expectedNanoCoreUsageFirst, cpuUsage)
+
+			cpuUsage, err = c.getUsageNanoCores(test.id, false, test.secondCPUValue, tenSecondAftertimeStamp)
+			assert.NoError(t, err)
+			assert.Equal(t, test.expectedNanoCoreUsageSecond, cpuUsage)
+
+			container, err = c.containerStore.Get(test.id)
+			assert.NoError(t, err)
+			assert.NotNil(t, container.Stats)
+		})
+	}
+}
+
 func TestGetWorkingSet(t *testing.T) {
 	for desc, test := range map[string]struct {
 		memory   *v1.MemoryStat
