Merge pull request #108787 from 249043822/cadvisor_stat_provider_filter_0

filter out terminated containers in cadvisor_stats_provider

pkg/kubelet/stats/cadvisor_stats_provider.go

@@ -351,7 +351,11 @@ func filterTerminatedContainerInfoAndAssembleByPodCgroupKey(containerInfo map[st
 	result := make(map[string]cadvisorapiv2.ContainerInfo)
 	for _, refs := range cinfoMap {
 		if len(refs) == 1 {
+			// ContainerInfo with no CPU/memory usage for uncleaned cgroups of
+			// already terminated containers, which should not be shown in the results.
+			if !isContainerTerminated(&refs[0].cinfo) {
 				result[refs[0].cgroup] = refs[0].cinfo
+			}
 			continue
 		}
 		sort.Sort(ByCreationTime(refs))
@@ -411,6 +415,23 @@ func hasMemoryAndCPUInstUsage(info *cadvisorapiv2.ContainerInfo) bool {
 	return cstat.CpuInst.Usage.Total != 0 && cstat.Memory.RSS != 0
 }
 
+// isContainerTerminated returns true if the specified container info has
+// both zero CPU instantaneous usage and zero memory RSS usage, and
+// false otherwise.
+func isContainerTerminated(info *cadvisorapiv2.ContainerInfo) bool {
+	if !info.Spec.HasCpu && !info.Spec.HasMemory {
+		return true
+	}
+	cstat, found := latestContainerStats(info)
+	if !found {
+		return true
+	}
+	if cstat.CpuInst == nil || cstat.Memory == nil {
+		return true
+	}
+	return cstat.CpuInst.Usage.Total == 0 && cstat.Memory.RSS == 0
+}
+
 func getCadvisorContainerInfo(ca cadvisor.Interface) (map[string]cadvisorapiv2.ContainerInfo, error) {
 	infos, err := ca.ContainerInfoV2("/", cadvisorapiv2.RequestOptions{
 		IdType:    cadvisorapiv2.TypeName,

pkg/kubelet/stats/cadvisor_stats_provider_test.go

@@ -49,6 +49,10 @@ func TestFilterTerminatedContainerInfoAndAssembleByPodCgroupKey(t *testing.T) {
 		namespace = "test"
 		pName0    = "pod0"
 		cName00   = "c0"
+		pName1    = "pod1"
+		cName11   = "c1"
+		pName2    = "pod2"
+		cName22   = "c2"
 	)
 	infos := map[string]cadvisorapiv2.ContainerInfo{
 		// ContainerInfo with past creation time and no CPU/memory usage for
@@ -66,16 +70,25 @@ func TestFilterTerminatedContainerInfoAndAssembleByPodCgroupKey(t *testing.T) {
 		// The latest containers, which should be in the results.
 		"/pod0-i":  getTestContainerInfo(seedPod0Infra, pName0, namespace, leaky.PodInfraContainerName),
 		"/pod0-c0": getTestContainerInfo(seedPod0Container0, pName0, namespace, cName00),
+
+		"/pod1-i.slice":  getTestContainerInfo(seedPod0Infra, pName1, namespace, leaky.PodInfraContainerName),
+		"/pod1-c1.slice": getTestContainerInfo(seedPod0Container0, pName1, namespace, cName11),
+
+		"/pod2-i-terminated-1": getTerminatedContainerInfo(seedPastPod0Infra, pName2, namespace, leaky.PodInfraContainerName),
+		// ContainerInfo with past creation time and no CPU/memory usage for
+		// simulating uncleaned cgroups of already terminated containers, which
+		// should not be shown in the results.
+		"/pod2-c0-terminated-1": getTerminatedContainerInfo(seedPastPod0Container0, pName2, namespace, cName22),
 	}
 	filteredInfos, allInfos := filterTerminatedContainerInfoAndAssembleByPodCgroupKey(infos)
-	assert.Len(t, filteredInfos, 2)
+	assert.Len(t, filteredInfos, 4)
-	assert.Len(t, allInfos, 6)
+	assert.Len(t, allInfos, 10)
 	for _, c := range []string{"/pod0-i", "/pod0-c0"} {
 		if _, found := filteredInfos[c]; !found {
 			t.Errorf("%q is expected to be in the output\n", c)
 		}
 	}
-	for _, c := range []string{"pod0-i-terminated-1", "pod0-c0-terminated-1", "pod0-i-terminated-2", "pod0-c0-terminated-2", "pod0-i", "pod0-c0"} {
+	for _, c := range []string{"pod0-i-terminated-1", "pod0-c0-terminated-1", "pod0-i-terminated-2", "pod0-c0-terminated-2", "pod0-i", "pod0-c0", "c1"} {
 		if _, found := allInfos[c]; !found {
 			t.Errorf("%q is expected to be in the output\n", c)
 		}
@@ -305,7 +318,8 @@ func TestCadvisorListPodStats(t *testing.T) {
 
 	ps, found = indexPods[prf3]
 	assert.True(t, found)
-	assert.Len(t, ps.Containers, 2)
+	// /pod3-c0-init has no stats should be filtered
+	assert.Len(t, ps.Containers, 1)
 	indexCon = make(map[string]statsapi.ContainerStats, len(ps.Containers))
 	for _, con := range ps.Containers {
 		indexCon[con.Name] = con
@@ -314,10 +328,6 @@ func TestCadvisorListPodStats(t *testing.T) {
 	assert.Equal(t, cName31, con.Name)
 	checkCPUStats(t, "Pod3Container1", seedPod3Container1, con.CPU)
 	checkMemoryStats(t, "Pod3Container1", seedPod3Container1, infos["/pod3-c1"], con.Memory)
-	con = indexCon[cName30]
-	assert.Equal(t, cName30, con.Name)
-	checkEmptyCPUStats(t, "Pod3Container0", seedPod3Container0, con.CPU)
-	checkEmptyMemoryStats(t, "Pod3Container0", seedPod3Container0, infos["/pod3-c0-init"], con.Memory)
 }
 
 func TestCadvisorListPodCPUAndMemoryStats(t *testing.T) {

pkg/kubelet/stats/provider_test.go

@@ -647,28 +647,6 @@ func checkNetworkStats(t *testing.T, label string, seed int, stats *statsapi.Net
 
 }
 
-// container which had no stats should have zero-valued CPU usage
-func checkEmptyCPUStats(t *testing.T, label string, seed int, stats *statsapi.CPUStats) {
-	require.NotNil(t, stats.Time, label+".CPU.Time")
-	require.NotNil(t, stats.UsageNanoCores, label+".CPU.UsageNanoCores")
-	require.NotNil(t, stats.UsageNanoCores, label+".CPU.UsageCoreSeconds")
-	assert.EqualValues(t, testTime(timestamp, seed).Unix(), stats.Time.Time.Unix(), label+".CPU.Time")
-	assert.EqualValues(t, 0, *stats.UsageNanoCores, label+".CPU.UsageCores")
-	assert.EqualValues(t, 0, *stats.UsageCoreNanoSeconds, label+".CPU.UsageCoreSeconds")
-}
-
-// container which had no stats should have zero-valued Memory usage
-func checkEmptyMemoryStats(t *testing.T, label string, seed int, info cadvisorapiv2.ContainerInfo, stats *statsapi.MemoryStats) {
-	assert.EqualValues(t, testTime(timestamp, seed).Unix(), stats.Time.Time.Unix(), label+".Mem.Time")
-	require.NotNil(t, stats.WorkingSetBytes, label+".Mem.WorkingSetBytes")
-	assert.EqualValues(t, 0, *stats.WorkingSetBytes, label+".Mem.WorkingSetBytes")
-	assert.Nil(t, stats.UsageBytes, label+".Mem.UsageBytes")
-	assert.Nil(t, stats.RSSBytes, label+".Mem.RSSBytes")
-	assert.Nil(t, stats.PageFaults, label+".Mem.PageFaults")
-	assert.Nil(t, stats.MajorPageFaults, label+".Mem.MajorPageFaults")
-	assert.Nil(t, stats.AvailableBytes, label+".Mem.AvailableBytes")
-}
-
 func checkCPUStats(t *testing.T, label string, seed int, stats *statsapi.CPUStats) {
 	require.NotNil(t, stats.Time, label+".CPU.Time")
 	require.NotNil(t, stats.UsageNanoCores, label+".CPU.UsageNanoCores")