Skip to content

Commit

Permalink
Cumulative stats can't decrease
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jsturtevant committed Aug 28, 2024
1 parent 283149d commit f6677a4
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
5 changes: 5 additions & 0 deletions internal/cri/server/container_stats_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,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))

Expand Down
25 changes: 17 additions & 8 deletions internal/cri/server/container_stats_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,48 +35,57 @@ 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 _, 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: 450,
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: 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)
})
Expand Down

0 comments on commit f6677a4

Please sign in to comment.