Skip to content

Commit

Permalink
skip eviction when replica count is below evictor minReplicas thres…
Browse files Browse the repository at this point in the history
…hold setting
  • Loading branch information
a7i committed Oct 7, 2023
1 parent 3bd9dfc commit 903c51b
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 14 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ The Default Evictor Plugin is used by default for filtering pods before processi
|`labelSelector`|`metav1.LabelSelector`||(see [label filtering](#label-filtering))|
|`priorityThreshold`|`priorityThreshold`||(see [priority filtering](#priority-filtering))|
|`nodeFit`|`bool`|`false`|(see [node fit filtering](#node-fit-filtering))|
|`minReplicas`|`uint`|`0`| ignore eviction of pods where owner (e.g. Deployment) replicas is below this threshold |

### Example policy

Expand All @@ -165,6 +166,7 @@ profiles:
evictFailedBarePods: true
evictLocalStoragePods: true
nodeFit: true
minReplicas: 2
plugins:
# DefaultEvictor is enabled for both `filter` and `preEvictionFilter`
# filter:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.10.0
go.opentelemetry.io/otel/sdk v1.10.0
go.opentelemetry.io/otel/trace v1.10.0
golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e
google.golang.org/grpc v1.54.0
k8s.io/api v0.28.0
k8s.io/apimachinery v0.28.0
Expand Down Expand Up @@ -87,7 +88,6 @@ require (
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.19.0 // indirect
golang.org/x/crypto v0.11.0 // indirect
golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect
golang.org/x/mod v0.10.0 // indirect
golang.org/x/net v0.13.0 // indirect
golang.org/x/oauth2 v0.8.0 // indirect
Expand Down
18 changes: 18 additions & 0 deletions pkg/descheduler/pod/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package pod
import (
"sort"

"golang.org/x/exp/slices"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand Down Expand Up @@ -257,3 +258,20 @@ func SortPodsBasedOnAge(pods []*v1.Pod) {
return pods[i].CreationTimestamp.Before(&pods[j].CreationTimestamp)
})
}

// GetOwnerReplicasCount returns the count of pods that match the owner ref of the given pod
func GetOwnerReplicasCount(pods []*v1.Pod, pod *v1.Pod) uint {
var count uint
podOwnerRefs := OwnerRef(pod)
for _, otherPod := range pods {
otherOwnerRefs := OwnerRef(otherPod)

if slices.EqualFunc(podOwnerRefs, otherOwnerRefs, func(l, r metav1.OwnerReference) bool {
return l.UID == r.UID
}) {
count++
}
}

return count
}
15 changes: 15 additions & 0 deletions pkg/framework/plugins/defaultevictor/defaultevictor.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,21 @@ func New(args runtime.Object, handle frameworktypes.Handle) (frameworktypes.Plug
})
}

if defaultEvictorArgs.MinReplicas > 1 {
podLister := handle.SharedInformerFactory().Core().V1().Pods().Lister()
ev.constraints = append(ev.constraints, func(pod *v1.Pod) error {
pods, err := podLister.Pods(pod.Namespace).List(labels.Everything())
if err != nil {
return fmt.Errorf("unable to list pods for minReplicas filter in the policy parameter")
}
ownerReplicas := podutil.GetOwnerReplicasCount(pods, pod)
if ownerReplicas < defaultEvictorArgs.MinReplicas {
return fmt.Errorf("owner has %d replicas which is less than minReplicas of %d", ownerReplicas, defaultEvictorArgs.MinReplicas)
}
return nil
})
}

return ev, nil
}

Expand Down
52 changes: 39 additions & 13 deletions pkg/framework/plugins/defaultevictor/defaultevictor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods bool
priorityThreshold *int32
nodeFit bool
minReplicas uint
result bool
}

Expand Down Expand Up @@ -527,7 +528,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
result: true,
}, {
description: "Pod not evicted becasuse it is part of a daemonSet",
description: "Pod not evicted because it is part of a daemonSet",
pods: []*v1.Pod{
test.BuildTestPod("p8", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -538,7 +539,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
result: false,
}, {
description: "Pod is evicted becasuse it is part of a daemonSet, but it has scheduler.alpha.kubernetes.io/evict annotation",
description: "Pod is evicted because it is part of a daemonSet, but it has scheduler.alpha.kubernetes.io/evict annotation",
pods: []*v1.Pod{
test.BuildTestPod("p9", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.Annotations = map[string]string{"descheduler.alpha.kubernetes.io/evict": "true"}
Expand All @@ -549,7 +550,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
result: true,
}, {
description: "Pod not evicted becasuse it is a mirror poddsa",
description: "Pod not evicted because it is a mirror poddsa",
pods: []*v1.Pod{
test.BuildTestPod("p10", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -560,7 +561,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
result: false,
}, {
description: "Pod is evicted becasuse it is a mirror pod, but it has scheduler.alpha.kubernetes.io/evict annotation",
description: "Pod is evicted because it is a mirror pod, but it has scheduler.alpha.kubernetes.io/evict annotation",
pods: []*v1.Pod{
test.BuildTestPod("p11", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -572,7 +573,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
result: true,
}, {
description: "Pod not evicted becasuse it has system critical priority",
description: "Pod not evicted because it has system critical priority",
pods: []*v1.Pod{
test.BuildTestPod("p12", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -584,7 +585,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
result: false,
}, {
description: "Pod is evicted becasuse it has system critical priority, but it has scheduler.alpha.kubernetes.io/evict annotation",
description: "Pod is evicted because it has system critical priority, but it has scheduler.alpha.kubernetes.io/evict annotation",
pods: []*v1.Pod{
test.BuildTestPod("p13", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -599,7 +600,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
result: true,
}, {
description: "Pod not evicted becasuse it has a priority higher than the configured priority threshold",
description: "Pod not evicted because it has a priority higher than the configured priority threshold",
pods: []*v1.Pod{
test.BuildTestPod("p14", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -611,7 +612,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
priorityThreshold: &lowPriority,
result: false,
}, {
description: "Pod is evicted becasuse it has a priority higher than the configured priority threshold, but it has scheduler.alpha.kubernetes.io/evict annotation",
description: "Pod is evicted because it has a priority higher than the configured priority threshold, but it has scheduler.alpha.kubernetes.io/evict annotation",
pods: []*v1.Pod{
test.BuildTestPod("p15", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -624,7 +625,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
priorityThreshold: &lowPriority,
result: true,
}, {
description: "Pod is evicted becasuse it has system critical priority, but evictSystemCriticalPods = true",
description: "Pod is evicted because it has system critical priority, but evictSystemCriticalPods = true",
pods: []*v1.Pod{
test.BuildTestPod("p16", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -636,7 +637,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: true,
result: true,
}, {
description: "Pod is evicted becasuse it has system critical priority, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
description: "Pod is evicted because it has system critical priority, but evictSystemCriticalPods = true and it has scheduler.alpha.kubernetes.io/evict annotation",
pods: []*v1.Pod{
test.BuildTestPod("p16", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -649,7 +650,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: true,
result: true,
}, {
description: "Pod is evicted becasuse it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true",
description: "Pod is evicted because it has a priority higher than the configured priority threshold, but evictSystemCriticalPods = true",
pods: []*v1.Pod{
test.BuildTestPod("p17", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand All @@ -661,7 +662,7 @@ func TestDefaultEvictorFilter(t *testing.T) {
priorityThreshold: &lowPriority,
result: true,
}, {
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",
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",
pods: []*v1.Pod{
test.BuildTestPod("p17", 400, 0, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
Expand Down Expand Up @@ -704,6 +705,30 @@ func TestDefaultEvictorFilter(t *testing.T) {
evictSystemCriticalPods: false,
nodeFit: true,
result: true,
}, {
description: "minReplicas of 2, owner with 2 replicas, evicts",
pods: []*v1.Pod{
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
}),
test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
}),
},
minReplicas: 2,
result: true,
}, {
description: "minReplicas of 3, owner with 2 replicas, no eviction",
pods: []*v1.Pod{
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
}),
test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
}),
},
minReplicas: 3,
result: false,
},
}

Expand Down Expand Up @@ -741,7 +766,8 @@ func TestDefaultEvictorFilter(t *testing.T) {
PriorityThreshold: &api.PriorityThreshold{
Value: test.priorityThreshold,
},
NodeFit: test.nodeFit,
NodeFit: test.nodeFit,
MinReplicas: test.minReplicas,
}

evictorPlugin, err := New(
Expand Down
1 change: 1 addition & 0 deletions pkg/framework/plugins/defaultevictor/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ type DefaultEvictorArgs struct {
LabelSelector *metav1.LabelSelector `json:"labelSelector"`
PriorityThreshold *api.PriorityThreshold `json:"priorityThreshold"`
NodeFit bool `json:"nodeFit"`
MinReplicas uint `json:"minReplicas"`
}
11 changes: 11 additions & 0 deletions test/e2e/e2e_duplicatepods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func TestRemoveDuplicates(t *testing.T) {
replicasNum int
beforeFunc func(deployment *appsv1.Deployment)
expectedEvictedPodCount uint
minReplicas uint
}{
{
description: "Evict Pod even Pods schedule to specific node",
Expand Down Expand Up @@ -135,6 +136,15 @@ func TestRemoveDuplicates(t *testing.T) {
},
expectedEvictedPodCount: 2,
},
{
description: "Ignores eviction with minReplicas of 4",
replicasNum: 3,
beforeFunc: func(deployment *appsv1.Deployment) {
deployment.Spec.Replicas = pointer.Int32(3)
},
expectedEvictedPodCount: 0,
minReplicas: 4,
},
}
for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {
Expand Down Expand Up @@ -179,6 +189,7 @@ func TestRemoveDuplicates(t *testing.T) {
IgnorePvcPods: false,
EvictFailedBarePods: false,
NodeFit: false,
MinReplicas: tc.minReplicas,
}

evictorFilter, err := defaultevictor.New(
Expand Down

0 comments on commit 903c51b

Please sign in to comment.