Skip to content

Commit a931227

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#83504 from lyft/remove-all-terminated-containers
cri_stats_provider: do not consider exited containers when calculating cpu usage
2 parents a13ddd5 + 4b339e6 commit a931227

File tree

2 files changed

+32
-32
lines changed

2 files changed

+32
-32
lines changed

‎pkg/kubelet/stats/cri_stats_provider.go‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -734,9 +734,8 @@ func removeTerminatedPods(pods []*runtimeapi.PodSandbox) []*runtimeapi.PodSandbo
734734
return result
735735
}
736736

737-
// removeTerminatedContainers returns containers with terminated ones.
738-
// It only removes a terminated container when there is a running instance
739-
// of the container.
737+
// removeTerminatedContainers removes all terminated containers since they should
738+
// not be used for usage calculations.
740739
func removeTerminatedContainers(containers []*runtimeapi.Container) []*runtimeapi.Container {
741740
containerMap := make(map[containerID][]*runtimeapi.Container)
742741
// Sort order by create time
@@ -753,20 +752,11 @@ func removeTerminatedContainers(containers []*runtimeapi.Container) []*runtimeap
753752

754753
result := make([]*runtimeapi.Container, 0)
755754
for _, refs := range containerMap {
756-
if len(refs) == 1 {
757-
result = append(result, refs[0])
758-
continue
759-
}
760-
found := false
761755
for i := 0; i < len(refs); i++ {
762756
if refs[i].State == runtimeapi.ContainerState_CONTAINER_RUNNING {
763-
found = true
764757
result = append(result, refs[i])
765758
}
766759
}
767-
if !found {
768-
result = append(result, refs[len(refs)-1])
769-
}
770760
}
771761
return result
772762
}

‎pkg/kubelet/stats/cri_stats_provider_test.go‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ const (
6161
seedContainer2 = 5000
6262
seedSandbox2 = 6000
6363
seedContainer3 = 7000
64+
seedSandbox3 = 8000
6465
)
6566

6667
const (
@@ -77,6 +78,7 @@ const (
7778
cName5 = "container5-name"
7879
cName6 = "container6-name"
7980
cName7 = "container7-name"
81+
cName8 = "container8-name"
8082
)
8183

8284
func TestCRIListPodStats(t *testing.T) {
@@ -109,10 +111,15 @@ func TestCRIListPodStats(t *testing.T) {
109111
containerStats4 = makeFakeContainerStats(container4, imageFsMountpoint)
110112
containerLogStats4 = makeFakeLogStats(4000)
111113

114+
// Running pod with a terminated container and a running container
112115
sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns", false)
116+
sandbox3Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox3.PodSandboxStatus.Metadata.Uid))
113117
container5 = makeFakeContainer(sandbox3, cName5, 0, true)
114118
containerStats5 = makeFakeContainerStats(container5, imageFsMountpoint)
115119
containerLogStats5 = makeFakeLogStats(5000)
120+
container8 = makeFakeContainer(sandbox3, cName8, 0, false)
121+
containerStats8 = makeFakeContainerStats(container8, imageFsMountpoint)
122+
containerLogStats8 = makeFakeLogStats(6000)
116123

117124
// Terminated pod sandbox
118125
sandbox4 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", true)
@@ -153,6 +160,7 @@ func TestCRIListPodStats(t *testing.T) {
153160
sandbox2.PodSandboxStatus.Id: getTestContainerInfo(seedSandbox2, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, leaky.PodInfraContainerName),
154161
sandbox2Cgroup: getTestContainerInfo(seedSandbox2, "", "", ""),
155162
container4.ContainerStatus.Id: getTestContainerInfo(seedContainer3, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, cName3),
163+
sandbox3Cgroup: getTestContainerInfo(seedSandbox3, "", "", ""),
156164
}
157165

158166
options := cadvisorapiv2.RequestOptions{
@@ -170,10 +178,10 @@ func TestCRIListPodStats(t *testing.T) {
170178
sandbox0, sandbox1, sandbox2, sandbox3, sandbox4, sandbox5,
171179
})
172180
fakeRuntimeService.SetFakeContainers([]*critest.FakeContainer{
173-
container0, container1, container2, container3, container4, container5, container6, container7,
181+
container0, container1, container2, container3, container4, container5, container6, container7, container8,
174182
})
175183
fakeRuntimeService.SetFakeContainerStats([]*runtimeapi.ContainerStats{
176-
containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7,
184+
containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, containerStats8,
177185
})
178186

179187
ephemeralVolumes := makeFakeVolumeStats([]string{"ephVolume1, ephVolumes2"})
@@ -189,6 +197,7 @@ func TestCRIListPodStats(t *testing.T) {
189197
kuberuntime.BuildContainerLogsDirectory("sandbox1-ns", "sandbox1-name", types.UID("sandbox1-uid"), cName2): containerLogStats2,
190198
kuberuntime.BuildContainerLogsDirectory("sandbox2-ns", "sandbox2-name", types.UID("sandbox2-uid"), cName3): containerLogStats4,
191199
kuberuntime.BuildContainerLogsDirectory("sandbox3-ns", "sandbox3-name", types.UID("sandbox3-uid"), cName5): containerLogStats5,
200+
kuberuntime.BuildContainerLogsDirectory("sandbox3-ns", "sandbox3-name", types.UID("sandbox3-uid"), cName8): containerLogStats8,
192201
filepath.Join(kuberuntime.BuildPodLogsDirectory("sandbox0-ns", "sandbox0-name", types.UID("sandbox0-uid")), podLogName0): podLogStats0,
193202
filepath.Join(kuberuntime.BuildPodLogsDirectory("sandbox1-ns", "sandbox1-name", types.UID("sandbox1-uid")), podLogName1): podLogStats1,
194203
}
@@ -296,14 +305,12 @@ func TestCRIListPodStats(t *testing.T) {
296305
assert.Equal(sandbox3.CreatedAt, p3.StartTime.UnixNano())
297306
assert.Equal(1, len(p3.Containers))
298307

299-
c5 := p3.Containers[0]
300-
assert.Equal(cName5, c5.Name)
301-
assert.Equal(container5.CreatedAt, c5.StartTime.UnixNano())
302-
assert.NotNil(c5.CPU.Time)
303-
assert.Zero(*c5.CPU.UsageCoreNanoSeconds)
304-
assert.Zero(*c5.CPU.UsageNanoCores)
305-
assert.NotNil(c5.Memory.Time)
306-
assert.Zero(*c5.Memory.WorkingSetBytes)
308+
c8 := p3.Containers[0]
309+
assert.Equal(cName8, c8.Name)
310+
assert.Equal(container8.CreatedAt, c8.StartTime.UnixNano())
311+
assert.NotNil(c8.CPU.Time)
312+
assert.NotNil(c8.Memory.Time)
313+
checkCRIPodCPUAndMemoryStats(assert, p3, infos[sandbox3Cgroup].Stats[0])
307314

308315
mockCadvisor.AssertExpectations(t)
309316
}
@@ -333,9 +340,13 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) {
333340
container4 = makeFakeContainer(sandbox2, cName3, 1, false)
334341
containerStats4 = makeFakeContainerStats(container4, imageFsMountpoint)
335342

343+
// Running pod with a terminated container and a running container
336344
sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns", false)
345+
sandbox3Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox3.PodSandboxStatus.Metadata.Uid))
337346
container5 = makeFakeContainer(sandbox3, cName5, 0, true)
338347
containerStats5 = makeFakeContainerStats(container5, imageFsMountpoint)
348+
container8 = makeFakeContainer(sandbox3, cName8, 0, false)
349+
containerStats8 = makeFakeContainerStats(container8, imageFsMountpoint)
339350

340351
// Terminated pod sandbox
341352
sandbox4 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", true)
@@ -370,6 +381,7 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) {
370381
sandbox2.PodSandboxStatus.Id: getTestContainerInfo(seedSandbox2, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, leaky.PodInfraContainerName),
371382
sandbox2Cgroup: getTestContainerInfo(seedSandbox2, "", "", ""),
372383
container4.ContainerStatus.Id: getTestContainerInfo(seedContainer3, pName2, sandbox2.PodSandboxStatus.Metadata.Namespace, cName3),
384+
sandbox3Cgroup: getTestContainerInfo(seedSandbox3, "", "", ""),
373385
}
374386

375387
options := cadvisorapiv2.RequestOptions{
@@ -384,10 +396,10 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) {
384396
sandbox0, sandbox1, sandbox2, sandbox3, sandbox4, sandbox5,
385397
})
386398
fakeRuntimeService.SetFakeContainers([]*critest.FakeContainer{
387-
container0, container1, container2, container3, container4, container5, container6, container7,
399+
container0, container1, container2, container3, container4, container5, container6, container7, container8,
388400
})
389401
fakeRuntimeService.SetFakeContainerStats([]*runtimeapi.ContainerStats{
390-
containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7,
402+
containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, containerStats8,
391403
})
392404

393405
ephemeralVolumes := makeFakeVolumeStats([]string{"ephVolume1, ephVolumes2"})
@@ -484,14 +496,12 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) {
484496
assert.Equal(sandbox3.CreatedAt, p3.StartTime.UnixNano())
485497
assert.Equal(1, len(p3.Containers))
486498

487-
c5 := p3.Containers[0]
488-
assert.Equal(cName5, c5.Name)
489-
assert.Equal(container5.CreatedAt, c5.StartTime.UnixNano())
490-
assert.NotNil(c5.CPU.Time)
491-
assert.Zero(*c5.CPU.UsageCoreNanoSeconds)
492-
assert.Zero(*c5.CPU.UsageNanoCores)
493-
assert.NotNil(c5.Memory.Time)
494-
assert.Zero(*c5.Memory.WorkingSetBytes)
499+
c8 := p3.Containers[0]
500+
assert.Equal(cName8, c8.Name)
501+
assert.Equal(container8.CreatedAt, c8.StartTime.UnixNano())
502+
assert.NotNil(c8.CPU.Time)
503+
assert.NotNil(c8.Memory.Time)
504+
checkCRIPodCPUAndMemoryStats(assert, p3, infos[sandbox3Cgroup].Stats[0])
495505

496506
mockCadvisor.AssertExpectations(t)
497507
}

0 commit comments

Comments
 (0)