Cumulative stats can't decrease

jsturtevant · jsturtevant · commit 88d001c749a8 · 2024-09-03T18:01:30.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/sbserver/container_stats_list.go b/pkg/cri/sbserver/container_stats_list.go
@@ -192,6 +192,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/sbserver/container_stats_list_test.go b/pkg/cri/sbserver/container_stats_list_test.go
@@ -35,45 +35,56 @@ import (
 func TestContainerMetricsCPUNanoCoreUsage(t *testing.T) {
 	c := newTestCRIService()
 	timestamp := time.Now()
-	secondAfterTimeStamp := timestamp.Add(time.Second)
-	ID := "ID"
+	tenSecondAftertimeStamp := timestamp.Add(time.Second * 10)
 
 	for desc, test := range map[string]struct {
+		id                          string
+		desc                        string
 		firstCPUValue               uint64
 		secondCPUValue              uint64
 		expectedNanoCoreUsageFirst  uint64
 		expectedNanoCoreUsageSecond uint64
 	}{
 		"metrics": {
+			id:                          "id1",
+			desc:                        "metrics",
 			firstCPUValue:               50,
 			secondCPUValue:              500,
 			expectedNanoCoreUsageFirst:  0,
-			expectedNanoCoreUsageSecond: 450,
+			expectedNanoCoreUsageSecond: 45,
+		},
+		"no metrics in second CPU sample": {
+			id:                          "id2",
+			desc:                        "metrics",
+			firstCPUValue:               234235,
+			secondCPUValue:              0,
+			expectedNanoCoreUsageFirst:  0,
+			expectedNanoCoreUsageSecond: 0,
 		},
 	} {
 		t.Run(desc, func(t *testing.T) {
 			container, err := containerstore.NewContainer(
-				containerstore.Metadata{ID: ID},
+				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(ID, false, test.firstCPUValue, timestamp)
+			cpuUsage, err := c.getUsageNanoCores(test.id, false, test.firstCPUValue, timestamp)
 			assert.NoError(t, err)
 
-			container, err = c.containerStore.Get(ID)
+			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(ID, false, test.secondCPUValue, secondAfterTimeStamp)
+			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(ID)
+			container, err = c.containerStore.Get(test.id)
 			assert.NoError(t, err)
 			assert.NotNil(t, container.Stats)
 		})
