From df54470e9b86f3861a34f8306bd77be4374c0fb8 Mon Sep 17 00:00:00 2001 From: Kevin Torres Date: Fri, 3 Jan 2025 18:21:01 +0000 Subject: [PATCH] Test terminated pods are evicted on disk pressure --- pkg/kubelet/eviction/eviction_manager_test.go | 468 +++++++++++++++--- 1 file changed, 399 insertions(+), 69 deletions(-) diff --git a/pkg/kubelet/eviction/eviction_manager_test.go b/pkg/kubelet/eviction/eviction_manager_test.go index a72c145409b..4c8f02879bf 100644 --- a/pkg/kubelet/eviction/eviction_manager_test.go +++ b/pkg/kubelet/eviction/eviction_manager_test.go @@ -65,6 +65,36 @@ func (m *mockPodKiller) killPodNow(pod *v1.Pod, evict bool, gracePeriodOverride return nil } +// While for running pods only one pod is killed per eviction interval, +// when there is disk pressure, all terminated pods are killed in the same cycle +// mockPodsKiller is used to test which pods are killed +type mockPodsKiller struct { + pods []*v1.Pod + evict bool + statusFn func(*v1.PodStatus) + gracePeriodOverride *int64 +} + +// killPodsNow records the pods that were killed +func (m *mockPodsKiller) killPodsNow(pod *v1.Pod, evict bool, gracePeriodOverride *int64, statusFn func(*v1.PodStatus)) error { + m.pods = append(m.pods, pod) + m.statusFn = statusFn + m.evict = evict + m.gracePeriodOverride = gracePeriodOverride + return nil +} + +func setDiskStatsBasedOnFs(whichFs string, diskPressure string, diskStat diskStats) diskStats { + if whichFs == "nodefs" { + diskStat.rootFsAvailableBytes = diskPressure + } else if whichFs == "imagefs" { + diskStat.imageFsAvailableBytes = diskPressure + } else if whichFs == "containerfs" { + diskStat.containerFsAvailableBytes = diskPressure + } + return diskStat +} + // mockDiskInfoProvider is used to simulate testing. type mockDiskInfoProvider struct { dedicatedImageFs *bool @@ -296,6 +326,10 @@ func TestMemoryPressure_VerifyPodStatus(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -329,7 +363,7 @@ func TestMemoryPressure_VerifyPodStatus(t *testing.T) { } // synchronize to detect the memory pressure - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -396,6 +430,10 @@ func TestPIDPressure_VerifyPodStatus(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false), dedicatedContainerFs: ptr.To(false)} @@ -429,7 +467,7 @@ func TestPIDPressure_VerifyPodStatus(t *testing.T) { } // synchronize to detect the PID pressure - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -574,6 +612,10 @@ func TestDiskPressureNodeFs_VerifyPodStatus(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: tc.dedicatedImageFs, dedicatedContainerFs: &tc.writeableSeparateFromReadOnly} @@ -605,7 +647,7 @@ func TestDiskPressureNodeFs_VerifyPodStatus(t *testing.T) { } // synchronize - pods, synchErr := manager.synchronize(diskInfoProvider, activePodsFunc) + pods, synchErr := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if synchErr == nil && tc.expectErr != "" { t.Fatalf("Manager should report error but did not") @@ -663,6 +705,10 @@ func TestMemoryPressure(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -709,7 +755,7 @@ func TestMemoryPressure(t *testing.T) { burstablePodToAdmit, _ := podMaker("burst-admit", defaultPriority, newResourceList("100m", "100Mi", ""), newResourceList("200m", "200Mi", ""), "0Gi") // synchronize - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -731,7 +777,7 @@ func TestMemoryPressure(t *testing.T) { // induce soft threshold fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("1500Mi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -750,7 +796,7 @@ func TestMemoryPressure(t *testing.T) { // step forward in time pass the grace period fakeClock.Step(3 * time.Minute) summaryProvider.result = summaryStatsMaker("1500Mi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -779,7 +825,7 @@ func TestMemoryPressure(t *testing.T) { // remove memory pressure fakeClock.Step(20 * time.Minute) summaryProvider.result = summaryStatsMaker("3Gi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -793,7 +839,7 @@ func TestMemoryPressure(t *testing.T) { // induce memory pressure! fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("500Mi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -825,7 +871,7 @@ func TestMemoryPressure(t *testing.T) { fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("2Gi", podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -853,7 +899,7 @@ func TestMemoryPressure(t *testing.T) { fakeClock.Step(5 * time.Minute) summaryProvider.result = summaryStatsMaker("2Gi", podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -933,6 +979,10 @@ func TestPIDPressure(t *testing.T) { podToEvict := pods[tc.evictPodIndex] activePodsFunc := func() []*v1.Pod { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -979,7 +1029,7 @@ func TestPIDPressure(t *testing.T) { podToAdmit, _ := podMaker("pod-to-admit", defaultPriority, 50) // synchronize - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -998,7 +1048,7 @@ func TestPIDPressure(t *testing.T) { // induce soft threshold for PID pressure fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker(tc.totalPID, tc.pressurePIDUsageWithGracePeriod, podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1017,7 +1067,7 @@ func TestPIDPressure(t *testing.T) { // step forward in time past the grace period fakeClock.Step(3 * time.Minute) // no change in PID stats to simulate continued pressure - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1047,7 +1097,7 @@ func TestPIDPressure(t *testing.T) { // remove PID pressure by simulating increased PID availability fakeClock.Step(20 * time.Minute) summaryProvider.result = summaryStatsMaker(tc.totalPID, tc.noPressurePIDUsage, podStats) // Simulate increased PID availability - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1061,7 +1111,7 @@ func TestPIDPressure(t *testing.T) { // re-induce PID pressure fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker(tc.totalPID, tc.pressurePIDUsageWithoutGracePeriod, podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1093,7 +1143,7 @@ func TestPIDPressure(t *testing.T) { fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker(tc.totalPID, tc.noPressurePIDUsage, podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1117,7 +1167,7 @@ func TestPIDPressure(t *testing.T) { // move the clock past the transition period fakeClock.Step(5 * time.Minute) summaryProvider.result = summaryStatsMaker(tc.totalPID, tc.noPressurePIDUsage, podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1191,6 +1241,7 @@ func TestDiskPressureNodeFs(t *testing.T) { writeableSeparateFromReadOnly bool thresholdToMonitor []evictionapi.Threshold podToMakes []podToMake + terminatedPodToMakes []podToMake dedicatedImageFs *bool expectErr string inducePressureOnWhichFs string @@ -1319,6 +1370,10 @@ func TestDiskPressureNodeFs(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: tc.dedicatedImageFs, dedicatedContainerFs: &tc.writeableSeparateFromReadOnly} @@ -1356,7 +1411,7 @@ func TestDiskPressureNodeFs(t *testing.T) { podToAdmit, _ := podMaker("pod-to-admit", defaultPriority, newResourceList("", "", ""), newResourceList("", "", ""), "0Gi", "0Gi", "0Gi", nil) // synchronize - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1383,7 +1438,7 @@ func TestDiskPressureNodeFs(t *testing.T) { diskStatStart.containerFsAvailableBytes = tc.softDiskPressure } summaryProvider.result = summaryStatsMaker(diskStatStart) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1402,7 +1457,7 @@ func TestDiskPressureNodeFs(t *testing.T) { // step forward in time pass the grace period fakeClock.Step(3 * time.Minute) summaryProvider.result = summaryStatsMaker(diskStatStart) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1431,7 +1486,7 @@ func TestDiskPressureNodeFs(t *testing.T) { // remove disk pressure fakeClock.Step(20 * time.Minute) summaryProvider.result = summaryStatsMaker(diskStatConst) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1452,7 +1507,7 @@ func TestDiskPressureNodeFs(t *testing.T) { diskStatStart.containerFsAvailableBytes = tc.hardDiskPressure } summaryProvider.result = summaryStatsMaker(diskStatStart) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) @@ -1482,7 +1537,7 @@ func TestDiskPressureNodeFs(t *testing.T) { summaryProvider.result = summaryStatsMaker(diskStatConst) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1506,7 +1561,7 @@ func TestDiskPressureNodeFs(t *testing.T) { fakeClock.Step(5 * time.Minute) summaryProvider.result = summaryStatsMaker(diskStatConst) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1530,6 +1585,259 @@ func TestDiskPressureNodeFs(t *testing.T) { } } +// Test terminated pods eviction on disk pressure works as expected +func TestTerminatedPodsEvictionOnDiskPressure(t *testing.T) { + testCases := map[string]struct { + nodeFsStats string + imageFsStats string + containerFsStats string + kubeletSeparateDiskFeature bool + writeableSeparateFromReadOnly bool + thresholdToMonitor evictionapi.Threshold + podToMakes []podToMake + terminatedPodToMakes []podToMake + dedicatedImageFs *bool + inducePressureOnWhichFs string + softDiskPressure string + hardDiskPressure string + expectContainerGcCall bool + expectImageGcCall bool + }{ + "eviction due to disk pressure; terminated pods": { + dedicatedImageFs: ptr.To(false), + nodeFsStats: "16Gi", + imageFsStats: "16Gi", + containerFsStats: "16Gi", + inducePressureOnWhichFs: "nodefs", + softDiskPressure: "1.5Gi", + hardDiskPressure: "750Mi", + expectImageGcCall: true, + expectContainerGcCall: true, + thresholdToMonitor: evictionapi.Threshold{ + Signal: evictionapi.SignalNodeFsAvailable, + Operator: evictionapi.OpLessThan, + Value: evictionapi.ThresholdValue{ + Quantity: quantityMustParse("1Gi"), + }, + }, + podToMakes: []podToMake{ + {name: "low-priority-high-usage", priority: lowPriority, requests: newResourceList("100m", "1Gi", ""), limits: newResourceList("100m", "1Gi", ""), rootFsUsed: "900Mi"}, + {name: "below-requests", priority: defaultPriority, requests: newResourceList("100m", "100Mi", ""), limits: newResourceList("200m", "1Gi", ""), logsFsUsed: "50Mi"}, + {name: "above-requests", priority: defaultPriority, requests: newResourceList("100m", "100Mi", ""), limits: newResourceList("200m", "1Gi", ""), rootFsUsed: "400Mi"}, + {name: "high-priority-high-usage", priority: highPriority, requests: newResourceList("", "", ""), limits: newResourceList("", "", ""), perLocalVolumeUsed: "400Mi"}, + {name: "low-priority-low-usage", priority: lowPriority, requests: newResourceList("", "", ""), limits: newResourceList("", "", ""), rootFsUsed: "100Mi"}, + }, + terminatedPodToMakes: []podToMake{ + {name: "terminated-pod-1", priority: lowPriority, requests: newResourceList("100m", "1Gi", ""), limits: newResourceList("100m", "1Gi", ""), rootFsUsed: "500Mi"}, + {name: "terminated-pod-2", priority: lowPriority, requests: newResourceList("100m", "1Gi", ""), limits: newResourceList("100m", "1Gi", ""), rootFsUsed: "500Mi"}, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletSeparateDiskGC, tc.kubeletSeparateDiskFeature) + + podMaker := makePodWithDiskStats + summaryStatsMaker := makeDiskStats + podsToMake := tc.podToMakes + pods := []*v1.Pod{} + podStats := map[*v1.Pod]statsapi.PodStats{} + for _, podToMake := range podsToMake { + pod, podStat := podMaker(podToMake.name, podToMake.priority, podToMake.requests, podToMake.limits, podToMake.rootFsUsed, podToMake.logsFsUsed, podToMake.perLocalVolumeUsed, nil) + pods = append(pods, pod) + podStats[pod] = podStat + } + activePodsFunc := func() []*v1.Pod { + return pods + } + + terminatedPodsToMake := tc.terminatedPodToMakes + terminatedPods := []*v1.Pod{} + for _, terminatedPodToMake := range terminatedPodsToMake { + terminatedPod, _ := podMaker(terminatedPodToMake.name, terminatedPodToMake.priority, terminatedPodToMake.requests, terminatedPodToMake.limits, terminatedPodToMake.rootFsUsed, terminatedPodToMake.logsFsUsed, terminatedPodToMake.perLocalVolumeUsed, nil) + terminatedPods = append(terminatedPods, terminatedPod) + } + terminatedPodsFunc := func() []*v1.Pod { + return terminatedPods + } + + fakeClock := testingclock.NewFakeClock(time.Now()) + podKiller := &mockPodsKiller{} + diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: tc.dedicatedImageFs, dedicatedContainerFs: &tc.writeableSeparateFromReadOnly} + nodeRef := &v1.ObjectReference{Kind: "Node", Name: "test", UID: types.UID("test"), Namespace: ""} + + config := Config{ + MaxPodGracePeriodSeconds: 5, + PressureTransitionPeriod: time.Minute * 5, + Thresholds: []evictionapi.Threshold{tc.thresholdToMonitor}, + } + diskStatStart := diskStats{ + rootFsAvailableBytes: tc.nodeFsStats, + imageFsAvailableBytes: tc.imageFsStats, + containerFsAvailableBytes: tc.containerFsStats, + podStats: podStats, + } + // This is a constant that we use to test that disk pressure is over. Don't change! + diskStatConst := diskStatStart + summaryProvider := &fakeSummaryProvider{result: summaryStatsMaker(diskStatStart)} + diskGC := &mockDiskGC{fakeSummaryProvider: summaryProvider, err: nil} + manager := &managerImpl{ + clock: fakeClock, + killPodFunc: podKiller.killPodsNow, + imageGC: diskGC, + containerGC: diskGC, + config: config, + recorder: &record.FakeRecorder{}, + summaryProvider: summaryProvider, + nodeRef: nodeRef, + nodeConditionsLastObservedAt: nodeConditionsObservedAt{}, + thresholdsFirstObservedAt: thresholdsObservedAt{}, + } + + // synchronize + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) + + if err != nil { + t.Fatalf("Manager should not have an error %v", err) + } + + // we should not have disk pressure + if manager.IsUnderDiskPressure() { + t.Errorf("Manager should not report disk pressure") + } + + // induce hard threshold + fakeClock.Step(1 * time.Minute) + + newDiskAfterHardEviction := setDiskStatsBasedOnFs(tc.inducePressureOnWhichFs, tc.hardDiskPressure, diskStatStart) + summaryProvider.result = summaryStatsMaker(newDiskAfterHardEviction) + // make GC successfully return disk usage to previous levels + diskGC.summaryAfterGC = summaryStatsMaker(diskStatConst) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) + + if err != nil { + t.Fatalf("Manager should not have an error %v", err) + } + + // we should have disk pressure + if !manager.IsUnderDiskPressure() { + t.Fatalf("Manager should report disk pressure since soft threshold was met") + } + + // verify image, container or both gc were called. + // split filesystem can have container gc called without image. + // same filesystem should have both. + if diskGC.imageGCInvoked != tc.expectImageGcCall && diskGC.containerGCInvoked != tc.expectContainerGcCall { + t.Fatalf("Manager should have invoked image gc") + } + + // compare expected killed pods with actual killed pods + checkIfSamePods := func(expectedPods, actualPods []*v1.Pod) bool { + expected := make(map[*v1.Pod]bool) + for _, pod := range expectedPods { + expected[pod] = true + } + + for _, pod := range actualPods { + if !expected[pod] { + return false + } + } + + return true + } + + // verify terminated pods were killed + if podKiller.pods == nil { + t.Fatalf("Manager should have killed terminated pods") + } + + // verify only terminated pods were killed because image gc was sufficient + if !checkIfSamePods(terminatedPods, podKiller.pods) { + t.Fatalf("Manager killed running pods") + } + + // reset state + diskGC.imageGCInvoked = false + diskGC.containerGCInvoked = false + podKiller.pods = nil + + // remove disk pressure + fakeClock.Step(20 * time.Minute) + summaryProvider.result = summaryStatsMaker(diskStatConst) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) + + if err != nil { + t.Fatalf("Manager should not have an error %v", err) + } + + // we should not have disk pressure + if manager.IsUnderDiskPressure() { + t.Fatalf("Manager should not report disk pressure") + } + + // induce disk pressure! + fakeClock.Step(1 * time.Minute) + softDiskPressure := setDiskStatsBasedOnFs(tc.inducePressureOnWhichFs, tc.hardDiskPressure, diskStatStart) + summaryProvider.result = summaryStatsMaker(softDiskPressure) + // Don't reclaim any disk + diskGC.summaryAfterGC = summaryStatsMaker(softDiskPressure) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) + + if err != nil { + t.Fatalf("Manager should not have an error %v", err) + } + + // we should have disk pressure + if !manager.IsUnderDiskPressure() { + t.Fatalf("Manager should report disk pressure") + } + + // verify image, container or both gc were called. + // split filesystem can have container gc called without image. + // same filesystem should have both. + if diskGC.imageGCInvoked != tc.expectImageGcCall && diskGC.containerGCInvoked != tc.expectContainerGcCall { + t.Fatalf("Manager should have invoked image gc") + } + + // Make array with only the pods names + podsNames := func(pods []*v1.Pod) []string { + var podsNames []string + for _, pod := range pods { + podsNames = append(podsNames, pod.Name) + } + return podsNames + } + + expectedKilledPods := append(terminatedPods, pods[0]) + // verify only terminated pods were killed because image gc was sufficient + if !checkIfSamePods(expectedKilledPods, podKiller.pods) { + t.Fatalf("Manager killed running pods. Expected: %v, actual: %v", podsNames(expectedKilledPods), podsNames(podKiller.pods)) + } + + // reset state + diskGC.imageGCInvoked = false + diskGC.containerGCInvoked = false + podKiller.pods = nil + + // remove disk pressure + fakeClock.Step(20 * time.Minute) + summaryProvider.result = summaryStatsMaker(diskStatConst) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) + + if err != nil { + t.Fatalf("Manager should not have an error %v", err) + } + + // we should not have disk pressure + if manager.IsUnderDiskPressure() { + t.Fatalf("Manager should not report disk pressure") + } + }) + } +} + // TestMinReclaim verifies that min-reclaim works as desired. func TestMinReclaim(t *testing.T) { podMaker := makePodWithMemoryStats @@ -1553,6 +1861,10 @@ func TestMinReclaim(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -1590,7 +1902,7 @@ func TestMinReclaim(t *testing.T) { } // synchronize - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Errorf("Manager should not report any errors") } @@ -1602,7 +1914,7 @@ func TestMinReclaim(t *testing.T) { // induce memory pressure! fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("500Mi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1626,7 +1938,7 @@ func TestMinReclaim(t *testing.T) { fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("1.2Gi", podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1650,7 +1962,7 @@ func TestMinReclaim(t *testing.T) { fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("2Gi", podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1670,7 +1982,7 @@ func TestMinReclaim(t *testing.T) { fakeClock.Step(5 * time.Minute) summaryProvider.result = summaryStatsMaker("2Gi", podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1841,6 +2153,10 @@ func TestNodeReclaimFuncs(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: tc.dedicatedImageFs, dedicatedContainerFs: &tc.writeableSeparateFromReadOnly} @@ -1875,7 +2191,7 @@ func TestNodeReclaimFuncs(t *testing.T) { } // synchronize - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1889,21 +2205,11 @@ func TestNodeReclaimFuncs(t *testing.T) { // induce hard threshold fakeClock.Step(1 * time.Minute) - setDiskStatsBasedOnFs := func(whichFs string, diskPressure string, diskStat diskStats) diskStats { - if tc.inducePressureOnWhichFs == "nodefs" { - diskStat.rootFsAvailableBytes = diskPressure - } else if tc.inducePressureOnWhichFs == "imagefs" { - diskStat.imageFsAvailableBytes = diskPressure - } else if tc.inducePressureOnWhichFs == "containerfs" { - diskStat.containerFsAvailableBytes = diskPressure - } - return diskStat - } newDiskAfterHardEviction := setDiskStatsBasedOnFs(tc.inducePressureOnWhichFs, tc.hardDiskPressure, diskStatStart) summaryProvider.result = summaryStatsMaker(newDiskAfterHardEviction) // make GC successfully return disk usage to previous levels diskGC.summaryAfterGC = summaryStatsMaker(diskStatConst) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1933,7 +2239,7 @@ func TestNodeReclaimFuncs(t *testing.T) { // remove disk pressure fakeClock.Step(20 * time.Minute) summaryProvider.result = summaryStatsMaker(diskStatConst) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1945,7 +2251,7 @@ func TestNodeReclaimFuncs(t *testing.T) { } // synchronize - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1963,7 +2269,7 @@ func TestNodeReclaimFuncs(t *testing.T) { // make GC return disk usage bellow the threshold, but not satisfying minReclaim gcBelowThreshold := setDiskStatsBasedOnFs(tc.inducePressureOnWhichFs, "1.1G", newDiskAfterHardEviction) diskGC.summaryAfterGC = summaryStatsMaker(gcBelowThreshold) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -1994,7 +2300,7 @@ func TestNodeReclaimFuncs(t *testing.T) { // remove disk pressure fakeClock.Step(20 * time.Minute) summaryProvider.result = summaryStatsMaker(diskStatConst) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2011,7 +2317,7 @@ func TestNodeReclaimFuncs(t *testing.T) { summaryProvider.result = summaryStatsMaker(softDiskPressure) // Don't reclaim any disk diskGC.summaryAfterGC = summaryStatsMaker(softDiskPressure) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2044,7 +2350,7 @@ func TestNodeReclaimFuncs(t *testing.T) { diskGC.imageGCInvoked = false // reset state diskGC.containerGCInvoked = false // reset state podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2070,7 +2376,7 @@ func TestNodeReclaimFuncs(t *testing.T) { diskGC.imageGCInvoked = false // reset state diskGC.containerGCInvoked = false // reset state podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2304,6 +2610,10 @@ func TestInodePressureFsInodes(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: tc.dedicatedImageFs, dedicatedContainerFs: &tc.writeableSeparateFromReadOnly} @@ -2335,7 +2645,7 @@ func TestInodePressureFsInodes(t *testing.T) { podToAdmit, _ := podMaker("pod-to-admit", defaultPriority, newResourceList("", "", ""), newResourceList("", "", ""), "0", "0", "0") // synchronize - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2354,7 +2664,7 @@ func TestInodePressureFsInodes(t *testing.T) { // induce soft threshold fakeClock.Step(1 * time.Minute) summaryProvider.result = setINodesFreeBasedOnFs(tc.inducePressureOnWhichFs, tc.softINodePressure, startingStatsModified) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2373,7 +2683,7 @@ func TestInodePressureFsInodes(t *testing.T) { // step forward in time pass the grace period fakeClock.Step(3 * time.Minute) summaryProvider.result = setINodesFreeBasedOnFs(tc.inducePressureOnWhichFs, tc.softINodePressure, startingStatsModified) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2402,7 +2712,7 @@ func TestInodePressureFsInodes(t *testing.T) { // remove inode pressure fakeClock.Step(20 * time.Minute) summaryProvider.result = startingStatsConst - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2416,7 +2726,7 @@ func TestInodePressureFsInodes(t *testing.T) { // induce inode pressure! fakeClock.Step(1 * time.Minute) summaryProvider.result = setINodesFreeBasedOnFs(tc.inducePressureOnWhichFs, tc.hardINodePressure, startingStatsModified) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2445,7 +2755,7 @@ func TestInodePressureFsInodes(t *testing.T) { fakeClock.Step(1 * time.Minute) summaryProvider.result = startingStatsConst podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2470,7 +2780,7 @@ func TestInodePressureFsInodes(t *testing.T) { fakeClock.Step(5 * time.Minute) summaryProvider.result = startingStatsConst podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2522,6 +2832,10 @@ func TestStaticCriticalPodsAreNotEvicted(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -2567,7 +2881,7 @@ func TestStaticCriticalPodsAreNotEvicted(t *testing.T) { fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("1500Mi", podStats) - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2586,7 +2900,7 @@ func TestStaticCriticalPodsAreNotEvicted(t *testing.T) { // step forward in time pass the grace period fakeClock.Step(3 * time.Minute) summaryProvider.result = summaryStatsMaker("1500Mi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2608,7 +2922,7 @@ func TestStaticCriticalPodsAreNotEvicted(t *testing.T) { // remove memory pressure fakeClock.Step(20 * time.Minute) summaryProvider.result = summaryStatsMaker("3Gi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2628,7 +2942,7 @@ func TestStaticCriticalPodsAreNotEvicted(t *testing.T) { // induce memory pressure! fakeClock.Step(1 * time.Minute) summaryProvider.result = summaryStatsMaker("500Mi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2685,6 +2999,10 @@ func TestStorageLimitEvictions(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -2727,7 +3045,7 @@ func TestStorageLimitEvictions(t *testing.T) { localStorageCapacityIsolation: true, } - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager expects no error but got %v", err) } @@ -2768,6 +3086,10 @@ func TestAllocatableMemoryPressure(t *testing.T) { return pods } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -2806,7 +3128,7 @@ func TestAllocatableMemoryPressure(t *testing.T) { burstablePodToAdmit, _ := podMaker("burst-admit", defaultPriority, newResourceList("100m", "100Mi", ""), newResourceList("200m", "200Mi", ""), "0Gi") // synchronize - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2830,7 +3152,7 @@ func TestAllocatableMemoryPressure(t *testing.T) { pod, podStat := podMaker("guaranteed-high-2", defaultPriority, newResourceList("100m", "1Gi", ""), newResourceList("100m", "1Gi", ""), "1Gi") podStats[pod] = podStat summaryProvider.result = summaryStatsMaker("500Mi", podStats) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2870,7 +3192,7 @@ func TestAllocatableMemoryPressure(t *testing.T) { } summaryProvider.result = summaryStatsMaker("2Gi", podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2898,7 +3220,7 @@ func TestAllocatableMemoryPressure(t *testing.T) { fakeClock.Step(5 * time.Minute) summaryProvider.result = summaryStatsMaker("2Gi", podStats) podKiller.pod = nil // reset state - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2928,6 +3250,10 @@ func TestUpdateMemcgThreshold(t *testing.T) { return []*v1.Pod{} } + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + fakeClock := testingclock.NewFakeClock(time.Now()) podKiller := &mockPodKiller{} diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: ptr.To(false)} @@ -2968,14 +3294,14 @@ func TestUpdateMemcgThreshold(t *testing.T) { } // The UpdateThreshold method should have been called once, since this is the first run. - _, err := manager.synchronize(diskInfoProvider, activePodsFunc) + _, err := manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) } // The UpdateThreshold method should not have been called again, since not enough time has passed - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2983,7 +3309,7 @@ func TestUpdateMemcgThreshold(t *testing.T) { // The UpdateThreshold method should be called again since enough time has passed fakeClock.Step(2 * notifierRefreshInterval) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -2998,7 +3324,7 @@ func TestUpdateMemcgThreshold(t *testing.T) { // The UpdateThreshold method should be called because at least notifierRefreshInterval time has passed. // The Description method should be called because UpdateThreshold returned an error fakeClock.Step(2 * notifierRefreshInterval) - _, err = manager.synchronize(diskInfoProvider, activePodsFunc) + _, err = manager.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have an error %v", err) @@ -3066,7 +3392,11 @@ func TestManagerWithLocalStorageCapacityIsolationOpen(t *testing.T) { return pods } - evictedPods, err := mgr.synchronize(diskInfoProvider, activePodsFunc) + terminatedPodsFunc := func() []*v1.Pod { + return []*v1.Pod{} + } + + evictedPods, err := mgr.synchronize(diskInfoProvider, activePodsFunc, terminatedPodsFunc) if err != nil { t.Fatalf("Manager should not have error but got %v", err)