Merge pull request #110950 from yangjunmyfm192085/fixmetricsjudgement

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 {
-			result[refs[0].cgroup] = refs[0].cinfo
+			// ContainerInfo with no CPU/memory/network 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,35 @@ func hasMemoryAndCPUInstUsage(info *cadvisorapiv2.ContainerInfo) bool {
 	return cstat.CpuInst.Usage.Total != 0 && cstat.Memory.RSS != 0
 }
 
+// isContainerTerminated returns true if the specified container meet one of the following conditions
+// 1. info.spec both cpu memory and network are false conditions
+// 2. info.Stats both network and cpu or memory are nil
+// 3. both zero CPU instantaneous usage zero memory RSS usage and zero network usage,
+// and false otherwise.
+func isContainerTerminated(info *cadvisorapiv2.ContainerInfo) bool {
+	if !info.Spec.HasCpu && !info.Spec.HasMemory && !info.Spec.HasNetwork {
+		return true
+	}
+	cstat, found := latestContainerStats(info)
+	if !found {
+		return true
+	}
+	if cstat.Network != nil {
+		iStats := cadvisorInfoToNetworkStats(info)
+		if iStats != nil {
+			for _, iStat := range iStats.Interfaces {
+				if *iStat.RxErrors != 0 || *iStat.TxErrors != 0 || *iStat.RxBytes != 0 || *iStat.TxBytes != 0 {
+					return false
+				}
+			}
+		}
+	}
+	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

@@ -19,7 +19,7 @@ package stats
 import (
 	"testing"
 
-	gomock "github.com/golang/mock/gomock"
+	"github.com/golang/mock/gomock"
 	cadvisorapiv2 "github.com/google/cadvisor/info/v2"
 	"github.com/stretchr/testify/assert"
 
@@ -49,6 +49,11 @@ func TestFilterTerminatedContainerInfoAndAssembleByPodCgroupKey(t *testing.T) {
 		namespace = "test"
 		pName0    = "pod0"
 		cName00   = "c0"
+		pName1    = "pod1"
+		cName11   = "c1"
+		pName2    = "pod2"
+		cName22   = "c2"
+		cName222  = "c222"
 	)
 	infos := map[string]cadvisorapiv2.ContainerInfo{
 		// ContainerInfo with past creation time and no CPU/memory usage for
@@ -66,16 +71,28 @@ 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-c2-terminated-1": getTerminatedContainerInfo(seedPastPod0Container0, pName2, namespace, cName22),
+
+		//ContainerInfo with no CPU/memory usage but has network usage for uncleaned cgroups, should not be filtered out
+		"/pod2-c222-zerocpumem-1": getContainerInfoWithZeroCpuMem(seedPastPod0Container0, pName2, namespace, cName222),
 	}
 	filteredInfos, allInfos := filterTerminatedContainerInfoAndAssembleByPodCgroupKey(infos)
-	assert.Len(t, filteredInfos, 2)
-	assert.Len(t, allInfos, 6)
+	assert.Len(t, filteredInfos, 5)
+	assert.Len(t, allInfos, 11)
 	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)
 		}
@@ -203,6 +220,7 @@ func TestCadvisorListPodStats(t *testing.T) {
 		info.Spec.Cpu = cadvisorapiv2.CpuSpec{}
 		info.Spec.HasMemory = false
 		info.Spec.HasCpu = false
+		info.Spec.HasNetwork = false
 		infos[name] = info
 	}
 
@@ -305,7 +323,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 +333,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

@@ -21,7 +21,7 @@ import (
 	"testing"
 	"time"
 
-	gomock "github.com/golang/mock/gomock"
+	"github.com/golang/mock/gomock"
 	cadvisorapiv1 "github.com/google/cadvisor/info/v1"
 	cadvisorapiv2 "github.com/google/cadvisor/info/v2"
 	fuzz "github.com/google/gofuzz"
@@ -447,6 +447,28 @@ func TestHasDedicatedImageFs(t *testing.T) {
 }
 
 func getTerminatedContainerInfo(seed int, podName string, podNamespace string, containerName string) cadvisorapiv2.ContainerInfo {
+	cinfo := getTestContainerInfo(seed, podName, podNamespace, containerName)
+	cinfo.Stats[0].Memory.RSS = 0
+	cinfo.Stats[0].CpuInst.Usage.Total = 0
+	cinfo.Stats[0].Network = &cadvisorapiv2.NetworkStats{
+		Interfaces: []cadvisorapiv1.InterfaceStats{{
+			Name:     "eth0",
+			RxBytes:  0,
+			RxErrors: 0,
+			TxBytes:  0,
+			TxErrors: 0,
+		}, {
+			Name:     "cbr0",
+			RxBytes:  0,
+			RxErrors: 0,
+			TxBytes:  0,
+			TxErrors: 0,
+		}},
+	}
+	return cinfo
+}
+
+func getContainerInfoWithZeroCpuMem(seed int, podName string, podNamespace string, containerName string) cadvisorapiv2.ContainerInfo {
 	cinfo := getTestContainerInfo(seed, podName, podNamespace, containerName)
 	cinfo.Stats[0].Memory.RSS = 0
 	cinfo.Stats[0].CpuInst.Usage.Total = 0
@@ -647,28 +669,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")