Skip to content

Commit 3284a8e

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

File tree

7 files changed

+173
-26
lines changed

7 files changed

+173
-26
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. `ReplicaSet`) 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

+41-11
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"
@@ -142,21 +143,25 @@ func BuildGetPodsAssignedToNodeFunc(podInformer cache.SharedIndexInformer) (GetP
142143
if err != nil {
143144
return nil, err
144145
}
145-
pods := make([]*v1.Pod, 0, len(objs))
146-
for _, obj := range objs {
147-
pod, ok := obj.(*v1.Pod)
148-
if !ok {
149-
continue
150-
}
151-
if filter(pod) {
152-
pods = append(pods, pod)
153-
}
154-
}
155-
return pods, nil
146+
return ConvertToPods(objs, filter), nil
156147
}
157148
return getPodsAssignedToNode, nil
158149
}
159150

151+
func ConvertToPods(objs []interface{}, filter FilterFunc) []*v1.Pod {
152+
pods := make([]*v1.Pod, 0, len(objs))
153+
for _, obj := range objs {
154+
pod, ok := obj.(*v1.Pod)
155+
if !ok {
156+
continue
157+
}
158+
if filter == nil || filter(pod) {
159+
pods = append(pods, pod)
160+
}
161+
}
162+
return pods
163+
}
164+
160165
// ListPodsOnNodes returns all pods on given nodes.
161166
func ListPodsOnNodes(nodes []*v1.Node, getPodsAssignedToNode GetPodsAssignedToNodeFunc, filter FilterFunc) ([]*v1.Pod, error) {
162167
pods := make([]*v1.Pod, 0)
@@ -215,6 +220,14 @@ func OwnerRef(pod *v1.Pod) []metav1.OwnerReference {
215220
return pod.ObjectMeta.GetOwnerReferences()
216221
}
217222

223+
func OwnerRefUIDs(pod *v1.Pod) []string {
224+
var ownerRefUIDs []string
225+
for _, ownerRef := range OwnerRef(pod) {
226+
ownerRefUIDs = append(ownerRefUIDs, string(ownerRef.UID))
227+
}
228+
return ownerRefUIDs
229+
}
230+
218231
func IsBestEffortPod(pod *v1.Pod) bool {
219232
return utils.GetPodQOS(pod) == v1.PodQOSBestEffort
220233
}
@@ -266,3 +279,20 @@ func GroupByNodeName(pods []*v1.Pod) map[string][]*v1.Pod {
266279
}
267280
return m
268281
}
282+
283+
// GetOwnerReplicasCount returns the count of pods that match the owner ref of the given pod
284+
func GetOwnerReplicasCount(pods []*v1.Pod, pod *v1.Pod) uint {
285+
var count uint
286+
podOwnerRefs := OwnerRef(pod)
287+
for _, otherPod := range pods {
288+
otherOwnerRefs := OwnerRef(otherPod)
289+
290+
if slices.EqualFunc(podOwnerRefs, otherOwnerRefs, func(l, r metav1.OwnerReference) bool {
291+
return l.UID == r.UID
292+
}) {
293+
count++
294+
}
295+
}
296+
297+
return count
298+
}

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

+53-2
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ package defaultevictor
1616
import (
1717
// "context"
1818
"context"
19+
"errors"
1920
"fmt"
2021

2122
v1 "k8s.io/api/core/v1"
2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2324
"k8s.io/apimachinery/pkg/labels"
2425
"k8s.io/apimachinery/pkg/runtime"
25-
"k8s.io/apimachinery/pkg/util/errors"
26+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
27+
"k8s.io/client-go/tools/cache"
2628
"k8s.io/klog/v2"
2729
nodeutil "sigs.k8s.io/descheduler/pkg/descheduler/node"
2830
podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
@@ -143,6 +145,36 @@ func New(args runtime.Object, handle frameworktypes.Handle) (frameworktypes.Plug
143145
})
144146
}
145147

148+
if defaultEvictorArgs.MinReplicas > 1 {
149+
indexName := "metadata.ownerReferences"
150+
indexer, err := getPodIndexerByOwnerRefs(indexName, handle)
151+
if err != nil {
152+
return nil, err
153+
}
154+
ev.constraints = append(ev.constraints, func(pod *v1.Pod) error {
155+
if len(pod.OwnerReferences) == 0 {
156+
return nil
157+
}
158+
159+
if len(pod.OwnerReferences) > 1 {
160+
klog.V(5).InfoS("pod has multiple owner references which is not supported for minReplicas check", "size", len(pod.OwnerReferences), "pod", klog.KObj(pod))
161+
return nil
162+
}
163+
164+
ownerRef := pod.OwnerReferences[0]
165+
objs, err := indexer.ByIndex(indexName, string(ownerRef.UID))
166+
if err != nil {
167+
return fmt.Errorf("unable to list pods for minReplicas filter in the policy parameter")
168+
}
169+
170+
if uint(len(objs)) < defaultEvictorArgs.MinReplicas {
171+
return fmt.Errorf("owner has %d replicas which is less than minReplicas of %d", len(objs), defaultEvictorArgs.MinReplicas)
172+
}
173+
174+
return nil
175+
})
176+
}
177+
146178
return ev, nil
147179
}
148180

@@ -199,9 +231,28 @@ func (d *DefaultEvictor) Filter(pod *v1.Pod) bool {
199231
}
200232

201233
if len(checkErrs) > 0 {
202-
klog.V(4).InfoS("Pod fails the following checks", "pod", klog.KObj(pod), "checks", errors.NewAggregate(checkErrs).Error())
234+
klog.V(4).InfoS("Pod fails the following checks", "pod", klog.KObj(pod), "checks", utilerrors.NewAggregate(checkErrs).Error())
203235
return false
204236
}
205237

206238
return true
207239
}
240+
241+
func getPodIndexerByOwnerRefs(indexName string, handle frameworktypes.Handle) (cache.Indexer, error) {
242+
podInformer := handle.SharedInformerFactory().Core().V1().Pods().Informer()
243+
if err := podInformer.AddIndexers(cache.Indexers{
244+
indexName: func(obj interface{}) ([]string, error) {
245+
pod, ok := obj.(*v1.Pod)
246+
if !ok {
247+
return []string{}, errors.New("unexpected object")
248+
}
249+
250+
return podutil.OwnerRefUIDs(pod), nil
251+
},
252+
}); err != nil {
253+
return nil, err
254+
}
255+
256+
indexer := podInformer.GetIndexer()
257+
return indexer, nil
258+
}

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

+59-13
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
v1 "k8s.io/api/core/v1"
2121
"k8s.io/apimachinery/pkg/api/resource"
2222
"k8s.io/apimachinery/pkg/runtime"
23+
"k8s.io/apimachinery/pkg/util/uuid"
2324
"k8s.io/client-go/informers"
2425
"k8s.io/client-go/kubernetes/fake"
2526
"sigs.k8s.io/descheduler/pkg/api"
@@ -363,6 +364,8 @@ func TestDefaultEvictorFilter(t *testing.T) {
363364
nodeTaintKey := "hardware"
364365
nodeTaintValue := "gpu"
365366

367+
ownerRefUUID := uuid.NewUUID()
368+
366369
type testCase struct {
367370
description string
368371
pods []*v1.Pod
@@ -372,6 +375,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
372375
evictSystemCriticalPods bool
373376
priorityThreshold *int32
374377
nodeFit bool
378+
minReplicas uint
375379
result bool
376380
}
377381

@@ -527,7 +531,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
527531
evictSystemCriticalPods: false,
528532
result: true,
529533
}, {
530-
description: "Pod not evicted becasuse it is part of a daemonSet",
534+
description: "Pod not evicted because it is part of a daemonSet",
531535
pods: []*v1.Pod{
532536
test.BuildTestPod("p8", 400, 0, n1.Name, func(pod *v1.Pod) {
533537
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -538,7 +542,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
538542
evictSystemCriticalPods: false,
539543
result: false,
540544
}, {
541-
description: "Pod is evicted becasuse it is part of a daemonSet, but it has scheduler.alpha.kubernetes.io/evict annotation",
545+
description: "Pod is evicted because it is part of a daemonSet, but it has scheduler.alpha.kubernetes.io/evict annotation",
542546
pods: []*v1.Pod{
543547
test.BuildTestPod("p9", 400, 0, n1.Name, func(pod *v1.Pod) {
544548
pod.Annotations = map[string]string{"descheduler.alpha.kubernetes.io/evict": "true"}
@@ -549,7 +553,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
549553
evictSystemCriticalPods: false,
550554
result: true,
551555
}, {
552-
description: "Pod not evicted becasuse it is a mirror poddsa",
556+
description: "Pod not evicted because it is a mirror poddsa",
553557
pods: []*v1.Pod{
554558
test.BuildTestPod("p10", 400, 0, n1.Name, func(pod *v1.Pod) {
555559
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -560,7 +564,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
560564
evictSystemCriticalPods: false,
561565
result: false,
562566
}, {
563-
description: "Pod is evicted becasuse it is a mirror pod, but it has scheduler.alpha.kubernetes.io/evict annotation",
567+
description: "Pod is evicted because it is a mirror pod, but it has scheduler.alpha.kubernetes.io/evict annotation",
564568
pods: []*v1.Pod{
565569
test.BuildTestPod("p11", 400, 0, n1.Name, func(pod *v1.Pod) {
566570
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -572,7 +576,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
572576
evictSystemCriticalPods: false,
573577
result: true,
574578
}, {
575-
description: "Pod not evicted becasuse it has system critical priority",
579+
description: "Pod not evicted because it has system critical priority",
576580
pods: []*v1.Pod{
577581
test.BuildTestPod("p12", 400, 0, n1.Name, func(pod *v1.Pod) {
578582
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -584,7 +588,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
584588
evictSystemCriticalPods: false,
585589
result: false,
586590
}, {
587-
description: "Pod is evicted becasuse it has system critical priority, but it has scheduler.alpha.kubernetes.io/evict annotation",
591+
description: "Pod is evicted because it has system critical priority, but it has scheduler.alpha.kubernetes.io/evict annotation",
588592
pods: []*v1.Pod{
589593
test.BuildTestPod("p13", 400, 0, n1.Name, func(pod *v1.Pod) {
590594
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -599,7 +603,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
599603
evictSystemCriticalPods: false,
600604
result: true,
601605
}, {
602-
description: "Pod not evicted becasuse it has a priority higher than the configured priority threshold",
606+
description: "Pod not evicted because it has a priority higher than the configured priority threshold",
603607
pods: []*v1.Pod{
604608
test.BuildTestPod("p14", 400, 0, n1.Name, func(pod *v1.Pod) {
605609
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -611,7 +615,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
611615
priorityThreshold: &lowPriority,
612616
result: false,
613617
}, {
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",
618+
description: "Pod is evicted because it has a priority higher than the configured priority threshold, but it has scheduler.alpha.kubernetes.io/evict annotation",
615619
pods: []*v1.Pod{
616620
test.BuildTestPod("p15", 400, 0, n1.Name, func(pod *v1.Pod) {
617621
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -624,7 +628,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
624628
priorityThreshold: &lowPriority,
625629
result: true,
626630
}, {
627-
description: "Pod is evicted becasuse it has system critical priority, but evictSystemCriticalPods = true",
631+
description: "Pod is evicted because it has system critical priority, but evictSystemCriticalPods = true",
628632
pods: []*v1.Pod{
629633
test.BuildTestPod("p16", 400, 0, n1.Name, func(pod *v1.Pod) {
630634
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -636,7 +640,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
636640
evictSystemCriticalPods: true,
637641
result: true,
638642
}, {
639-
description: "Pod is evicted becasuse it has system critical priority, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
643+
description: "Pod is evicted because it has system critical priority, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
640644
pods: []*v1.Pod{
641645
test.BuildTestPod("p16", 400, 0, n1.Name, func(pod *v1.Pod) {
642646
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -649,7 +653,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
649653
evictSystemCriticalPods: true,
650654
result: true,
651655
}, {
652-
description: "Pod is evicted becasuse it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true",
656+
description: "Pod is evicted because it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true",
653657
pods: []*v1.Pod{
654658
test.BuildTestPod("p17", 400, 0, n1.Name, func(pod *v1.Pod) {
655659
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -661,7 +665,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
661665
priorityThreshold: &lowPriority,
662666
result: true,
663667
}, {
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",
668+
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",
665669
pods: []*v1.Pod{
666670
test.BuildTestPod("p17", 400, 0, n1.Name, func(pod *v1.Pod) {
667671
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
@@ -704,6 +708,47 @@ func TestDefaultEvictorFilter(t *testing.T) {
704708
evictSystemCriticalPods: false,
705709
nodeFit: true,
706710
result: true,
711+
}, {
712+
description: "minReplicas of 2, owner with 2 replicas, evicts",
713+
pods: []*v1.Pod{
714+
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
715+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
716+
pod.ObjectMeta.OwnerReferences[0].UID = ownerRefUUID
717+
}),
718+
test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) {
719+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
720+
pod.ObjectMeta.OwnerReferences[0].UID = ownerRefUUID
721+
}),
722+
},
723+
minReplicas: 2,
724+
result: true,
725+
}, {
726+
description: "minReplicas of 3, owner with 2 replicas, no eviction",
727+
pods: []*v1.Pod{
728+
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
729+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
730+
pod.ObjectMeta.OwnerReferences[0].UID = ownerRefUUID
731+
}),
732+
test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) {
733+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
734+
pod.ObjectMeta.OwnerReferences[0].UID = ownerRefUUID
735+
}),
736+
},
737+
minReplicas: 3,
738+
result: false,
739+
}, {
740+
description: "minReplicas of 2, multiple owners, no eviction",
741+
pods: []*v1.Pod{
742+
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
743+
pod.ObjectMeta.OwnerReferences = append(test.GetNormalPodOwnerRefList(), test.GetNormalPodOwnerRefList()...)
744+
pod.ObjectMeta.OwnerReferences[0].UID = ownerRefUUID
745+
}),
746+
test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) {
747+
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
748+
}),
749+
},
750+
minReplicas: 2,
751+
result: true,
707752
},
708753
}
709754

@@ -741,7 +786,8 @@ func TestDefaultEvictorFilter(t *testing.T) {
741786
PriorityThreshold: &api.PriorityThreshold{
742787
Value: test.priorityThreshold,
743788
},
744-
NodeFit: test.nodeFit,
789+
NodeFit: test.nodeFit,
790+
MinReplicas: test.minReplicas,
745791
}
746792

747793
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
}

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

+6
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ package defaultevictor
1616
import (
1717
"fmt"
1818

19+
"k8s.io/klog/v2"
20+
1921
"k8s.io/apimachinery/pkg/runtime"
2022
)
2123

@@ -26,5 +28,9 @@ func ValidateDefaultEvictorArgs(obj runtime.Object) error {
2628
return fmt.Errorf("priority threshold misconfigured, only one of priorityThreshold fields can be set, got %v", args)
2729
}
2830

31+
if args.MinReplicas == 1 {
32+
klog.V(4).Info("DefaultEvictor minReplicas must be greater than 1 to check for min pods during eviction. This check will be ignored during eviction.")
33+
}
34+
2935
return nil
3036
}

0 commit comments

Comments
 (0)