From 3be31530d5a3dc9b75960b773a79192c6fb75039 Mon Sep 17 00:00:00 2001 From: Ashley Kasim Date: Fri, 4 Oct 2019 09:07:53 -0700 Subject: [PATCH] cri_stats_provider: do not consider exited containers when calculating cpu usage --- pkg/kubelet/stats/cri_stats_provider.go | 14 +----- pkg/kubelet/stats/cri_stats_provider_test.go | 47 +++++++++++--------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/pkg/kubelet/stats/cri_stats_provider.go b/pkg/kubelet/stats/cri_stats_provider.go index 492c16bbd77..48884364e97 100644 --- a/pkg/kubelet/stats/cri_stats_provider.go +++ b/pkg/kubelet/stats/cri_stats_provider.go @@ -733,9 +733,8 @@ func removeTerminatedPods(pods []*runtimeapi.PodSandbox) []*runtimeapi.PodSandbo return result } -// removeTerminatedContainers returns containers with terminated ones. -// It only removes a terminated container when there is a running instance -// of the container. +// removeTerminatedContainers removes all terminated containers since they should +// not be used for usage calculations. func removeTerminatedContainers(containers []*runtimeapi.Container) []*runtimeapi.Container { containerMap := make(map[containerID][]*runtimeapi.Container) // Sort order by create time @@ -752,20 +751,11 @@ func removeTerminatedContainers(containers []*runtimeapi.Container) []*runtimeap result := make([]*runtimeapi.Container, 0) for _, refs := range containerMap { - if len(refs) == 1 { - result = append(result, refs[0]) - continue - } - found := false for i := 0; i < len(refs); i++ { if refs[i].State == runtimeapi.ContainerState_CONTAINER_RUNNING { - found = true result = append(result, refs[i]) } } - if !found { - result = append(result, refs[len(refs)-1]) - } } return result } diff --git a/pkg/kubelet/stats/cri_stats_provider_test.go b/pkg/kubelet/stats/cri_stats_provider_test.go index 00391d1eccb..505a378a8e8 100644 --- a/pkg/kubelet/stats/cri_stats_provider_test.go +++ b/pkg/kubelet/stats/cri_stats_provider_test.go @@ -77,6 +77,7 @@ const ( cName5 = "container5-name" cName6 = "container6-name" cName7 = "container7-name" + cName8 = "container8-name" ) func TestCRIListPodStats(t *testing.T) { @@ -109,10 +110,14 @@ func TestCRIListPodStats(t *testing.T) { containerStats4 = makeFakeContainerStats(container4, imageFsMountpoint) containerLogStats4 = makeFakeLogStats(4000) + // Running pod with a terminated container and a running container sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns", false) + sandbox3Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox2.PodSandboxStatus.Metadata.Uid)) container5 = makeFakeContainer(sandbox3, cName5, 0, true) containerStats5 = makeFakeContainerStats(container5, imageFsMountpoint) containerLogStats5 = makeFakeLogStats(5000) + container8 = makeFakeContainer(sandbox3, cName8, 0, false) + containerStats8 = makeFakeContainerStats(container8, imageFsMountpoint) // Terminated pod sandbox sandbox4 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", true) @@ -153,6 +158,7 @@ func TestCRIListPodStats(t *testing.T) { sandbox2.PodSandboxStatus.Id: getTestContainerInfo(seedSandbox2, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, leaky.PodInfraContainerName), sandbox2Cgroup: getTestContainerInfo(seedSandbox2, "", "", ""), container4.ContainerStatus.Id: getTestContainerInfo(seedContainer3, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, cName3), + sandbox3Cgroup: getTestContainerInfo(seedSandbox3, "", "", ""), } options := cadvisorapiv2.RequestOptions{ @@ -170,10 +176,10 @@ func TestCRIListPodStats(t *testing.T) { sandbox0, sandbox1, sandbox2, sandbox3, sandbox4, sandbox5, }) fakeRuntimeService.SetFakeContainers([]*critest.FakeContainer{ - container0, container1, container2, container3, container4, container5, container6, container7, + container0, container1, container2, container3, container4, container5, container6, container7, container8, }) fakeRuntimeService.SetFakeContainerStats([]*runtimeapi.ContainerStats{ - containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, + containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, containerStats8 }) ephemeralVolumes := makeFakeVolumeStats([]string{"ephVolume1, ephVolumes2"}) @@ -296,14 +302,12 @@ func TestCRIListPodStats(t *testing.T) { assert.Equal(sandbox3.CreatedAt, p3.StartTime.UnixNano()) assert.Equal(1, len(p3.Containers)) - c5 := p3.Containers[0] - assert.Equal(cName5, c5.Name) - assert.Equal(container5.CreatedAt, c5.StartTime.UnixNano()) - assert.NotNil(c5.CPU.Time) - assert.Zero(*c5.CPU.UsageCoreNanoSeconds) - assert.Zero(*c5.CPU.UsageNanoCores) - assert.NotNil(c5.Memory.Time) - assert.Zero(*c5.Memory.WorkingSetBytes) + c8 := p3.Containers[0] + assert.Equal(cName8, c8.Name) + assert.Equal(container8.CreatedAt, c8.StartTime.UnixNano()) + assert.NotNil(c8.CPU.Time) + assert.NotNil(c8.Memory.Time) + checkCRIPodCPUAndMemoryStats(assert, p3, infos[sandbox3Cgroup].Stats[0]) mockCadvisor.AssertExpectations(t) } @@ -333,9 +337,13 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) { container4 = makeFakeContainer(sandbox2, cName3, 1, false) containerStats4 = makeFakeContainerStats(container4, imageFsMountpoint) + // Running pod with a terminated container and a running container sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns", false) + sandbox3Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox2.PodSandboxStatus.Metadata.Uid)) container5 = makeFakeContainer(sandbox3, cName5, 0, true) containerStats5 = makeFakeContainerStats(container5, imageFsMountpoint) + container8 = makeFakeContainer(sandbox3, cName8, 0, false) + containerStats8 = makeFakeContainerStats(container8, imageFsMountpoint) // Terminated pod sandbox sandbox4 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", true) @@ -370,6 +378,7 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) { sandbox2.PodSandboxStatus.Id: getTestContainerInfo(seedSandbox2, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, leaky.PodInfraContainerName), sandbox2Cgroup: getTestContainerInfo(seedSandbox2, "", "", ""), container4.ContainerStatus.Id: getTestContainerInfo(seedContainer3, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, cName3), + sandbox3Cgroup: getTestContainerInfo(seedSandbox3, "", "", ""), } options := cadvisorapiv2.RequestOptions{ @@ -384,10 +393,10 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) { sandbox0, sandbox1, sandbox2, sandbox3, sandbox4, sandbox5, }) fakeRuntimeService.SetFakeContainers([]*critest.FakeContainer{ - container0, container1, container2, container3, container4, container5, container6, container7, + container0, container1, container2, container3, container4, container5, container6, container7, container8, }) fakeRuntimeService.SetFakeContainerStats([]*runtimeapi.ContainerStats{ - containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, + containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, containerStats8, }) ephemeralVolumes := makeFakeVolumeStats([]string{"ephVolume1, ephVolumes2"}) @@ -484,14 +493,12 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) { assert.Equal(sandbox3.CreatedAt, p3.StartTime.UnixNano()) assert.Equal(1, len(p3.Containers)) - c5 := p3.Containers[0] - assert.Equal(cName5, c5.Name) - assert.Equal(container5.CreatedAt, c5.StartTime.UnixNano()) - assert.NotNil(c5.CPU.Time) - assert.Zero(*c5.CPU.UsageCoreNanoSeconds) - assert.Zero(*c5.CPU.UsageNanoCores) - assert.NotNil(c5.Memory.Time) - assert.Zero(*c5.Memory.WorkingSetBytes) + c8 := p3.Containers[0] + assert.Equal(cName8, c8.Name) + assert.Equal(container8.CreatedAt, c8.StartTime.UnixNano()) + assert.NotNil(c8.CPU.Time) + assert.NotNil(c8.Memory.Time) + checkCRIPodCPUAndMemoryStats(assert, p3, infos[sandbox3Cgroup].Stats[0]) mockCadvisor.AssertExpectations(t) }