diff --git a/README.md b/README.md index b1febbc69..aef447b3b 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ See the [user guide](docs/user-guide.md) in the `/docs` directory. ## Policy, Default Evictor and Strategy plugins -**⚠️ v1alpha1 configuration is still suported, but deprecated (and soon will be removed). Please consider migrating to v1alpha2 (described bellow). For previous v1alpha1 documentation go to [docs/deprecated/v1alpha1.md](docs/deprecated/v1alpha1.md) ⚠️** +**⚠️ v1alpha1 configuration is still supported, but deprecated (and soon will be removed). Please consider migrating to v1alpha2 (described bellow). For previous v1alpha1 documentation go to [docs/deprecated/v1alpha1.md](docs/deprecated/v1alpha1.md) ⚠️** The Descheduler Policy is configurable and includes default strategy plugins that can be enabled or disabled. It includes a common eviction configuration at the top level, as well as configuration from the Evictor plugin (Default Evictor, if not specified otherwise). Top-level configuration and Evictor plugin configuration are applied to all evictions. @@ -525,6 +525,11 @@ This strategy requires k8s version 1.18 at a minimum. By default, this strategy only deals with hard constraints, setting parameter `includeSoftConstraints` to `true` will include soft constraints. +The `topologyBalanceNodeFit` arg is used when balancing topology domains while the Default Evictor's `nodeFit` is used in pre-eviction to determine if a pod can be evicted. +```yaml +topologyBalanceNodeFit: false +``` + Strategy parameter `labelSelector` is not utilized when balancing topology domains and is only applied during eviction to determine if the pod can be evicted. **Parameters:** @@ -534,6 +539,7 @@ Strategy parameter `labelSelector` is not utilized when balancing topology domai |`includeSoftConstraints`|bool| |`namespaces`|(see [namespace filtering](#namespace-filtering))| |`labelSelector`|(see [label filtering](#label-filtering))| +|`topologyBalanceNodeFit`|bool|default `true`. [node fit filtering](#node-fit-filtering) when balancing topology domains| **Example:** diff --git a/pkg/api/v1alpha1/strategymigration.go b/pkg/api/v1alpha1/strategymigration.go index 302b768ce..1c7153aea 100644 --- a/pkg/api/v1alpha1/strategymigration.go +++ b/pkg/api/v1alpha1/strategymigration.go @@ -20,6 +20,7 @@ import ( "fmt" "k8s.io/klog/v2" + utilpointer "k8s.io/utils/pointer" "sigs.k8s.io/descheduler/pkg/api" "sigs.k8s.io/descheduler/pkg/framework/plugins/nodeutilization" "sigs.k8s.io/descheduler/pkg/framework/plugins/podlifetime" @@ -174,6 +175,7 @@ var StrategyParamsToPluginArgs = map[string]func(params *StrategyParameters) (*a Namespaces: v1alpha1NamespacesToInternal(params.Namespaces), LabelSelector: params.LabelSelector, IncludeSoftConstraints: params.IncludeSoftConstraints, + TopologyBalanceNodeFit: utilpointer.Bool(true), } if err := removepodsviolatingtopologyspreadconstraint.ValidateRemovePodsViolatingTopologySpreadConstraintArgs(args); err != nil { klog.ErrorS(err, "unable to validate plugin arguments", "pluginName", removepodsviolatingtopologyspreadconstraint.PluginName) diff --git a/pkg/api/v1alpha1/strategymigration_test.go b/pkg/api/v1alpha1/strategymigration_test.go index 983168014..bf083607d 100644 --- a/pkg/api/v1alpha1/strategymigration_test.go +++ b/pkg/api/v1alpha1/strategymigration_test.go @@ -568,6 +568,7 @@ func TestStrategyParamsToPluginArgsRemovePodsViolatingTopologySpreadConstraint(t Name: removepodsviolatingtopologyspreadconstraint.PluginName, Args: &removepodsviolatingtopologyspreadconstraint.RemovePodsViolatingTopologySpreadConstraintArgs{ IncludeSoftConstraints: true, + TopologyBalanceNodeFit: utilpointer.Bool(true), Namespaces: &api.Namespaces{ Exclude: []string{"test1"}, }, diff --git a/pkg/descheduler/policyconfig_test.go b/pkg/descheduler/policyconfig_test.go index 629bb3f40..7b8e28809 100644 --- a/pkg/descheduler/policyconfig_test.go +++ b/pkg/descheduler/policyconfig_test.go @@ -399,7 +399,9 @@ func TestV1alpha1ToV1alpha2(t *testing.T) { defaultEvictorPluginConfig, { Name: removepodsviolatingtopologyspreadconstraint.PluginName, - Args: &removepodsviolatingtopologyspreadconstraint.RemovePodsViolatingTopologySpreadConstraintArgs{}, + Args: &removepodsviolatingtopologyspreadconstraint.RemovePodsViolatingTopologySpreadConstraintArgs{ + TopologyBalanceNodeFit: utilpointer.Bool(true), + }, }, }, Plugins: api.Plugins{ @@ -715,6 +717,7 @@ func TestV1alpha1ToV1alpha2(t *testing.T) { Name: removepodsviolatingtopologyspreadconstraint.PluginName, Args: &removepodsviolatingtopologyspreadconstraint.RemovePodsViolatingTopologySpreadConstraintArgs{ IncludeSoftConstraints: true, + TopologyBalanceNodeFit: utilpointer.Bool(true), }, }, }, diff --git a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults.go b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults.go index ff468c215..66d3d0e45 100644 --- a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults.go +++ b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults.go @@ -15,6 +15,7 @@ package removepodsviolatingtopologyspreadconstraint import ( "k8s.io/apimachinery/pkg/runtime" + utilpointer "k8s.io/utils/pointer" ) func addDefaultingFuncs(scheme *runtime.Scheme) error { @@ -34,4 +35,7 @@ func SetDefaults_RemovePodsViolatingTopologySpreadConstraintArgs(obj runtime.Obj if !args.IncludeSoftConstraints { args.IncludeSoftConstraints = false } + if args.TopologyBalanceNodeFit == nil { + args.TopologyBalanceNodeFit = utilpointer.Bool(true) + } } diff --git a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults_test.go b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults_test.go index 605bd5a11..c89f783ec 100644 --- a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults_test.go +++ b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/defaults_test.go @@ -20,9 +20,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + utilpointer "k8s.io/utils/pointer" "sigs.k8s.io/descheduler/pkg/api" ) +var scheme *runtime.Scheme + +func init() { + scheme = runtime.NewScheme() + scheme.AddTypeDefaultingFunc(&RemovePodsViolatingTopologySpreadConstraintArgs{}, func(obj interface{}) { + SetDefaults_RemovePodsViolatingTopologySpreadConstraintArgs(obj.(*RemovePodsViolatingTopologySpreadConstraintArgs)) + }) + utilruntime.Must(AddToScheme(scheme)) +} + func TestSetDefaults_RemovePodsViolatingTopologySpreadConstraintArgs(t *testing.T) { tests := []struct { name string @@ -36,6 +47,7 @@ func TestSetDefaults_RemovePodsViolatingTopologySpreadConstraintArgs(t *testing. Namespaces: nil, LabelSelector: nil, IncludeSoftConstraints: false, + TopologyBalanceNodeFit: utilpointer.Bool(true), }, }, { @@ -49,12 +61,27 @@ func TestSetDefaults_RemovePodsViolatingTopologySpreadConstraintArgs(t *testing. Namespaces: &api.Namespaces{}, LabelSelector: &metav1.LabelSelector{}, IncludeSoftConstraints: true, + TopologyBalanceNodeFit: utilpointer.Bool(true), + }, + }, + { + name: "RemovePodsViolatingTopologySpreadConstraintArgs without TopologyBalanceNodeFit", + in: &RemovePodsViolatingTopologySpreadConstraintArgs{}, + want: &RemovePodsViolatingTopologySpreadConstraintArgs{ + TopologyBalanceNodeFit: utilpointer.Bool(true), + }, + }, + { + name: "RemovePodsViolatingTopologySpreadConstraintArgs with TopologyBalanceNodeFit=false", + in: &RemovePodsViolatingTopologySpreadConstraintArgs{ + TopologyBalanceNodeFit: utilpointer.Bool(false), + }, + want: &RemovePodsViolatingTopologySpreadConstraintArgs{ + TopologyBalanceNodeFit: utilpointer.Bool(false), }, }, } for _, tc := range tests { - scheme := runtime.NewScheme() - utilruntime.Must(AddToScheme(scheme)) t.Run(tc.name, func(t *testing.T) { scheme.Default(tc.in) if diff := cmp.Diff(tc.in, tc.want); diff != "" { diff --git a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint.go b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint.go index eeb049b50..2d5fbbf65 100644 --- a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint.go +++ b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" + utilpointer "k8s.io/utils/pointer" "sigs.k8s.io/descheduler/pkg/descheduler/evictions" "sigs.k8s.io/descheduler/pkg/descheduler/node" @@ -197,7 +198,7 @@ func (d *RemovePodsViolatingTopologySpreadConstraint) Balance(ctx context.Contex klog.V(2).InfoS("Skipping topology constraint because it is already balanced", "constraint", constraint) continue } - balanceDomains(d.handle.GetPodsAssignedToNodeFunc(), podsForEviction, constraint, constraintTopologies, sumPods, d.handle.Evictor().Filter, nodes) + d.balanceDomains(podsForEviction, constraint, constraintTopologies, sumPods, nodes) } } @@ -270,17 +271,18 @@ func topologyIsBalanced(topology map[topologyPair][]*v1.Pod, constraint v1.Topol // Following this, the above topology domains end up "sorted" as: // [5, 5, 5, 5, 5, 5] // (assuming even distribution by the scheduler of the evicted pods) -func balanceDomains( - getPodsAssignedToNode podutil.GetPodsAssignedToNodeFunc, +func (d *RemovePodsViolatingTopologySpreadConstraint) balanceDomains( podsForEviction map[*v1.Pod]struct{}, constraint v1.TopologySpreadConstraint, constraintTopologies map[topologyPair][]*v1.Pod, sumPods float64, - isEvictable func(pod *v1.Pod) bool, nodes []*v1.Node, ) { idealAvg := sumPods / float64(len(constraintTopologies)) + isEvictable := d.handle.Evictor().Filter sortedDomains := sortDomains(constraintTopologies, isEvictable) + getPodsAssignedToNode := d.handle.GetPodsAssignedToNodeFunc() + topologyBalanceNodeFit := utilpointer.BoolDeref(d.args.TopologyBalanceNodeFit, true) nodesBelowIdealAvg := filterNodesBelowIdealAvg(nodes, sortedDomains, constraint.TopologyKey, idealAvg) @@ -335,7 +337,7 @@ func balanceDomains( // This is because the chosen pods aren't sorted, but immovable pods still count as "evicted" toward the PTS algorithm. // So, a better selection heuristic could improve performance. - if !node.PodFitsAnyOtherNode(getPodsAssignedToNode, aboveToEvict[k], nodesBelowIdealAvg) { + if topologyBalanceNodeFit && !node.PodFitsAnyOtherNode(getPodsAssignedToNode, aboveToEvict[k], nodesBelowIdealAvg) { klog.V(2).InfoS("ignoring pod for eviction as it does not fit on any other node", "pod", klog.KObj(aboveToEvict[k])) continue } diff --git a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/types.go b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/types.go index a661608b9..14a1da276 100644 --- a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/types.go +++ b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/types.go @@ -17,6 +17,7 @@ limitations under the License. package removepodsviolatingtopologyspreadconstraint import ( + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/descheduler/pkg/api" ) @@ -28,7 +29,8 @@ import ( type RemovePodsViolatingTopologySpreadConstraintArgs struct { metav1.TypeMeta `json:",inline"` - Namespaces *api.Namespaces `json:"namespaces"` - LabelSelector *metav1.LabelSelector `json:"labelSelector"` - IncludeSoftConstraints bool `json:"includeSoftConstraints"` + Namespaces *api.Namespaces `json:"namespaces"` + LabelSelector *metav1.LabelSelector `json:"labelSelector"` + Constraints []v1.UnsatisfiableConstraintAction `json:"constraints"` + TopologyBalanceNodeFit *bool `json:"topologyBalanceNodeFit"` } diff --git a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/zz_generated.deepcopy.go b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/zz_generated.deepcopy.go index dd15101a1..2a92c4746 100644 --- a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/zz_generated.deepcopy.go +++ b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/zz_generated.deepcopy.go @@ -41,6 +41,11 @@ func (in *RemovePodsViolatingTopologySpreadConstraintArgs) DeepCopyInto(out *Rem *out = new(v1.LabelSelector) (*in).DeepCopyInto(*out) } + if in.TopologyBalanceNodeFit != nil { + in, out := &in.TopologyBalanceNodeFit, &out.TopologyBalanceNodeFit + *out = new(bool) + **out = **in + } return }