From 257bd559091c686695825c5c8c7b29c2d974b3ce Mon Sep 17 00:00:00 2001 From: googs1025 Date: Tue, 19 Aug 2025 17:52:54 +0800 Subject: [PATCH] standardize protectionEnabled param --- .../plugins/defaultevictor/constraints.go | 133 ------------------ .../plugins/defaultevictor/defaultevictor.go | 102 ++++++++++++-- .../defaultevictor/defaultevictor_test.go | 10 +- 3 files changed, 92 insertions(+), 153 deletions(-) diff --git a/pkg/framework/plugins/defaultevictor/constraints.go b/pkg/framework/plugins/defaultevictor/constraints.go index ffa57be89..69961dab7 100644 --- a/pkg/framework/plugins/defaultevictor/constraints.go +++ b/pkg/framework/plugins/defaultevictor/constraints.go @@ -14,7 +14,6 @@ limitations under the License. package defaultevictor import ( - "context" "fmt" "time" @@ -23,109 +22,9 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/klog/v2" - "sigs.k8s.io/descheduler/pkg/api" - podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod" frameworktypes "sigs.k8s.io/descheduler/pkg/framework/types" - "sigs.k8s.io/descheduler/pkg/utils" ) -func evictionConstraintsForFailedBarePods(logger klog.Logger, evictFailedBarePods bool) []constraint { - if evictFailedBarePods { - logger.V(1).Info("Warning: EvictFailedBarePods is set to True. This could cause eviction of pods without ownerReferences.") - return []constraint{ - func(pod *v1.Pod) error { - ownerRefList := podutil.OwnerRef(pod) - if len(ownerRefList) == 0 && pod.Status.Phase != v1.PodFailed { - return fmt.Errorf("pod does not have any ownerRefs and is not in failed phase") - } - return nil - }, - } - } - return []constraint{ - func(pod *v1.Pod) error { - if len(podutil.OwnerRef(pod)) == 0 { - return fmt.Errorf("pod does not have any ownerRefs") - } - return nil - }, - } -} - -func evictionConstraintsForSystemCriticalPods(logger klog.Logger, evictSystemCriticalPods bool, priorityThreshold *api.PriorityThreshold, handle frameworktypes.Handle) ([]constraint, error) { - var constraints []constraint - - if !evictSystemCriticalPods { - constraints = append(constraints, func(pod *v1.Pod) error { - if utils.IsCriticalPriorityPod(pod) { - return fmt.Errorf("pod has system critical priority") - } - return nil - }) - - if priorityThreshold != nil && (priorityThreshold.Value != nil || len(priorityThreshold.Name) > 0) { - thresholdPriority, err := utils.GetPriorityValueFromPriorityThreshold(context.TODO(), handle.ClientSet(), priorityThreshold) - if err != nil { - logger.Error(err, "failed to get priority threshold") - return nil, err - } - constraints = append(constraints, func(pod *v1.Pod) error { - if !IsPodEvictableBasedOnPriority(pod, thresholdPriority) { - return fmt.Errorf("pod has higher priority than specified priority class threshold") - } - return nil - }) - } - } else { - logger.V(1).Info("Warning: EvictSystemCriticalPods is set to True. This could cause eviction of Kubernetes system pods.") - } - - return constraints, nil -} - -func evictionConstraintsForLocalStoragePods(evictLocalStoragePods bool) []constraint { - if !evictLocalStoragePods { - return []constraint{ - func(pod *v1.Pod) error { - if utils.IsPodWithLocalStorage(pod) { - return fmt.Errorf("pod has local storage and descheduler is not configured with evictLocalStoragePods") - } - return nil - }, - } - } - return nil -} - -func evictionConstraintsForDaemonSetPods(evictDaemonSetPods bool) []constraint { - if !evictDaemonSetPods { - return []constraint{ - func(pod *v1.Pod) error { - ownerRefList := podutil.OwnerRef(pod) - if utils.IsDaemonsetPod(ownerRefList) { - return fmt.Errorf("pod is related to daemonset and descheduler is not configured with evictDaemonSetPods") - } - return nil - }, - } - } - return nil -} - -func evictionConstraintsForPvcPods(ignorePvcPods bool) []constraint { - if ignorePvcPods { - return []constraint{ - func(pod *v1.Pod) error { - if utils.IsPodWithPVC(pod) { - return fmt.Errorf("pod has a PVC and descheduler is configured to ignore PVC pods") - } - return nil - }, - } - } - return nil -} - func evictionConstraintsForLabelSelector(logger klog.Logger, labelSelector *metav1.LabelSelector) ([]constraint, error) { if labelSelector != nil { selector, err := metav1.LabelSelectorAsSelector(labelSelector) @@ -193,35 +92,3 @@ func evictionConstraintsForMinPodAge(minPodAge *metav1.Duration) []constraint { } return nil } - -func evictionConstraintsForIgnorePodsWithoutPDB(ignorePodsWithoutPDB bool, handle frameworktypes.Handle) []constraint { - if ignorePodsWithoutPDB { - return []constraint{ - func(pod *v1.Pod) error { - hasPdb, err := utils.IsPodCoveredByPDB(pod, handle.SharedInformerFactory().Policy().V1().PodDisruptionBudgets().Lister()) - if err != nil { - return fmt.Errorf("unable to check if pod is covered by PodDisruptionBudget: %w", err) - } - if !hasPdb { - return fmt.Errorf("no PodDisruptionBudget found for pod") - } - return nil - }, - } - } - return nil -} - -func evictionConstraintsForResourceClaimsPods(protectionEnabled bool) []constraint { - if protectionEnabled { - return []constraint{ - func(pod *v1.Pod) error { - if utils.IsPodWithResourceClaims(pod) { - return fmt.Errorf("pod has ResourceClaims and descheduler is configured to protect ResourceClaims pods") - } - return nil - }, - } - } - return nil -} diff --git a/pkg/framework/plugins/defaultevictor/defaultevictor.go b/pkg/framework/plugins/defaultevictor/defaultevictor.go index a200269ac..909473f2f 100644 --- a/pkg/framework/plugins/defaultevictor/defaultevictor.go +++ b/pkg/framework/plugins/defaultevictor/defaultevictor.go @@ -131,47 +131,119 @@ func applyEffectivePodProtections(d *DefaultEvictor, podProtections []PodProtect func applyFailedBarePodsProtection(d *DefaultEvictor, protectionMap map[PodProtection]bool) { isProtectionEnabled := protectionMap[FailedBarePods] - d.constraints = append(d.constraints, evictionConstraintsForFailedBarePods(d.logger, !isProtectionEnabled)...) + if !isProtectionEnabled { + d.logger.V(1).Info("Warning: EvictFailedBarePods is set to True. This could cause eviction of pods without ownerReferences.") + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + ownerRefList := podutil.OwnerRef(pod) + if len(ownerRefList) == 0 && pod.Status.Phase != v1.PodFailed { + return fmt.Errorf("pod does not have any ownerRefs and is not in failed phase") + } + return nil + }) + } else { + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + if len(podutil.OwnerRef(pod)) == 0 { + return fmt.Errorf("pod does not have any ownerRefs") + } + return nil + }) + } } func applySystemCriticalPodsProtection(d *DefaultEvictor, protectionMap map[PodProtection]bool, handle frameworktypes.Handle) error { isProtectionEnabled := protectionMap[SystemCriticalPods] - constraints, err := evictionConstraintsForSystemCriticalPods( - d.logger, - !isProtectionEnabled, - d.args.PriorityThreshold, - handle, - ) - if err != nil { - return err + if !isProtectionEnabled { + d.logger.V(1).Info("Warning: System critical pod protection is disabled. This could cause eviction of Kubernetes system pods.") + return nil + } + + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + if utils.IsCriticalPriorityPod(pod) { + return fmt.Errorf("pod has system critical priority and is protected against eviction") + } + return nil + }) + + priorityThreshold := d.args.PriorityThreshold + if priorityThreshold != nil && (priorityThreshold.Value != nil || len(priorityThreshold.Name) > 0) { + thresholdPriority, err := utils.GetPriorityValueFromPriorityThreshold(context.TODO(), handle.ClientSet(), priorityThreshold) + if err != nil { + d.logger.Error(err, "failed to get priority threshold") + return err + } + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + if !IsPodEvictableBasedOnPriority(pod, thresholdPriority) { + return fmt.Errorf("pod has higher priority than specified priority class threshold") + } + return nil + }) } - d.constraints = append(d.constraints, constraints...) return nil } func applyLocalStoragePodsProtection(d *DefaultEvictor, protectionMap map[PodProtection]bool) { isProtectionEnabled := protectionMap[PodsWithLocalStorage] - d.constraints = append(d.constraints, evictionConstraintsForLocalStoragePods(!isProtectionEnabled)...) + if isProtectionEnabled { + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + if utils.IsPodWithLocalStorage(pod) { + return fmt.Errorf("pod has local storage and is protected against eviction") + } + return nil + }) + } } func applyDaemonSetPodsProtection(d *DefaultEvictor, protectionMap map[PodProtection]bool) { isProtectionEnabled := protectionMap[DaemonSetPods] - d.constraints = append(d.constraints, evictionConstraintsForDaemonSetPods(!isProtectionEnabled)...) + if isProtectionEnabled { + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + ownerRefList := podutil.OwnerRef(pod) + if utils.IsDaemonsetPod(ownerRefList) { + return fmt.Errorf("daemonset pods are protected against eviction") + } + return nil + }) + } } func applyPvcPodsProtection(d *DefaultEvictor, protectionMap map[PodProtection]bool) { isProtectionEnabled := protectionMap[PodsWithPVC] - d.constraints = append(d.constraints, evictionConstraintsForPvcPods(isProtectionEnabled)...) + if isProtectionEnabled { + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + if utils.IsPodWithPVC(pod) { + return fmt.Errorf("pod with PVC is protected against eviction") + } + return nil + }) + } } func applyPodsWithoutPDBProtection(d *DefaultEvictor, protectionMap map[PodProtection]bool, handle frameworktypes.Handle) { isProtectionEnabled := protectionMap[PodsWithoutPDB] - d.constraints = append(d.constraints, evictionConstraintsForIgnorePodsWithoutPDB(isProtectionEnabled, handle)...) + if isProtectionEnabled { + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + hasPdb, err := utils.IsPodCoveredByPDB(pod, handle.SharedInformerFactory().Policy().V1().PodDisruptionBudgets().Lister()) + if err != nil { + return fmt.Errorf("unable to check if pod is covered by PodDisruptionBudget: %w", err) + } + if !hasPdb { + return fmt.Errorf("pod does not have a PodDisruptionBudget and is protected against eviction") + } + return nil + }) + } } func applyPodsWithResourceClaimsProtection(d *DefaultEvictor, protectionMap map[PodProtection]bool) { isProtectionEnabled := protectionMap[PodsWithResourceClaims] - d.constraints = append(d.constraints, evictionConstraintsForResourceClaimsPods(isProtectionEnabled)...) + if isProtectionEnabled { + d.constraints = append(d.constraints, func(pod *v1.Pod) error { + if utils.IsPodWithResourceClaims(pod) { + return fmt.Errorf("pod has ResourceClaims and descheduler is configured to protect ResourceClaims pods") + } + return nil + }) + } } // getEffectivePodProtections determines which policies are currently active. diff --git a/pkg/framework/plugins/defaultevictor/defaultevictor_test.go b/pkg/framework/plugins/defaultevictor/defaultevictor_test.go index 56d89a223..0f2484788 100644 --- a/pkg/framework/plugins/defaultevictor/defaultevictor_test.go +++ b/pkg/framework/plugins/defaultevictor/defaultevictor_test.go @@ -794,7 +794,7 @@ func TestDefaultEvictorFilter(t *testing.T) { result: true, }, { - description: "Pod with local storage is evicted because 'withLocalStorage' is in disabledDefaultPodProtections", + description: "Pod with local storage is evicted because 'PodsWithLocalStorage' is in DefaultDisabled", pods: []*v1.Pod{ test.BuildTestPod("p18", 400, 0, n1.Name, func(pod *v1.Pod) { pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() @@ -813,7 +813,7 @@ func TestDefaultEvictorFilter(t *testing.T) { result: true, }, { - description: "DaemonSet pod is evicted because 'daemonSetPods' is in disabledDefaultPodProtections", + description: "DaemonSet pod is evicted because 'DaemonSetPods' is in DefaultDisabled", pods: []*v1.Pod{ test.BuildTestPod("p19", 400, 0, n1.Name, func(pod *v1.Pod) { pod.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ @@ -831,7 +831,7 @@ func TestDefaultEvictorFilter(t *testing.T) { result: true, }, { - description: "Pod with PVC is not evicted because 'withPVC' is in extraPodProtections", + description: "Pod with PVC is not evicted because 'PodsWithPVC' is in ExtraEnabled", pods: []*v1.Pod{ test.BuildTestPod("p20", 400, 0, n1.Name, func(pod *v1.Pod) { pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() @@ -850,7 +850,7 @@ func TestDefaultEvictorFilter(t *testing.T) { result: false, }, { - description: "Pod without PDB is not evicted because 'withoutPDB' is in extraPodProtections", + description: "Pod without PDB is not evicted because 'PodsWithoutPDB' is in ExtraEnabled", pods: []*v1.Pod{ test.BuildTestPod("p21", 400, 0, n1.Name, func(pod *v1.Pod) { pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() @@ -862,7 +862,7 @@ func TestDefaultEvictorFilter(t *testing.T) { result: false, }, { - description: "Pod with ResourceClaims is not evicted because 'PodsWithResourceClaims' is in extraPodProtections", + description: "Pod with ResourceClaims is not evicted because 'PodsWithResourceClaims' is in ExtraEnabled", pods: []*v1.Pod{ test.BuildTestPod("p20", 400, 0, n1.Name, func(pod *v1.Pod) { pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()