diff --git a/README.md b/README.md index 0b7cc47009..e4281bc45f 100644 --- a/README.md +++ b/README.md @@ -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 @@ -165,6 +166,7 @@ profiles: evictFailedBarePods: true evictLocalStoragePods: true nodeFit: true + minReplicas: 2 plugins: # DefaultEvictor is enabled for both `filter` and `preEvictionFilter` # filter: diff --git a/go.mod b/go.mod index fd006f61cf..51e62358d7 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 diff --git a/pkg/descheduler/pod/pods.go b/pkg/descheduler/pod/pods.go index 6a2e808684..bcf0c74240 100644 --- a/pkg/descheduler/pod/pods.go +++ b/pkg/descheduler/pod/pods.go @@ -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" @@ -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 +} diff --git a/pkg/framework/plugins/defaultevictor/defaultevictor.go b/pkg/framework/plugins/defaultevictor/defaultevictor.go index d2f13c8a6e..53db383895 100644 --- a/pkg/framework/plugins/defaultevictor/defaultevictor.go +++ b/pkg/framework/plugins/defaultevictor/defaultevictor.go @@ -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 } diff --git a/pkg/framework/plugins/defaultevictor/defaultevictor_test.go b/pkg/framework/plugins/defaultevictor/defaultevictor_test.go index a4871aa991..d646cd10a5 100644 --- a/pkg/framework/plugins/defaultevictor/defaultevictor_test.go +++ b/pkg/framework/plugins/defaultevictor/defaultevictor_test.go @@ -372,6 +372,7 @@ func TestDefaultEvictorFilter(t *testing.T) { evictSystemCriticalPods bool priorityThreshold *int32 nodeFit bool + minReplicas uint result bool } @@ -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() @@ -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"} @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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, }, } @@ -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( diff --git a/pkg/framework/plugins/defaultevictor/types.go b/pkg/framework/plugins/defaultevictor/types.go index 0f9b4f66de..d68c71a70d 100644 --- a/pkg/framework/plugins/defaultevictor/types.go +++ b/pkg/framework/plugins/defaultevictor/types.go @@ -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"` } diff --git a/test/e2e/e2e_duplicatepods_test.go b/test/e2e/e2e_duplicatepods_test.go index cfcf956339..4eb94df90c 100644 --- a/test/e2e/e2e_duplicatepods_test.go +++ b/test/e2e/e2e_duplicatepods_test.go @@ -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", @@ -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) { @@ -179,6 +189,7 @@ func TestRemoveDuplicates(t *testing.T) { IgnorePvcPods: false, EvictFailedBarePods: false, NodeFit: false, + MinReplicas: tc.minReplicas, } evictorFilter, err := defaultevictor.New(