From 941c36f9a538687246fd4f1f02e1ad19dca3b613 Mon Sep 17 00:00:00 2001 From: Rob Wilson Date: Wed, 19 Aug 2020 08:56:40 +0100 Subject: [PATCH 1/3] Add evict-system-critical-pods option to permit evicting system critical pods, see also: #378 --- charts/descheduler/values.yaml | 1 + cmd/descheduler/app/options/options.go | 2 ++ pkg/apis/componentconfig/types.go | 3 +++ pkg/apis/componentconfig/v1alpha1/types.go | 3 +++ .../v1alpha1/zz_generated.conversion.go | 1 + pkg/descheduler/descheduler.go | 1 + pkg/descheduler/evictions/evictions.go | 13 +++++++------ pkg/descheduler/evictions/evictions_test.go | 11 ++++++----- 8 files changed, 24 insertions(+), 11 deletions(-) diff --git a/charts/descheduler/values.yaml b/charts/descheduler/values.yaml index 61cd1ed1c5..4e9c8b12fe 100644 --- a/charts/descheduler/values.yaml +++ b/charts/descheduler/values.yaml @@ -16,6 +16,7 @@ schedule: "*/2 * * * *" cmdOptions: v: 3 # evict-local-storage-pods: + # evict-system-critical-pods: # max-pods-to-evict-per-node: 10 # node-selector: "key1=value1,key2=value2" diff --git a/cmd/descheduler/app/options/options.go b/cmd/descheduler/app/options/options.go index d398592e00..bc9ddc2cf0 100644 --- a/cmd/descheduler/app/options/options.go +++ b/cmd/descheduler/app/options/options.go @@ -58,4 +58,6 @@ func (rs *DeschedulerServer) AddFlags(fs *pflag.FlagSet) { fs.IntVar(&rs.MaxNoOfPodsToEvictPerNode, "max-pods-to-evict-per-node", rs.MaxNoOfPodsToEvictPerNode, "Limits the maximum number of pods to be evicted per node by descheduler") // evict-local-storage-pods allows eviction of pods that are using local storage. This is false by default. fs.BoolVar(&rs.EvictLocalStoragePods, "evict-local-storage-pods", rs.EvictLocalStoragePods, "Enables evicting pods using local storage by descheduler") + // evict-system-critical-pods allows eviction of critical pods. This is false by default. + fs.BoolVar(&rs.EvictSystemCriticalPods, "evict-system-critical-pods", rs.EvictSystemCriticalPods, "Enables evicting critical pods by descheduler") } diff --git a/pkg/apis/componentconfig/types.go b/pkg/apis/componentconfig/types.go index 1588d59f52..0a57d46e54 100644 --- a/pkg/apis/componentconfig/types.go +++ b/pkg/apis/componentconfig/types.go @@ -48,4 +48,7 @@ type DeschedulerConfiguration struct { // EvictLocalStoragePods allows pods using local storage to be evicted. EvictLocalStoragePods bool + + // EvictSystemCriticalPods allows critical pods to be evicted. + EvictSystemCriticalPods bool } diff --git a/pkg/apis/componentconfig/v1alpha1/types.go b/pkg/apis/componentconfig/v1alpha1/types.go index 69121ea636..00552cedd2 100644 --- a/pkg/apis/componentconfig/v1alpha1/types.go +++ b/pkg/apis/componentconfig/v1alpha1/types.go @@ -48,4 +48,7 @@ type DeschedulerConfiguration struct { // EvictLocalStoragePods allows pods using local storage to be evicted. EvictLocalStoragePods bool `json:"evictLocalStoragePods,omitempty"` + + // EvictSystemCriticalPods allows critical pods to be evicted. + EvictSystemCriticalPods bool `json:"evictSystemCriticalPods,omitempty"` } diff --git a/pkg/apis/componentconfig/v1alpha1/zz_generated.conversion.go b/pkg/apis/componentconfig/v1alpha1/zz_generated.conversion.go index 7042540268..85f84b308d 100644 --- a/pkg/apis/componentconfig/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/componentconfig/v1alpha1/zz_generated.conversion.go @@ -56,6 +56,7 @@ func autoConvert_v1alpha1_DeschedulerConfiguration_To_componentconfig_Deschedule out.NodeSelector = in.NodeSelector out.MaxNoOfPodsToEvictPerNode = in.MaxNoOfPodsToEvictPerNode out.EvictLocalStoragePods = in.EvictLocalStoragePods + out.EvictSystemCriticalPods = in.EvictSystemCriticalPods return nil } diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go index 768567ed5d..86bc83ce43 100644 --- a/pkg/descheduler/descheduler.go +++ b/pkg/descheduler/descheduler.go @@ -100,6 +100,7 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer rs.MaxNoOfPodsToEvictPerNode, nodes, rs.EvictLocalStoragePods, + rs.EvictSystemCriticalPods, ) for name, f := range strategyFuncs { diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index 7a5c712344..a0417a757a 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -45,12 +45,13 @@ const ( type nodePodEvictedCount map[*v1.Node]int type PodEvictor struct { - client clientset.Interface - policyGroupVersion string - dryRun bool - maxPodsToEvictPerNode int - nodepodCount nodePodEvictedCount - evictLocalStoragePods bool + client clientset.Interface + policyGroupVersion string + dryRun bool + maxPodsToEvictPerNode int + nodepodCount nodePodEvictedCount + evictLocalStoragePods bool + evictSystemCriticalPods bool } func NewPodEvictor( diff --git a/pkg/descheduler/evictions/evictions_test.go b/pkg/descheduler/evictions/evictions_test.go index 81f3155f57..58ecfc1071 100644 --- a/pkg/descheduler/evictions/evictions_test.go +++ b/pkg/descheduler/evictions/evictions_test.go @@ -74,11 +74,12 @@ func TestIsEvictable(t *testing.T) { lowPriority := int32(800) highPriority := int32(900) type testCase struct { - pod *v1.Pod - runBefore func(*v1.Pod) - evictLocalStoragePods bool - priorityThreshold *int32 - result bool + pod *v1.Pod + runBefore func(*v1.Pod) + evictLocalStoragePods bool + evictSystemCriticalPods bool + priorityThreshold *int32 + result bool } testCases := []testCase{ From 2f9d75d89f6baadcb85cc4884ab72a1d8bbfdab8 Mon Sep 17 00:00:00 2001 From: Rob Wilson Date: Wed, 19 Aug 2020 09:04:32 +0100 Subject: [PATCH 2/3] Update eviction logic to check CLI flag option --- pkg/descheduler/evictions/evictions.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index a0417a757a..65b06e34b2 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -81,8 +81,8 @@ func NewPodEvictor( // IsEvictable checks if a pod is evictable or not. func (pe *PodEvictor) IsEvictable(pod *v1.Pod, thresholdPriority int32) bool { checkErrs := []error{} - if IsCriticalPod(pod) { - checkErrs = append(checkErrs, fmt.Errorf("pod is critical")) + if IsCriticalPod(pod) && IsPodWithSystemCritical(pod) { + checkErrs = append(checkErrs, fmt.Errorf("pod is critical and descheduler is not configured with --evict-system-critical-pods")) } if !IsPodEvictableBasedOnPriority(pod, thresholdPriority) { From d6ed464f70e621fa63df77246837677cb94f42bf Mon Sep 17 00:00:00 2001 From: Rob Wilson Date: Wed, 19 Aug 2020 10:45:22 +0100 Subject: [PATCH 3/3] Correct eviction test --- pkg/descheduler/evictions/evictions.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index 65b06e34b2..e257898f1d 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -61,6 +61,7 @@ func NewPodEvictor( maxPodsToEvictPerNode int, nodes []*v1.Node, evictLocalStoragePods bool, + evictSystemCriticalPods bool, ) *PodEvictor { var nodePodCount = make(nodePodEvictedCount) for _, node := range nodes { @@ -75,13 +76,14 @@ func NewPodEvictor( maxPodsToEvictPerNode: maxPodsToEvictPerNode, nodepodCount: nodePodCount, evictLocalStoragePods: evictLocalStoragePods, + evictSystemCriticalPods: evictSystemCriticalPods, } } // IsEvictable checks if a pod is evictable or not. func (pe *PodEvictor) IsEvictable(pod *v1.Pod, thresholdPriority int32) bool { checkErrs := []error{} - if IsCriticalPod(pod) && IsPodWithSystemCritical(pod) { + if !pe.evictSystemCriticalPods && IsCriticalPod(pod) { checkErrs = append(checkErrs, fmt.Errorf("pod is critical and descheduler is not configured with --evict-system-critical-pods")) }