From 1c3357db76687ded50fbcbda3881576bfa958581 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Tue, 15 Feb 2022 16:28:17 -0500 Subject: [PATCH] kubelet/stats: take container log stats into account when checking ephemeral stats this commit updates checkEphemeralStorage to be able to add container log stats, if applicable. It also updates the old check when container log stats aren't found to be more accurate. Specifically, this check previously worked because of a fluke programming accident: according to this block in pkg/kubelet/stats/helper.go:113 ``` if result.Rootfs != nil { rootfsUsage := *cfs.BaseUsageBytes result.Rootfs.UsedBytes = &rootfsUsage } ``` BaseUsageBytes should be the value added, not TotalUsageBytes. However, since in this case one also needs to account for the calculated log size, which is TotalUsageBytes - BaseUsageBytes using TotalUsageBytes value accidentally worked. Updating the case to use the correct value AND log offset fixes this accident and makes the behavior more in line with what happens when calculating ephemeral storage. Signed-off-by: Peter Hunt --- .../stats/cadvisor_stats_provider_test.go | 2 +- pkg/kubelet/stats/provider_test.go | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/kubelet/stats/cadvisor_stats_provider_test.go b/pkg/kubelet/stats/cadvisor_stats_provider_test.go index 303d62526ba..96a3f81abfc 100644 --- a/pkg/kubelet/stats/cadvisor_stats_provider_test.go +++ b/pkg/kubelet/stats/cadvisor_stats_provider_test.go @@ -270,7 +270,7 @@ func TestCadvisorListPodStats(t *testing.T) { assert.EqualValues(t, p0Time.Unix(), ps.StartTime.Time.Unix()) checkNetworkStats(t, "Pod0", seedPod0Infra, ps.Network) - checkEphemeralStats(t, "Pod0", []int{seedPod0Container0, seedPod0Container1}, []int{seedEphemeralVolume1, seedEphemeralVolume2}, ps.EphemeralStorage) + checkEphemeralStats(t, "Pod0", []int{seedPod0Container0, seedPod0Container1}, []int{seedEphemeralVolume1, seedEphemeralVolume2}, nil, ps.EphemeralStorage) if ps.CPU != nil { checkCPUStats(t, "Pod0", seedPod0Infra, ps.CPU) } diff --git a/pkg/kubelet/stats/provider_test.go b/pkg/kubelet/stats/provider_test.go index 3bbc8334ba0..220d000e937 100644 --- a/pkg/kubelet/stats/provider_test.go +++ b/pkg/kubelet/stats/provider_test.go @@ -37,6 +37,7 @@ import ( kubepodtest "k8s.io/kubernetes/pkg/kubelet/pod/testing" serverstats "k8s.io/kubernetes/pkg/kubelet/server/stats" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" + "k8s.io/kubernetes/pkg/volume" ) const ( @@ -699,16 +700,27 @@ func checkFsStats(t *testing.T, label string, seed int, stats *statsapi.FsStats) assert.EqualValues(t, seed+offsetFsInodesFree, *stats.InodesFree, label+".InodesFree") } -func checkEphemeralStats(t *testing.T, label string, containerSeeds []int, volumeSeeds []int, stats *statsapi.FsStats) { +func checkEphemeralStats(t *testing.T, label string, containerSeeds []int, volumeSeeds []int, containerLogStats []*volume.Metrics, stats *statsapi.FsStats) { var usedBytes, inodeUsage int for _, cseed := range containerSeeds { - usedBytes = usedBytes + cseed + offsetFsTotalUsageBytes + usedBytes += cseed + offsetFsBaseUsageBytes inodeUsage += cseed + offsetFsInodeUsage + // If containerLogStats is nil, then the log stats calculated from cAdvisor + // information is used. Since it's Total - Base, and these values are + // set to the offset, we can use the calculated difference in the offset + // to account for this. + if containerLogStats == nil { + usedBytes += offsetFsTotalUsageBytes - offsetFsBaseUsageBytes + } } for _, vseed := range volumeSeeds { - usedBytes = usedBytes + vseed + offsetFsUsage + usedBytes += vseed + offsetFsUsage inodeUsage += vseed + offsetFsInodeUsage } + for _, logStats := range containerLogStats { + usedBytes += int(logStats.Used.Value()) + inodeUsage += int(logStats.InodesUsed.Value()) + } assert.EqualValues(t, usedBytes, int(*stats.UsedBytes), label+".UsedBytes") assert.EqualValues(t, inodeUsage, int(*stats.InodesUsed), label+".InodesUsed") }