removepodsviolatingtopologyspreadconstraint: topologyBalanceNodeFit to control whether to perform nodefit when balacning domains

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:**
 

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)

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"},
 					},

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),
 								},
 							},
 						},

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)
+	}
 }

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 != "" {

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
 			}

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"`
 }

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
 }