Skip to content

Commit 6fdb783

Browse files
a7ia7i
committed
skip eviction when replica count is below evictor minReplicas threshold setting
Signed-off-by: Amir Alavi <[email protected]>
1 parent 0d096ed commit 6fdb783

File tree

6 files changed

+86
-13
lines changed

6 files changed

+86
-13
lines changed

‎README.md

+2
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ The Default Evictor Plugin is used by default for filtering pods before processi
142142
|`labelSelector`|`metav1.LabelSelector`||(see [label filtering](#label-filtering))|
143143
|`priorityThreshold`|`priorityThreshold`||(see [priority filtering](#priority-filtering))|
144144
|`nodeFit`|`bool`|`false`|(see [node fit filtering](#node-fit-filtering))|
145+
|`minReplicas`|`uint`|`0`| ignore eviction of pods where owner (e.g. Deployment) replicas is below this threshold |
145146

146147
### Example policy
147148

@@ -166,6 +167,7 @@ profiles:
166167
evictFailedBarePods: true
167168
evictLocalStoragePods: true
168169
nodeFit: true
170+
minReplicas: 2
169171
plugins:
170172
# DefaultEvictor is enabled for both `filter` and `preEvictionFilter`
171173
# filter:

‎pkg/descheduler/pod/pods.go

+18
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package pod
1818

1919
import (
20+
"slices"
2021
"sort"
2122

2223
v1 "k8s.io/api/core/v1"
@@ -257,3 +258,20 @@ func SortPodsBasedOnAge(pods []*v1.Pod) {
257258
return pods[i].CreationTimestamp.Before(&pods[j].CreationTimestamp)
258259
})
259260
}
261+
262+
// GetOwnerReplicasCount returns the count of pods that match the owner ref of the given pod
263+
func GetOwnerReplicasCount(pods []*v1.Pod, pod *v1.Pod) uint {
264+
var count uint
265+
podOwnerRefs := OwnerRef(pod)
266+
for _, otherPod := range pods {
267+
otherOwnerRefs := OwnerRef(otherPod)
268+
269+
if slices.EqualFunc(podOwnerRefs, otherOwnerRefs, func(l, r metav1.OwnerReference) bool {
270+
return l.UID == r.UID
271+
}) {
272+
count++
273+
}
274+
}
275+
276+
return count
277+
}

‎pkg/framework/plugins/defaultevictor/defaultevictor.go

+15
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,21 @@ func New(args runtime.Object, handle frameworktypes.Handle) (frameworktypes.Plug
143143
})
144144
}
145145

146+
if defaultEvictorArgs.MinReplicas > 1 {
147+
podLister := handle.SharedInformerFactory().Core().V1().Pods().Lister()
148+
ev.constraints = append(ev.constraints, func(pod *v1.Pod) error {
149+
pods, err := podLister.Pods(pod.Namespace).List(labels.Everything())
150+
if err != nil {
151+
return fmt.Errorf("unable to list pods for minReplicas filter in the policy parameter")
152+
}
153+
ownerReplicas := podutil.GetOwnerReplicasCount(pods, pod)
154+
if ownerReplicas < defaultEvictorArgs.MinReplicas {
155+
return fmt.Errorf("owner has %d replicas which is less than minReplicas of %d", ownerReplicas, defaultEvictorArgs.MinReplicas)
156+
}
157+
return nil
158+
})
159+
}
160+
146161
return ev, nil
147162
}
148163

‎pkg/framework/plugins/defaultevictor/defaultevictor_test.go

+39-13
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
372372
evictSystemCriticalPods bool
373373
priorityThreshold *int32
374374
nodeFit bool
375+
minReplicas uint
375376
result bool
376377
}
377378

@@ -527,7 +528,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
527528
evictSystemCriticalPods: false,
528529
result: true,
529530
}, {
530-
description: "Pod not evicted becasuse it is part of a daemonSet",
531+
description: "Pod not evicted because it is part of a daemonSet",
531532
pods: []*v1.Pod{
532533
test.BuildTestPod("p8", 400, 0, n1.Name, func(pod *v1.Pod) {
533534
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -538,7 +539,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
538539
evictSystemCriticalPods: false,
539540
result: false,
540541
}, {
541-
description: "Pod is evicted becasuse it is part of a daemonSet, but it has scheduler.alpha.kubernetes.io/evict annotation",
542+
description: "Pod is evicted because it is part of a daemonSet, but it has scheduler.alpha.kubernetes.io/evict annotation",
542543
pods: []*v1.Pod{
543544
test.BuildTestPod("p9", 400, 0, n1.Name, func(pod *v1.Pod) {
544545
pod.Annotations = map[string]string{"descheduler.alpha.kubernetes.io/evict": "true"}
@@ -549,7 +550,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
549550
evictSystemCriticalPods: false,
550551
result: true,
551552
}, {
552-
description: "Pod not evicted becasuse it is a mirror poddsa",
553+
description: "Pod not evicted because it is a mirror poddsa",
553554
pods: []*v1.Pod{
554555
test.BuildTestPod("p10", 400, 0, n1.Name, func(pod *v1.Pod) {
555556
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -560,7 +561,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
560561
evictSystemCriticalPods: false,
561562
result: false,
562563
}, {
563-
description: "Pod is evicted becasuse it is a mirror pod, but it has scheduler.alpha.kubernetes.io/evict annotation",
564+
description: "Pod is evicted because it is a mirror pod, but it has scheduler.alpha.kubernetes.io/evict annotation",
564565
pods: []*v1.Pod{
565566
test.BuildTestPod("p11", 400, 0, n1.Name, func(pod *v1.Pod) {
566567
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -572,7 +573,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
572573
evictSystemCriticalPods: false,
573574
result: true,
574575
}, {
575-
description: "Pod not evicted becasuse it has system critical priority",
576+
description: "Pod not evicted because it has system critical priority",
576577
pods: []*v1.Pod{
577578
test.BuildTestPod("p12", 400, 0, n1.Name, func(pod *v1.Pod) {
578579
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -584,7 +585,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
584585
evictSystemCriticalPods: false,
585586
result: false,
586587
}, {
587-
description: "Pod is evicted becasuse it has system critical priority, but it has scheduler.alpha.kubernetes.io/evict annotation",
588+
description: "Pod is evicted because it has system critical priority, but it has scheduler.alpha.kubernetes.io/evict annotation",
588589
pods: []*v1.Pod{
589590
test.BuildTestPod("p13", 400, 0, n1.Name, func(pod *v1.Pod) {
590591
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -599,7 +600,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
599600
evictSystemCriticalPods: false,
600601
result: true,
601602
}, {
602-
description: "Pod not evicted becasuse it has a priority higher than the configured priority threshold",
603+
description: "Pod not evicted because it has a priority higher than the configured priority threshold",
603604
pods: []*v1.Pod{
604605
test.BuildTestPod("p14", 400, 0, n1.Name, func(pod *v1.Pod) {
605606
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -611,7 +612,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
611612
priorityThreshold: &lowPriority,
612613
result: false,
613614
}, {
614-
description: "Pod is evicted becasuse it has a priority higher than the configured priority threshold, but it has scheduler.alpha.kubernetes.io/evict annotation",
615+
description: "Pod is evicted because it has a priority higher than the configured priority threshold, but it has scheduler.alpha.kubernetes.io/evict annotation",
615616
pods: []*v1.Pod{
616617
test.BuildTestPod("p15", 400, 0, n1.Name, func(pod *v1.Pod) {
617618
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -624,7 +625,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
624625
priorityThreshold: &lowPriority,
625626
result: true,
626627
}, {
627-
description: "Pod is evicted becasuse it has system critical priority, but evictSystemCriticalPods = true",
628+
description: "Pod is evicted because it has system critical priority, but evictSystemCriticalPods = true",
628629
pods: []*v1.Pod{
629630
test.BuildTestPod("p16", 400, 0, n1.Name, func(pod *v1.Pod) {
630631
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -636,7 +637,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
636637
evictSystemCriticalPods: true,
637638
result: true,
638639
}, {
639-
description: "Pod is evicted becasuse it has system critical priority, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
640+
description: "Pod is evicted because it has system critical priority, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
640641
pods: []*v1.Pod{
641642
test.BuildTestPod("p16", 400, 0, n1.Name, func(pod *v1.Pod) {
642643
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -649,7 +650,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
649650
evictSystemCriticalPods: true,
650651
result: true,
651652
}, {
652-
description: "Pod is evicted becasuse it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true",
653+
description: "Pod is evicted because it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true",
653654
pods: []*v1.Pod{
654655
test.BuildTestPod("p17", 400, 0, n1.Name, func(pod *v1.Pod) {
655656
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -661,7 +662,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
661662
priorityThreshold: &lowPriority,
662663
result: true,
663664
}, {
664-
description: "Pod is evicted becasuse it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
665+
description: "Pod is evicted because it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
665666
pods: []*v1.Pod{
666667
test.BuildTestPod("p17", 400, 0, n1.Name, func(pod *v1.Pod) {
667668
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -704,6 +705,30 @@ func TestDefaultEvictorFilter(t *testing.T) {
704705
evictSystemCriticalPods: false,
705706
nodeFit: true,
706707
result: true,
708+
}, {
709+
description: "minReplicas of 2, owner with 2 replicas, evicts",
710+
pods: []*v1.Pod{
711+
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
712+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
713+
}),
714+
test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) {
715+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
716+
}),
717+
},
718+
minReplicas: 2,
719+
result: true,
720+
}, {
721+
description: "minReplicas of 3, owner with 2 replicas, no eviction",
722+
pods: []*v1.Pod{
723+
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
724+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
725+
}),
726+
test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) {
727+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
728+
}),
729+
},
730+
minReplicas: 3,
731+
result: false,
707732
},
708733
}
709734

@@ -741,7 +766,8 @@ func TestDefaultEvictorFilter(t *testing.T) {
741766
PriorityThreshold: &api.PriorityThreshold{
742767
Value: test.priorityThreshold,
743768
},
744-
NodeFit: test.nodeFit,
769+
NodeFit: test.nodeFit,
770+
MinReplicas: test.minReplicas,
745771
}
746772

747773
evictorPlugin, err := New(

‎pkg/framework/plugins/defaultevictor/types.go

+1
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,5 @@ type DefaultEvictorArgs struct {
3333
LabelSelector *metav1.LabelSelector `json:"labelSelector"`
3434
PriorityThreshold *api.PriorityThreshold `json:"priorityThreshold"`
3535
NodeFit bool `json:"nodeFit"`
36+
MinReplicas uint `json:"minReplicas"`
3637
}

‎test/e2e/e2e_duplicatepods_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ func TestRemoveDuplicates(t *testing.T) {
107107
replicasNum int
108108
beforeFunc func(deployment *appsv1.Deployment)
109109
expectedEvictedPodCount uint
110+
minReplicas uint
110111
}{
111112
{
112113
description: "Evict Pod even Pods schedule to specific node",
@@ -135,6 +136,15 @@ func TestRemoveDuplicates(t *testing.T) {
135136
},
136137
expectedEvictedPodCount: 2,
137138
},
139+
{
140+
description: "Ignores eviction with minReplicas of 4",
141+
replicasNum: 3,
142+
beforeFunc: func(deployment *appsv1.Deployment) {
143+
deployment.Spec.Replicas = pointer.Int32(3)
144+
},
145+
expectedEvictedPodCount: 0,
146+
minReplicas: 4,
147+
},
138148
}
139149
for _, tc := range tests {
140150
t.Run(tc.description, func(t *testing.T) {
@@ -179,6 +189,7 @@ func TestRemoveDuplicates(t *testing.T) {
179189
IgnorePvcPods: false,
180190
EvictFailedBarePods: false,
181191
NodeFit: false,
192+
MinReplicas: tc.minReplicas,
182193
}
183194

184195
evictorFilter, err := defaultevictor.New(

0 commit comments

Comments
 (0)