From 4762985fb6819bb958076830ce35f957db41f594 Mon Sep 17 00:00:00 2001 From: Ryan Phillips Date: Fri, 6 Dec 2019 11:47:28 -0600 Subject: [PATCH] kubelet: guarantee at most only one cinfo per containerID The trailing for loop removed within this PR can include active and inactive references. This modification will gauarantee at most only one reference per container is returned. --- pkg/kubelet/stats/cadvisor_stats_provider.go | 9 ++------- pkg/kubelet/stats/cadvisor_stats_provider_test.go | 10 ++-------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/pkg/kubelet/stats/cadvisor_stats_provider.go b/pkg/kubelet/stats/cadvisor_stats_provider.go index 277bc415afa..319040349e9 100644 --- a/pkg/kubelet/stats/cadvisor_stats_provider.go +++ b/pkg/kubelet/stats/cadvisor_stats_provider.go @@ -329,17 +329,12 @@ func removeTerminatedContainerInfo(containerInfo map[string]cadvisorapiv2.Contai continue } sort.Sort(ByCreationTime(refs)) - i := 0 - for ; i < len(refs); i++ { + for i := len(refs) - 1; i >= 0; i-- { if hasMemoryAndCPUInstUsage(&refs[i].cinfo) { - // Stops removing when we first see an info with non-zero - // CPU/Memory usage. + result[refs[i].cgroup] = refs[i].cinfo break } } - for ; i < len(refs); i++ { - result[refs[i].cgroup] = refs[i].cinfo - } } return result } diff --git a/pkg/kubelet/stats/cadvisor_stats_provider_test.go b/pkg/kubelet/stats/cadvisor_stats_provider_test.go index 9c4b6ddd78a..1695c6503a5 100644 --- a/pkg/kubelet/stats/cadvisor_stats_provider_test.go +++ b/pkg/kubelet/stats/cadvisor_stats_provider_test.go @@ -62,16 +62,10 @@ func TestRemoveTerminatedContainerInfo(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), - - // Duplicated containers with non-zero CPU and memory usage. This case - // shouldn't happen unless something goes wrong, but we want to test - // that the metrics reporting logic works in this scenario. - "/pod0-i-duplicated": getTestContainerInfo(seedPod0Infra, pName0, namespace, leaky.PodInfraContainerName), - "/pod0-c0-duplicated": getTestContainerInfo(seedPod0Container0, pName0, namespace, cName00), } output := removeTerminatedContainerInfo(infos) - assert.Len(t, output, 4) - for _, c := range []string{"/pod0-i", "/pod0-c0", "/pod0-i-duplicated", "/pod0-c0-duplicated"} { + assert.Len(t, output, 2) + for _, c := range []string{"/pod0-i", "/pod0-c0"} { if _, found := output[c]; !found { t.Errorf("%q is expected to be in the output\n", c) }