From 760daaf1100c497bd74dbf9f6f22150d8976eefb Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Wed, 5 Feb 2025 10:38:19 +1300
Subject: [PATCH 01/18] feature(scheduler): Custom pod selection/ordering in
 DefaultPreemption

The current behavior is to select only based on pod priority, where any
pod with higher priority can preempt any pod with lower priority.

In our case, we have multiple priority classes but where only a subset
of those are considered preemptible. Here's roughly how this looks:

- High priority: higher in the scheduling queue, not preemptible
- Low priority: lower in the scheduling queue, not preemptible
- Preemptible priority: lowest in the scheduling queue, preemptible

This PR allows the preemption selection to be configured against the
`DefaultPreemption` plugin, rather than needing to reimplement the
plugin itself. The structure used here mimics [what's currently being
done for the `Evaluator`](https://github.com/kubernetes/kubernetes/blob/release-1.32/pkg/scheduler/framework/preemption/preemption.go#L161-L165),
where a `PreemptPod` callback may be overridden to customize what
happens when performing a preemption.

This PR also updates the plugin's tests to call the updated `New()`
function rather than constructing `DefaultPreemption` directly, so
that `New()` is now being exercised in tests.
---
 .../defaultpreemption/default_preemption.go   | 69 ++++++++-----
 .../default_preemption_test.go                | 96 ++++++++++---------
 pkg/scheduler/util/utils.go                   |  5 +-
 3 files changed, 100 insertions(+), 70 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index c36528530f9..8c9abbe9fb1 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -54,6 +54,14 @@ type DefaultPreemption struct {
 	podLister corelisters.PodLister
 	pdbLister policylisters.PodDisruptionBudgetLister
 	Evaluator *preemption.Evaluator
+
+	// EligiblePods returns victim pods which are allowed to be preempted by the provided preemptor.
+	// The default behavior is to allow any pods of lower priority to be preempted by any pods of higher priority.
+	EligiblePods func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo
+
+	// OrderedPods sorts eligible victims in-place in descending order of highest to lowest priority.
+	// Pods at the start of the slice are less likely to be preempted.
+	OrderedPods func(eligible []*framework.PodInfo)
 }
 
 var _ framework.PostFilterPlugin = &DefaultPreemption{}
@@ -86,6 +94,23 @@ func New(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feat
 	}
 	pl.Evaluator = preemption.NewEvaluator(Name, fh, &pl, fts.EnableAsyncPreemption)
 
+	// Default behavior: Any pods of lower priority may be preempted by any pods of higher priority.
+	pl.EligiblePods = func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo {
+		var eligible []*framework.PodInfo
+		podPriority := corev1helpers.PodPriority(preemptor)
+		for _, pi := range nodeInfo.Pods {
+			if corev1helpers.PodPriority(pi.Pod) < podPriority {
+				eligible = append(eligible, pi)
+			}
+		}
+		return eligible
+	}
+
+	// Default behavior: Sort by descending priority, then by descending runtime duration as secondary ordering.
+	pl.OrderedPods = func(eligible []*framework.PodInfo) {
+		sort.Slice(eligible, func(i, j int) bool { return util.MoreImportantPod(eligible[i].Pod, eligible[j].Pod) })
+	}
+
 	return &pl, nil
 }
 
@@ -165,7 +190,6 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	nodeInfo *framework.NodeInfo,
 	pdbs []*policy.PodDisruptionBudget) ([]*v1.Pod, int, *framework.Status) {
 	logger := klog.FromContext(ctx)
-	var potentialVictims []*framework.PodInfo
 	removePod := func(rpi *framework.PodInfo) error {
 		if err := nodeInfo.RemovePod(logger, rpi.Pod); err != nil {
 			return err
@@ -184,15 +208,12 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 		}
 		return nil
 	}
-	// As the first step, remove all the lower priority pods from the node and
-	// check if the given pod can be scheduled.
-	podPriority := corev1helpers.PodPriority(pod)
-	for _, pi := range nodeInfo.Pods {
-		if corev1helpers.PodPriority(pi.Pod) < podPriority {
-			potentialVictims = append(potentialVictims, pi)
-			if err := removePod(pi); err != nil {
-				return nil, 0, framework.AsStatus(err)
-			}
+	// As the first step, remove all pods eligible for preemption from the node and
+	// check if the given pod could be scheduled without them present.
+	potentialVictims := pl.EligiblePods(nodeInfo, pod)
+	for _, pi := range potentialVictims {
+		if err := removePod(pi); err != nil {
+			return nil, 0, framework.AsStatus(err)
 		}
 	}
 
@@ -201,7 +222,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 		return nil, 0, framework.NewStatus(framework.UnschedulableAndUnresolvable, "No preemption victims found for incoming pod")
 	}
 
-	// If the new pod does not fit after removing all the lower priority pods,
+	// If the new pod does not fit after removing all the eligible pods,
 	// we are almost done and this node is not suitable for preemption. The only
 	// condition that we could check is if the "pod" is failing to schedule due to
 	// inter-pod affinity to one or more victims, but we have decided not to
@@ -210,11 +231,11 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	if status := pl.fh.RunFilterPluginsWithNominatedPods(ctx, state, pod, nodeInfo); !status.IsSuccess() {
 		return nil, 0, status
 	}
-	var victims []*v1.Pod
+	var victims []*framework.PodInfo
 	numViolatingVictim := 0
-	// Sort potentialVictims by pod priority from high to low, which ensures to
-	// reprieve higher priority pods first.
-	sort.Slice(potentialVictims, func(i, j int) bool { return util.MoreImportantPod(potentialVictims[i].Pod, potentialVictims[j].Pod) })
+	// Sort potentialVictims by descending importance, which ensures reprieve of
+	// higher importance pods first.
+	pl.OrderedPods(potentialVictims)
 	// Try to reprieve as many pods as possible. We first try to reprieve the PDB
 	// violating victims and then other non-violating ones. In both cases, we start
 	// from the highest priority victims.
@@ -229,9 +250,8 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 			if err := removePod(pi); err != nil {
 				return false, err
 			}
-			rpi := pi.Pod
-			victims = append(victims, rpi)
-			logger.V(5).Info("Pod is a potential preemption victim on node", "pod", klog.KObj(rpi), "node", klog.KObj(nodeInfo.Node()))
+			victims = append(victims, pi)
+			logger.V(5).Info("Pod is a potential preemption victim on node", "pod", klog.KObj(pi.Pod), "node", klog.KObj(nodeInfo.Node()))
 		}
 		return fits, nil
 	}
@@ -251,9 +271,13 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 
 	// Sort victims after reprieving pods to keep the pods in the victims sorted in order of priority from high to low.
 	if len(violatingVictims) != 0 && len(nonViolatingVictims) != 0 {
-		sort.Slice(victims, func(i, j int) bool { return util.MoreImportantPod(victims[i], victims[j]) })
+		pl.OrderedPods(victims)
 	}
-	return victims, numViolatingVictim, framework.NewStatus(framework.Success)
+	var victimPods []*v1.Pod
+	for _, pi := range victims {
+		victimPods = append(victimPods, pi.Pod)
+	}
+	return victimPods, numViolatingVictim, framework.NewStatus(framework.Success)
 }
 
 // PodEligibleToPreemptOthers returns one bool and one string. The bool
@@ -279,9 +303,8 @@ func (pl *DefaultPreemption) PodEligibleToPreemptOthers(_ context.Context, pod *
 		}
 
 		if nodeInfo, _ := nodeInfos.Get(nomNodeName); nodeInfo != nil {
-			podPriority := corev1helpers.PodPriority(pod)
-			for _, p := range nodeInfo.Pods {
-				if corev1helpers.PodPriority(p.Pod) < podPriority && podTerminatingByPreemption(p.Pod) {
+			for _, p := range pl.EligiblePods(nodeInfo, pod) {
+				if podTerminatingByPreemption(p.Pod) {
 					// There is a terminating pod on the nominated node.
 					return false, "not eligible due to a terminating pod on the nominated node."
 				}
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index cb0b04a71e0..d8d470e19a2 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -57,12 +57,10 @@ import (
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity"
-	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/noderesources"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/podtopologyspread"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/queuesort"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration"
-	"k8s.io/kubernetes/pkg/scheduler/framework/preemption"
 	frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime"
 	"k8s.io/kubernetes/pkg/scheduler/metrics"
 	st "k8s.io/kubernetes/pkg/scheduler/testing"
@@ -436,13 +434,14 @@ func TestPostFilter(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			p := DefaultPreemption{
-				fh:        f,
-				podLister: informerFactory.Core().V1().Pods().Lister(),
-				pdbLister: getPDBLister(informerFactory),
-				args:      *getDefaultDefaultPreemptionArgs(),
+			pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			p, ok := pi.(*DefaultPreemption)
+			if !ok {
+				t.Fatal("Unable to cast to DefaultPreemption")
 			}
-			p.Evaluator = preemption.NewEvaluator(names.DefaultPreemption, f, &p, false)
 
 			state := framework.NewCycleState()
 			// Ensure <state> is populated.
@@ -1189,11 +1188,13 @@ func TestDryRunPreemption(t *testing.T) {
 			if tt.args == nil {
 				tt.args = getDefaultDefaultPreemptionArgs()
 			}
-			pl := &DefaultPreemption{
-				fh:        fwk,
-				podLister: informerFactory.Core().V1().Pods().Lister(),
-				pdbLister: getPDBLister(informerFactory),
-				args:      *tt.args,
+			pi, err := New(context.Background(), tt.args, fwk, feature.Features{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			pl, ok := pi.(*DefaultPreemption)
+			if !ok {
+				t.Fatal("Unable to cast to DefaultPreemption")
 			}
 
 			// Using 4 as a seed source to test getOffsetAndNumCandidates() deterministically.
@@ -1207,15 +1208,8 @@ func TestDryRunPreemption(t *testing.T) {
 				if _, status, _ := fwk.RunPreFilterPlugins(ctx, state, pod); !status.IsSuccess() {
 					t.Errorf("cycle %d: Unexpected PreFilter Status: %v", cycle, status)
 				}
-				pe := preemption.Evaluator{
-					PluginName: names.DefaultPreemption,
-					Handler:    pl.fh,
-					PodLister:  pl.podLister,
-					PdbLister:  pl.pdbLister,
-					Interface:  pl,
-				}
 				offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
-				got, _, _ := pe.DryRunPreemption(ctx, state, pod, nodeInfos, tt.pdbs, offset, numCandidates)
+				got, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, pod, nodeInfos, tt.pdbs, offset, numCandidates)
 				// Sort the values (inner victims) and the candidate itself (by its NominatedNodeName).
 				for i := range got {
 					victims := got[i].Victims().Pods
@@ -1425,6 +1419,7 @@ func TestSelectBestCandidate(t *testing.T) {
 				"",
 				frameworkruntime.WithPodNominator(internalqueue.NewSchedulingQueue(nil, informerFactory)),
 				frameworkruntime.WithSnapshotSharedLister(snapshot),
+				frameworkruntime.WithInformerFactory(informerFactory),
 				frameworkruntime.WithLogger(logger),
 			)
 			if err != nil {
@@ -1441,22 +1436,17 @@ func TestSelectBestCandidate(t *testing.T) {
 				t.Errorf("Unexpected PreFilter Status: %v", status)
 			}
 
-			pl := &DefaultPreemption{
-				fh:        fwk,
-				podLister: informerFactory.Core().V1().Pods().Lister(),
-				pdbLister: getPDBLister(informerFactory),
-				args:      *getDefaultDefaultPreemptionArgs(),
+			pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			if err != nil {
+				t.Fatal(err)
 			}
-			pe := preemption.Evaluator{
-				PluginName: names.DefaultPreemption,
-				Handler:    pl.fh,
-				PodLister:  pl.podLister,
-				PdbLister:  pl.pdbLister,
-				Interface:  pl,
+			pl, ok := pi.(*DefaultPreemption)
+			if !ok {
+				t.Fatal("Unable to cast to DefaultPreemption")
 			}
 			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
-			candidates, _, _ := pe.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)
-			s := pe.SelectCandidate(ctx, candidates)
+			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)
+			s := pl.Evaluator.SelectCandidate(ctx, candidates)
 			if s == nil || len(s.Name()) == 0 {
 				return
 			}
@@ -1534,18 +1524,33 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
 			for _, n := range test.nodes {
 				nodes = append(nodes, st.MakeNode().Name(n).Obj())
 			}
+			var pods []runtime.Object
+			pods = append(pods, test.pod)
+			for _, pod := range test.pods {
+				pods = append(pods, pod)
+			}
+			cs := clientsetfake.NewClientset(pods...)
+			informerFactory := informers.NewSharedInformerFactory(cs, 0)
 			registeredPlugins := []tf.RegisterPluginFunc{
 				tf.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
 				tf.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
 			}
 			f, err := tf.NewFramework(ctx, registeredPlugins, "",
 				frameworkruntime.WithSnapshotSharedLister(internalcache.NewSnapshot(test.pods, nodes)),
+				frameworkruntime.WithInformerFactory(informerFactory),
 				frameworkruntime.WithLogger(logger),
 			)
 			if err != nil {
 				t.Fatal(err)
 			}
-			pl := DefaultPreemption{fh: f, fts: test.fts}
+			pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			pl, ok := pi.(*DefaultPreemption)
+			if !ok {
+				t.Fatal("Unable to cast to DefaultPreemption")
+			}
 			if got, _ := pl.PodEligibleToPreemptOthers(ctx, test.pod, test.nominatedNodeStatus); got != test.expected {
 				t.Errorf("expected %t, got %t for pod: %s", test.expected, got, test.pod.Name)
 			}
@@ -1845,14 +1850,17 @@ func TestPreempt(t *testing.T) {
 					t.Errorf("Unexpected preFilterStatus: %v", s)
 				}
 				// Call preempt and check the expected results.
-				pl := DefaultPreemption{
-					fh:        fwk,
-					podLister: informerFactory.Core().V1().Pods().Lister(),
-					pdbLister: getPDBLister(informerFactory),
-					args:      *getDefaultDefaultPreemptionArgs(),
+				features := feature.Features{
+					EnableAsyncPreemption: asyncPreemptionEnabled,
+				}
+				pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, features)
+				if err != nil {
+					t.Fatal(err)
+				}
+				pl, ok := pi.(*DefaultPreemption)
+				if !ok {
+					t.Fatal("Unable to cast to DefaultPreemption")
 				}
-
-				pe := preemption.NewEvaluator(names.DefaultPreemption, pl.fh, &pl, asyncPreemptionEnabled)
 
 				// so that these nodes are eligible for preemption, we set their status
 				// to Unschedulable.
@@ -1862,7 +1870,7 @@ func TestPreempt(t *testing.T) {
 					nodeToStatusMap.Set(n.Name, framework.NewStatus(framework.Unschedulable))
 				}
 
-				res, status := pe.Preempt(ctx, state, testPod, nodeToStatusMap)
+				res, status := pl.Evaluator.Preempt(ctx, state, testPod, nodeToStatusMap)
 				if !status.IsSuccess() && !status.IsRejected() {
 					t.Errorf("unexpected error in preemption: %v", status.AsError())
 				}
@@ -1935,7 +1943,7 @@ func TestPreempt(t *testing.T) {
 				mu.RUnlock()
 
 				// Call preempt again and make sure it doesn't preempt any more pods.
-				res, status = pe.Preempt(ctx, state, testPod, framework.NewDefaultNodeToStatus())
+				res, status = pl.Evaluator.Preempt(ctx, state, testPod, framework.NewDefaultNodeToStatus())
 				if !status.IsSuccess() && !status.IsRejected() {
 					t.Errorf("unexpected error in preemption: %v", status.AsError())
 				}
diff --git a/pkg/scheduler/util/utils.go b/pkg/scheduler/util/utils.go
index e14b4e77453..c565aa97295 100644
--- a/pkg/scheduler/util/utils.go
+++ b/pkg/scheduler/util/utils.go
@@ -82,9 +82,8 @@ func GetEarliestPodStartTime(victims *extenderv1.Victims) *metav1.Time {
 }
 
 // MoreImportantPod return true when priority of the first pod is higher than
-// the second one. If two pods' priorities are equal, compare their StartTime.
-// It takes arguments of the type "interface{}" to be used with SortableList,
-// but expects those arguments to be *v1.Pod.
+// the second one. If two pods' priorities are equal, compare their StartTime,
+// treating the older pod as more important.
 func MoreImportantPod(pod1, pod2 *v1.Pod) bool {
 	p1 := corev1helpers.PodPriority(pod1)
 	p2 := corev1helpers.PodPriority(pod2)

From 2616202ac9eb71a3c0bfd0d154f6c224058324ce Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 7 Feb 2025 18:33:47 +1300
Subject: [PATCH 02/18] Implement tests with example customizations, add direct
 constructor

---
 .../defaultpreemption/default_preemption.go   |  24 +-
 .../default_preemption_test.go                | 311 ++++++++++++++++--
 2 files changed, 306 insertions(+), 29 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 8c9abbe9fb1..ddd4d21e8ef 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -39,6 +39,7 @@ import (
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
 	"k8s.io/kubernetes/pkg/scheduler/framework/preemption"
+	frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime"
 	"k8s.io/kubernetes/pkg/scheduler/metrics"
 	"k8s.io/kubernetes/pkg/scheduler/util"
 )
@@ -46,6 +47,16 @@ import (
 // Name of the plugin used in the plugin registry and configurations.
 const Name = names.DefaultPreemption
 
+// EligiblePodsFunc is a function which may be assigned to the DefaultPreemption plugin.
+// This function selects pods from the provided nodeInfo which are eligible to be preempted
+// in order to fit the provided preemptor.
+type EligiblePodsFunc func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo
+
+// OrderedPodsFunc is a function which may be assigned to the DefaultPreemption plugin.
+// This function orders the provided eligible pods in-place in descending order of highest
+// to lowest priority, where pods at the start of the slice are less likely to be preempted.
+type OrderedPodsFunc func(eligible []*framework.PodInfo)
+
 // DefaultPreemption is a PostFilter plugin implements the preemption logic.
 type DefaultPreemption struct {
 	fh        framework.Handle
@@ -57,11 +68,11 @@ type DefaultPreemption struct {
 
 	// EligiblePods returns victim pods which are allowed to be preempted by the provided preemptor.
 	// The default behavior is to allow any pods of lower priority to be preempted by any pods of higher priority.
-	EligiblePods func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo
+	EligiblePods EligiblePodsFunc
 
 	// OrderedPods sorts eligible victims in-place in descending order of highest to lowest priority.
 	// Pods at the start of the slice are less likely to be preempted.
-	OrderedPods func(eligible []*framework.PodInfo)
+	OrderedPods OrderedPodsFunc
 }
 
 var _ framework.PostFilterPlugin = &DefaultPreemption{}
@@ -73,7 +84,14 @@ func (pl *DefaultPreemption) Name() string {
 }
 
 // New initializes a new plugin and returns it.
-func New(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (framework.Plugin, error) {
+func New(ctx context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (framework.Plugin, error) {
+	return NewDefaultPreemption(ctx, dpArgs, fh, fts)
+}
+
+var _ frameworkruntime.PluginFactoryWithFts = New
+
+// NewDefaultPreemption initializes a new plugin and returns it. The plugin type is retained to allow modification.
+func NewDefaultPreemption(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (*DefaultPreemption, error) {
 	args, ok := dpArgs.(*config.DefaultPreemptionArgs)
 	if !ok {
 		return nil, fmt.Errorf("got args of type %T, want *DefaultPreemptionArgs", dpArgs)
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index d8d470e19a2..d0bd5a0465e 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"math"
 	"math/rand"
+	"reflect"
 	"sort"
 	"strings"
 	"sync"
@@ -43,6 +44,7 @@ import (
 	clientsetfake "k8s.io/client-go/kubernetes/fake"
 	clienttesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/events"
+	corev1helpers "k8s.io/component-helpers/scheduling/corev1"
 	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/ktesting"
 	kubeschedulerconfigv1 "k8s.io/kube-scheduler/config/v1"
@@ -65,6 +67,7 @@ import (
 	"k8s.io/kubernetes/pkg/scheduler/metrics"
 	st "k8s.io/kubernetes/pkg/scheduler/testing"
 	tf "k8s.io/kubernetes/pkg/scheduler/testing/framework"
+	"k8s.io/kubernetes/pkg/scheduler/util"
 )
 
 var (
@@ -434,14 +437,10 @@ func TestPostFilter(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			p, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
-			p, ok := pi.(*DefaultPreemption)
-			if !ok {
-				t.Fatal("Unable to cast to DefaultPreemption")
-			}
 
 			state := framework.NewCycleState()
 			// Ensure <state> is populated.
@@ -1188,14 +1187,10 @@ func TestDryRunPreemption(t *testing.T) {
 			if tt.args == nil {
 				tt.args = getDefaultDefaultPreemptionArgs()
 			}
-			pi, err := New(context.Background(), tt.args, fwk, feature.Features{})
+			pl, err := NewDefaultPreemption(context.Background(), tt.args, fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
-			pl, ok := pi.(*DefaultPreemption)
-			if !ok {
-				t.Fatal("Unable to cast to DefaultPreemption")
-			}
 
 			// Using 4 as a seed source to test getOffsetAndNumCandidates() deterministically.
 			// However, we need to do it after informerFactory.WaitforCacheSync() which might
@@ -1436,19 +1431,20 @@ func TestSelectBestCandidate(t *testing.T) {
 				t.Errorf("Unexpected PreFilter Status: %v", status)
 			}
 
-			pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
-			pl, ok := pi.(*DefaultPreemption)
-			if !ok {
-				t.Fatal("Unable to cast to DefaultPreemption")
-			}
 			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
 			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)
 			s := pl.Evaluator.SelectCandidate(ctx, candidates)
-			if s == nil || len(s.Name()) == 0 {
+			if len(tt.expected) == 0 {
+				if s != nil {
+					t.Errorf("expected no candidates, but got %v", s.Name())
+				}
 				return
+			} else if s == nil || len(s.Name()) == 0 {
+				t.Fatalf("expected any node in %v, but candidate is missing", tt.expected)
 			}
 			found := false
 			for _, nodeName := range tt.expected {
@@ -1464,6 +1460,277 @@ func TestSelectBestCandidate(t *testing.T) {
 	}
 }
 
+func TestCustomSelection(t *testing.T) {
+	labelIsEligible := func(key, val string) EligiblePodsFunc {
+		return func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo {
+			eligible := []*framework.PodInfo{}
+			for _, p := range nodeInfo.Pods {
+				pval, ok := p.Pod.Labels[key]
+				if !ok {
+					continue
+				}
+				if pval == val {
+					eligible = append(eligible, p)
+				}
+			}
+			return eligible
+		}
+	}
+	priorityBelowThresholdCannotPreempt := func(minPreempting int32) EligiblePodsFunc {
+		return func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo {
+			priority := corev1helpers.PodPriority(preemptor)
+			if priority >= minPreempting {
+				// For this example we allow all pods to be preempted, regardless of their priority
+				return nodeInfo.Pods
+			}
+			return []*framework.PodInfo{}
+		}
+	}
+	priorityAboveThresholdCannotBePreempted := func(maxPreemptible int32) EligiblePodsFunc {
+		return func(nodeInfo *framework.NodeInfo, _ *v1.Pod) []*framework.PodInfo {
+			eligible := []*framework.PodInfo{}
+			for _, p := range nodeInfo.Pods {
+				if corev1helpers.PodPriority(p.Pod) <= maxPreemptible {
+					// For this example we allow all pods to be preempted, regardless of preemptor priority
+					eligible = append(eligible, p)
+				}
+			}
+			return eligible
+		}
+	}
+
+	orderByOldestStart := func(eligible []*framework.PodInfo) {
+		sort.Slice(eligible, func(i, j int) bool { return util.GetPodStartTime(eligible[i].Pod).Before(util.GetPodStartTime(eligible[j].Pod)) })
+	}
+	orderByPodName := func(eligible []*framework.PodInfo) {
+		sort.Slice(eligible, func(i, j int) bool { return eligible[i].Pod.Name < eligible[j].Pod.Name })
+	}
+
+	tests := []struct {
+		name         string
+		eligiblePods EligiblePodsFunc
+		orderedPods  OrderedPodsFunc
+		nodeNames    []string
+		pod          *v1.Pod
+		pods         []*v1.Pod
+		expected     map[string][]string
+	}{
+		{
+			name:         "only pods with specified label are preemptable: high priority",
+			eligiblePods: labelIsEligible("preemptible", "yes"),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p1").UID("p1").Priority(highPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Label("preemptible", "no").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// must have "preemptible"="yes" label
+			expected: map[string][]string{"node2": {"v2"}},
+		},
+		{
+			name:         "only pods with specified label are preemptable: mid priority",
+			eligiblePods: labelIsEligible("preemptible", "yes"),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p2").UID("p2").Priority(midPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Label("preemptible", "no").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// midPriority can preempt equal priority
+			expected: map[string][]string{"node2": {"v2"}},
+		},
+		{
+			name:         "only pods with specified label are preemptable: low priority",
+			eligiblePods: labelIsEligible("preemptible", "yes"),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p3").UID("p3").Priority(lowPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Label("preemptible", "no").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// lowPriority can preempt higher priority
+			expected: map[string][]string{"node2": {"v2"}},
+		},
+		{
+			name:         "only pods at or above specified priority can preempted: high priority",
+			eligiblePods: priorityBelowThresholdCannotPreempt(highPriority),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p1").UID("p1").Priority(highPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// highPriority can preempt anything, including other highPriority
+			expected: map[string][]string{"node1": {"v1"}, "node2": {"v2"}, "node3": {"v3"}},
+		},
+		{
+			name:         "only pods at or above specified priority can preempted: mid priority",
+			eligiblePods: priorityBelowThresholdCannotPreempt(highPriority),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p2").UID("p2").Priority(midPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// midPriority can't preempt anything
+			expected: map[string][]string{},
+		},
+		{
+			name:         "only pods at or above specified priority can preempted: low priority",
+			eligiblePods: priorityBelowThresholdCannotPreempt(highPriority),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p3").UID("p3").Priority(lowPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// lowPriority can't preempt anything
+			expected: map[string][]string{},
+		},
+		{
+			name:         "only pods at or below specified priority can be preempted: high priority",
+			eligiblePods: priorityAboveThresholdCannotBePreempted(midPriority),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p1").UID("p1").Priority(highPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// the lowPriority pod can always be preempted regardless of preemptor priority
+			expected: map[string][]string{"node3": {"v3"}},
+		},
+		{
+			name:         "only pods at or below specified priority can be preempted: low priority",
+			eligiblePods: priorityAboveThresholdCannotBePreempted(midPriority),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p2").UID("p2").Priority(lowPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// the lowPriority pod can always be preempted regardless of preemptor priority
+			expected: map[string][]string{"node3": {"v3"}},
+		},
+		{
+			name:        "select newest pods",
+			orderedPods: orderByOldestStart,
+			nodeNames:   []string{"node1"},
+			pod:         st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
+			// size victims to require at least two to be preempted
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime2).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node1").Priority(midPriority).Req(mediumRes).StartTime(epochTime1).Obj(),
+			},
+			// the newest two pods are selected, despite one with higher priority
+			expected: map[string][]string{"node1": {"v3", "v1"}},
+		},
+		{
+			name:        "select alphabetically-last pods",
+			orderedPods: orderByPodName,
+			nodeNames:   []string{"node1"},
+			pod:         st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
+			// size victims to require at least two to be preempted
+			pods: []*v1.Pod{
+				st.MakePod().Name("foo").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("bar").UID("v2").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("baz").UID("v3").Node("node1").Priority(midPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+			},
+			// the last pods in alphabetic order are selected, despite one with higher priority
+			expected: map[string][]string{"node1": {"baz", "foo"}},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rand.Seed(4)
+			nodes := make([]*v1.Node, len(tt.nodeNames))
+			for i, nodeName := range tt.nodeNames {
+				nodes[i] = st.MakeNode().Name(nodeName).Capacity(veryLargeRes).Obj()
+			}
+
+			var objs []runtime.Object
+			objs = append(objs, tt.pod)
+			for _, pod := range tt.pods {
+				objs = append(objs, pod)
+			}
+			cs := clientsetfake.NewClientset(objs...)
+			informerFactory := informers.NewSharedInformerFactory(cs, 0)
+			snapshot := internalcache.NewSnapshot(tt.pods, nodes)
+			logger, ctx := ktesting.NewTestContext(t)
+			ctx, cancel := context.WithCancel(ctx)
+			defer cancel()
+			fwk, err := tf.NewFramework(
+				ctx,
+				[]tf.RegisterPluginFunc{
+					tf.RegisterPluginAsExtensions(noderesources.Name, nodeResourcesFitFunc, "Filter", "PreFilter"),
+					tf.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
+					tf.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
+				},
+				"",
+				frameworkruntime.WithPodNominator(internalqueue.NewSchedulingQueue(nil, informerFactory)),
+				frameworkruntime.WithSnapshotSharedLister(snapshot),
+				frameworkruntime.WithInformerFactory(informerFactory),
+				frameworkruntime.WithLogger(logger),
+			)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			state := framework.NewCycleState()
+			// Some tests rely on PreFilter plugin to compute its CycleState.
+			if _, status, _ := fwk.RunPreFilterPlugins(ctx, state, tt.pod); !status.IsSuccess() {
+				t.Errorf("Unexpected PreFilter Status: %v", status)
+			}
+			nodeInfos, err := snapshot.NodeInfos().List()
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			// Override selection logic
+			if tt.eligiblePods != nil {
+				pl.EligiblePods = tt.eligiblePods
+			}
+			if tt.orderedPods != nil {
+				pl.OrderedPods = tt.orderedPods
+			}
+			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
+			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)
+			s := pl.Evaluator.SelectCandidate(ctx, candidates)
+			if len(tt.expected) == 0 {
+				if s != nil {
+					t.Errorf("expected no candidates, but got %v", s.Name())
+				}
+				return
+			} else if s == nil || len(s.Name()) == 0 {
+				t.Fatalf("expected any node in %v, but candidate is missing", tt.expected)
+			}
+			expectPodNames, ok := tt.expected[s.Name()]
+			if !ok {
+				t.Errorf("expect any node in %v, but got %v", tt.expected, s.Name())
+			}
+			gotPodNames := []string{}
+			for _, p := range s.Victims().Pods {
+				gotPodNames = append(gotPodNames, p.Name)
+			}
+			if !reflect.DeepEqual(expectPodNames, gotPodNames) {
+				t.Errorf("expected pods %v, but got %v", expectPodNames, gotPodNames)
+			}
+		})
+	}
+}
+
 func TestPodEligibleToPreemptOthers(t *testing.T) {
 	tests := []struct {
 		name                string
@@ -1543,14 +1810,10 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
-			pl, ok := pi.(*DefaultPreemption)
-			if !ok {
-				t.Fatal("Unable to cast to DefaultPreemption")
-			}
 			if got, _ := pl.PodEligibleToPreemptOthers(ctx, test.pod, test.nominatedNodeStatus); got != test.expected {
 				t.Errorf("expected %t, got %t for pod: %s", test.expected, got, test.pod.Name)
 			}
@@ -1853,14 +2116,10 @@ func TestPreempt(t *testing.T) {
 				features := feature.Features{
 					EnableAsyncPreemption: asyncPreemptionEnabled,
 				}
-				pi, err := New(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, features)
+				pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, features)
 				if err != nil {
 					t.Fatal(err)
 				}
-				pl, ok := pi.(*DefaultPreemption)
-				if !ok {
-					t.Fatal("Unable to cast to DefaultPreemption")
-				}
 
 				// so that these nodes are eligible for preemption, we set their status
 				// to Unschedulable.

From 78b059c064e540c21f57645a3d3f7e8e2a90efe4 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 7 Feb 2025 18:45:21 +1300
Subject: [PATCH 03/18] rename OrderedPods -> OrderPods

---
 .../defaultpreemption/default_preemption.go   | 14 ++++++------
 .../default_preemption_test.go                | 22 +++++++++----------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index ddd4d21e8ef..09de2fe2eba 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -52,10 +52,10 @@ const Name = names.DefaultPreemption
 // in order to fit the provided preemptor.
 type EligiblePodsFunc func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo
 
-// OrderedPodsFunc is a function which may be assigned to the DefaultPreemption plugin.
+// OrderPodsFunc is a function which may be assigned to the DefaultPreemption plugin.
 // This function orders the provided eligible pods in-place in descending order of highest
 // to lowest priority, where pods at the start of the slice are less likely to be preempted.
-type OrderedPodsFunc func(eligible []*framework.PodInfo)
+type OrderPodsFunc func(eligible []*framework.PodInfo)
 
 // DefaultPreemption is a PostFilter plugin implements the preemption logic.
 type DefaultPreemption struct {
@@ -70,9 +70,9 @@ type DefaultPreemption struct {
 	// The default behavior is to allow any pods of lower priority to be preempted by any pods of higher priority.
 	EligiblePods EligiblePodsFunc
 
-	// OrderedPods sorts eligible victims in-place in descending order of highest to lowest priority.
+	// OrderPods sorts eligible victims in-place in descending order of highest to lowest priority.
 	// Pods at the start of the slice are less likely to be preempted.
-	OrderedPods OrderedPodsFunc
+	OrderPods OrderPodsFunc
 }
 
 var _ framework.PostFilterPlugin = &DefaultPreemption{}
@@ -125,7 +125,7 @@ func NewDefaultPreemption(_ context.Context, dpArgs runtime.Object, fh framework
 	}
 
 	// Default behavior: Sort by descending priority, then by descending runtime duration as secondary ordering.
-	pl.OrderedPods = func(eligible []*framework.PodInfo) {
+	pl.OrderPods = func(eligible []*framework.PodInfo) {
 		sort.Slice(eligible, func(i, j int) bool { return util.MoreImportantPod(eligible[i].Pod, eligible[j].Pod) })
 	}
 
@@ -253,7 +253,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	numViolatingVictim := 0
 	// Sort potentialVictims by descending importance, which ensures reprieve of
 	// higher importance pods first.
-	pl.OrderedPods(potentialVictims)
+	pl.OrderPods(potentialVictims)
 	// Try to reprieve as many pods as possible. We first try to reprieve the PDB
 	// violating victims and then other non-violating ones. In both cases, we start
 	// from the highest priority victims.
@@ -289,7 +289,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 
 	// Sort victims after reprieving pods to keep the pods in the victims sorted in order of priority from high to low.
 	if len(violatingVictims) != 0 && len(nonViolatingVictims) != 0 {
-		pl.OrderedPods(victims)
+		pl.OrderPods(victims)
 	}
 	var victimPods []*v1.Pod
 	for _, pi := range victims {
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index d0bd5a0465e..d63e157abcf 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -1509,7 +1509,7 @@ func TestCustomSelection(t *testing.T) {
 	tests := []struct {
 		name         string
 		eligiblePods EligiblePodsFunc
-		orderedPods  OrderedPodsFunc
+		orderPods  OrderPodsFunc
 		nodeNames    []string
 		pod          *v1.Pod
 		pods         []*v1.Pod
@@ -1620,10 +1620,10 @@ func TestCustomSelection(t *testing.T) {
 			expected: map[string][]string{"node3": {"v3"}},
 		},
 		{
-			name:        "select newest pods",
-			orderedPods: orderByOldestStart,
-			nodeNames:   []string{"node1"},
-			pod:         st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
+			name:      "select newest pods",
+			orderPods: orderByOldestStart,
+			nodeNames: []string{"node1"},
+			pod:       st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
 			// size victims to require at least two to be preempted
 			pods: []*v1.Pod{
 				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime2).Obj(),
@@ -1634,10 +1634,10 @@ func TestCustomSelection(t *testing.T) {
 			expected: map[string][]string{"node1": {"v3", "v1"}},
 		},
 		{
-			name:        "select alphabetically-last pods",
-			orderedPods: orderByPodName,
-			nodeNames:   []string{"node1"},
-			pod:         st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
+			name:      "select alphabetically-last pods",
+			orderPods: orderByPodName,
+			nodeNames: []string{"node1"},
+			pod:       st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
 			// size victims to require at least two to be preempted
 			pods: []*v1.Pod{
 				st.MakePod().Name("foo").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
@@ -1702,8 +1702,8 @@ func TestCustomSelection(t *testing.T) {
 			if tt.eligiblePods != nil {
 				pl.EligiblePods = tt.eligiblePods
 			}
-			if tt.orderedPods != nil {
-				pl.OrderedPods = tt.orderedPods
+			if tt.orderPods != nil {
+				pl.OrderPods = tt.orderPods
 			}
 			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
 			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)

From c6f2d3879b800fae7873cbd1169abc51b82e77aa Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 7 Feb 2025 20:23:25 +1300
Subject: [PATCH 04/18] Fix gofmt in default_preemption_test.go

---
 .../plugins/defaultpreemption/default_preemption_test.go    | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index d63e157abcf..fa52d984975 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -1500,7 +1500,9 @@ func TestCustomSelection(t *testing.T) {
 	}
 
 	orderByOldestStart := func(eligible []*framework.PodInfo) {
-		sort.Slice(eligible, func(i, j int) bool { return util.GetPodStartTime(eligible[i].Pod).Before(util.GetPodStartTime(eligible[j].Pod)) })
+		sort.Slice(eligible, func(i, j int) bool {
+			return util.GetPodStartTime(eligible[i].Pod).Before(util.GetPodStartTime(eligible[j].Pod))
+		})
 	}
 	orderByPodName := func(eligible []*framework.PodInfo) {
 		sort.Slice(eligible, func(i, j int) bool { return eligible[i].Pod.Name < eligible[j].Pod.Name })
@@ -1509,7 +1511,7 @@ func TestCustomSelection(t *testing.T) {
 	tests := []struct {
 		name         string
 		eligiblePods EligiblePodsFunc
-		orderPods  OrderPodsFunc
+		orderPods    OrderPodsFunc
 		nodeNames    []string
 		pod          *v1.Pod
 		pods         []*v1.Pod

From c34f8db5594c15fcc6f7845ae8cf5bc734cb1be0 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Mon, 10 Feb 2025 09:30:22 +1300
Subject: [PATCH 05/18] Remove unnecessary context and typecheck, switch to
 cmp.Diff

---
 .../defaultpreemption/default_preemption.go       |  9 +++------
 .../defaultpreemption/default_preemption_test.go  | 15 +++++++--------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 09de2fe2eba..24f564fd31e 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -39,7 +39,6 @@ import (
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
 	"k8s.io/kubernetes/pkg/scheduler/framework/preemption"
-	frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime"
 	"k8s.io/kubernetes/pkg/scheduler/metrics"
 	"k8s.io/kubernetes/pkg/scheduler/util"
 )
@@ -84,14 +83,12 @@ func (pl *DefaultPreemption) Name() string {
 }
 
 // New initializes a new plugin and returns it.
-func New(ctx context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (framework.Plugin, error) {
-	return NewDefaultPreemption(ctx, dpArgs, fh, fts)
+func New(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (framework.Plugin, error) {
+	return NewDefaultPreemption(dpArgs, fh, fts)
 }
 
-var _ frameworkruntime.PluginFactoryWithFts = New
-
 // NewDefaultPreemption initializes a new plugin and returns it. The plugin type is retained to allow modification.
-func NewDefaultPreemption(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (*DefaultPreemption, error) {
+func NewDefaultPreemption(dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (*DefaultPreemption, error) {
 	args, ok := dpArgs.(*config.DefaultPreemptionArgs)
 	if !ok {
 		return nil, fmt.Errorf("got args of type %T, want *DefaultPreemptionArgs", dpArgs)
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index fa52d984975..8a0792bd14c 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"math"
 	"math/rand"
-	"reflect"
 	"sort"
 	"strings"
 	"sync"
@@ -437,7 +436,7 @@ func TestPostFilter(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			p, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			p, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), f, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1187,7 +1186,7 @@ func TestDryRunPreemption(t *testing.T) {
 			if tt.args == nil {
 				tt.args = getDefaultDefaultPreemptionArgs()
 			}
-			pl, err := NewDefaultPreemption(context.Background(), tt.args, fwk, feature.Features{})
+			pl, err := NewDefaultPreemption(tt.args, fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1431,7 +1430,7 @@ func TestSelectBestCandidate(t *testing.T) {
 				t.Errorf("Unexpected PreFilter Status: %v", status)
 			}
 
-			pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1696,7 +1695,7 @@ func TestCustomSelection(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1726,7 +1725,7 @@ func TestCustomSelection(t *testing.T) {
 			for _, p := range s.Victims().Pods {
 				gotPodNames = append(gotPodNames, p.Name)
 			}
-			if !reflect.DeepEqual(expectPodNames, gotPodNames) {
+			if diff := cmp.Diff(expectPodNames, gotPodNames); diff != "" {
 				t.Errorf("expected pods %v, but got %v", expectPodNames, gotPodNames)
 			}
 		})
@@ -1812,7 +1811,7 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), f, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -2118,7 +2117,7 @@ func TestPreempt(t *testing.T) {
 				features := feature.Features{
 					EnableAsyncPreemption: asyncPreemptionEnabled,
 				}
-				pl, err := NewDefaultPreemption(context.Background(), getDefaultDefaultPreemptionArgs(), fwk, features)
+				pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), fwk, features)
 				if err != nil {
 					t.Fatal(err)
 				}

From 4bf6841495e75326fb163333c8b018050e7e87af Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Wed, 26 Feb 2025 13:58:19 +1300
Subject: [PATCH 06/18] Update the customizations to operate on individual
 pods, for more flexibility later

---
 .../defaultpreemption/default_preemption.go   | 65 +++++++-------
 .../default_preemption_test.go                | 90 +++++++++----------
 2 files changed, 74 insertions(+), 81 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 24f564fd31e..0c657caaf79 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -46,15 +46,15 @@ import (
 // Name of the plugin used in the plugin registry and configurations.
 const Name = names.DefaultPreemption
 
-// EligiblePodsFunc is a function which may be assigned to the DefaultPreemption plugin.
-// This function selects pods from the provided nodeInfo which are eligible to be preempted
-// in order to fit the provided preemptor.
-type EligiblePodsFunc func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo
+// EligiblePodFunc is a function which may be assigned to the DefaultPreemption plugin.
+// This function returns whether a given victim pod on the provided nodeInfo is eligible
+// to be preempted by the provided preemptor.
+type EligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
 
-// OrderPodsFunc is a function which may be assigned to the DefaultPreemption plugin.
-// This function orders the provided eligible pods in-place in descending order of highest
-// to lowest priority, where pods at the start of the slice are less likely to be preempted.
-type OrderPodsFunc func(eligible []*framework.PodInfo)
+// MoreImportantPodFunc is a function which may be assigned to the DefaultPreemption plugin.
+// This function returns true if the priority of the first pod is higher than the second pod.
+// If the two pods are of equal priority then this will return false.
+type MoreImportantPodFunc func(pod1, pod2 *v1.Pod) bool
 
 // DefaultPreemption is a PostFilter plugin implements the preemption logic.
 type DefaultPreemption struct {
@@ -65,13 +65,14 @@ type DefaultPreemption struct {
 	pdbLister policylisters.PodDisruptionBudgetLister
 	Evaluator *preemption.Evaluator
 
-	// EligiblePods returns victim pods which are allowed to be preempted by the provided preemptor.
-	// The default behavior is to allow any pods of lower priority to be preempted by any pods of higher priority.
-	EligiblePods EligiblePodsFunc
+	// EligiblePod returns whether a victim pod is allowed to be preempted by a preemptor pod.
+	// The default behavior is to allow any pods of lower priority to be preempted by any pods
+	// of higher priority.
+	EligiblePod EligiblePodFunc
 
-	// OrderPods sorts eligible victims in-place in descending order of highest to lowest priority.
-	// Pods at the start of the slice are less likely to be preempted.
-	OrderPods OrderPodsFunc
+	// MoreImportantPod is used to sort eligible victims in-place in descending order of highest to
+	// lowest importance. Pods with higher importance are less likely to be preempted.
+	MoreImportantPod MoreImportantPodFunc
 }
 
 var _ framework.PostFilterPlugin = &DefaultPreemption{}
@@ -110,21 +111,12 @@ func NewDefaultPreemption(dpArgs runtime.Object, fh framework.Handle, fts featur
 	pl.Evaluator = preemption.NewEvaluator(Name, fh, &pl, fts.EnableAsyncPreemption)
 
 	// Default behavior: Any pods of lower priority may be preempted by any pods of higher priority.
-	pl.EligiblePods = func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo {
-		var eligible []*framework.PodInfo
-		podPriority := corev1helpers.PodPriority(preemptor)
-		for _, pi := range nodeInfo.Pods {
-			if corev1helpers.PodPriority(pi.Pod) < podPriority {
-				eligible = append(eligible, pi)
-			}
-		}
-		return eligible
+	pl.EligiblePod = func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
+		return corev1helpers.PodPriority(victim.Pod) < corev1helpers.PodPriority(preemptor)
 	}
 
 	// Default behavior: Sort by descending priority, then by descending runtime duration as secondary ordering.
-	pl.OrderPods = func(eligible []*framework.PodInfo) {
-		sort.Slice(eligible, func(i, j int) bool { return util.MoreImportantPod(eligible[i].Pod, eligible[j].Pod) })
-	}
+	pl.MoreImportantPod = util.MoreImportantPod
 
 	return &pl, nil
 }
@@ -205,6 +197,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	nodeInfo *framework.NodeInfo,
 	pdbs []*policy.PodDisruptionBudget) ([]*v1.Pod, int, *framework.Status) {
 	logger := klog.FromContext(ctx)
+	var potentialVictims []*framework.PodInfo
 	removePod := func(rpi *framework.PodInfo) error {
 		if err := nodeInfo.RemovePod(logger, rpi.Pod); err != nil {
 			return err
@@ -224,11 +217,13 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 		return nil
 	}
 	// As the first step, remove all pods eligible for preemption from the node and
-	// check if the given pod could be scheduled without them present.
-	potentialVictims := pl.EligiblePods(nodeInfo, pod)
-	for _, pi := range potentialVictims {
-		if err := removePod(pi); err != nil {
-			return nil, 0, framework.AsStatus(err)
+	// check if the given pod can be scheduled without them present.
+	for _, pi := range nodeInfo.Pods {
+		if pl.EligiblePod(nodeInfo, pi, pod) {
+			potentialVictims = append(potentialVictims, pi)
+			if err := removePod(pi); err != nil {
+				return nil, 0, framework.AsStatus(err)
+			}
 		}
 	}
 
@@ -250,7 +245,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	numViolatingVictim := 0
 	// Sort potentialVictims by descending importance, which ensures reprieve of
 	// higher importance pods first.
-	pl.OrderPods(potentialVictims)
+	sort.Slice(potentialVictims, func(i, j int) bool { return pl.MoreImportantPod(potentialVictims[i].Pod, potentialVictims[j].Pod) })
 	// Try to reprieve as many pods as possible. We first try to reprieve the PDB
 	// violating victims and then other non-violating ones. In both cases, we start
 	// from the highest priority victims.
@@ -286,7 +281,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 
 	// Sort victims after reprieving pods to keep the pods in the victims sorted in order of priority from high to low.
 	if len(violatingVictims) != 0 && len(nonViolatingVictims) != 0 {
-		pl.OrderPods(victims)
+		sort.Slice(victims, func(i, j int) bool { return pl.MoreImportantPod(victims[i].Pod, victims[j].Pod) })
 	}
 	var victimPods []*v1.Pod
 	for _, pi := range victims {
@@ -318,8 +313,8 @@ func (pl *DefaultPreemption) PodEligibleToPreemptOthers(_ context.Context, pod *
 		}
 
 		if nodeInfo, _ := nodeInfos.Get(nomNodeName); nodeInfo != nil {
-			for _, p := range pl.EligiblePods(nodeInfo, pod) {
-				if podTerminatingByPreemption(p.Pod) {
+			for _, p := range nodeInfo.Pods {
+				if pl.EligiblePod(nodeInfo, p, pod) && podTerminatingByPreemption(p.Pod) {
 					// There is a terminating pod on the nominated node.
 					return false, "not eligible due to a terminating pod on the nominated node."
 				}
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index 8a0792bd14c..d1f4cd9e316 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -1460,57 +1460,42 @@ func TestSelectBestCandidate(t *testing.T) {
 }
 
 func TestCustomSelection(t *testing.T) {
-	labelIsEligible := func(key, val string) EligiblePodsFunc {
-		return func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo {
-			eligible := []*framework.PodInfo{}
-			for _, p := range nodeInfo.Pods {
-				pval, ok := p.Pod.Labels[key]
-				if !ok {
-					continue
-				}
-				if pval == val {
-					eligible = append(eligible, p)
-				}
+	podLabelIsEligible := func(key, val string) EligiblePodFunc {
+		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
+			pval, ok := victim.Pod.Labels[key]
+			if !ok {
+				return false
 			}
-			return eligible
+			return pval == val
 		}
 	}
-	priorityBelowThresholdCannotPreempt := func(minPreempting int32) EligiblePodsFunc {
-		return func(nodeInfo *framework.NodeInfo, preemptor *v1.Pod) []*framework.PodInfo {
-			priority := corev1helpers.PodPriority(preemptor)
-			if priority >= minPreempting {
-				// For this example we allow all pods to be preempted, regardless of their priority
-				return nodeInfo.Pods
-			}
-			return []*framework.PodInfo{}
+	nodeNameIsEligible := func(name string) EligiblePodFunc {
+		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
+			return nodeInfo.Node().Name == name
 		}
 	}
-	priorityAboveThresholdCannotBePreempted := func(maxPreemptible int32) EligiblePodsFunc {
-		return func(nodeInfo *framework.NodeInfo, _ *v1.Pod) []*framework.PodInfo {
-			eligible := []*framework.PodInfo{}
-			for _, p := range nodeInfo.Pods {
-				if corev1helpers.PodPriority(p.Pod) <= maxPreemptible {
-					// For this example we allow all pods to be preempted, regardless of preemptor priority
-					eligible = append(eligible, p)
-				}
-			}
-			return eligible
+	priorityBelowThresholdCannotPreempt := func(minPreempting int32) EligiblePodFunc {
+		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
+			return corev1helpers.PodPriority(preemptor) >= minPreempting
+		}
+	}
+	priorityAboveThresholdCannotBePreempted := func(maxPreemptible int32) EligiblePodFunc {
+		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
+			return corev1helpers.PodPriority(victim.Pod) <= maxPreemptible
 		}
 	}
 
-	orderByOldestStart := func(eligible []*framework.PodInfo) {
-		sort.Slice(eligible, func(i, j int) bool {
-			return util.GetPodStartTime(eligible[i].Pod).Before(util.GetPodStartTime(eligible[j].Pod))
-		})
+	orderByOldestStart := func(pod1, pod2 *v1.Pod) bool {
+		return util.GetPodStartTime(pod1).Before(util.GetPodStartTime(pod2))
 	}
-	orderByPodName := func(eligible []*framework.PodInfo) {
-		sort.Slice(eligible, func(i, j int) bool { return eligible[i].Pod.Name < eligible[j].Pod.Name })
+	orderByPodName := func(pod1, pod2 *v1.Pod) bool {
+		return pod1.Name < pod2.Name
 	}
 
 	tests := []struct {
 		name         string
-		eligiblePods EligiblePodsFunc
-		orderPods    OrderPodsFunc
+		eligiblePods EligiblePodFunc
+		orderPods    MoreImportantPodFunc
 		nodeNames    []string
 		pod          *v1.Pod
 		pods         []*v1.Pod
@@ -1518,7 +1503,7 @@ func TestCustomSelection(t *testing.T) {
 	}{
 		{
 			name:         "only pods with specified label are preemptable: high priority",
-			eligiblePods: labelIsEligible("preemptible", "yes"),
+			eligiblePods: podLabelIsEligible("preemptible", "yes"),
 			nodeNames:    []string{"node1", "node2", "node3"},
 			pod:          st.MakePod().Name("p1").UID("p1").Priority(highPriority).Req(largeRes).Obj(),
 			pods: []*v1.Pod{
@@ -1526,12 +1511,12 @@ func TestCustomSelection(t *testing.T) {
 				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// must have "preemptible"="yes" label
+			// must have "preemptible"="yes" label, others are not preemptible despite high priority
 			expected: map[string][]string{"node2": {"v2"}},
 		},
 		{
 			name:         "only pods with specified label are preemptable: mid priority",
-			eligiblePods: labelIsEligible("preemptible", "yes"),
+			eligiblePods: podLabelIsEligible("preemptible", "yes"),
 			nodeNames:    []string{"node1", "node2", "node3"},
 			pod:          st.MakePod().Name("p2").UID("p2").Priority(midPriority).Req(largeRes).Obj(),
 			pods: []*v1.Pod{
@@ -1539,12 +1524,12 @@ func TestCustomSelection(t *testing.T) {
 				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// midPriority can preempt equal priority
+			// midPriority can preempt equal priority since we only look for matching pod label
 			expected: map[string][]string{"node2": {"v2"}},
 		},
 		{
 			name:         "only pods with specified label are preemptable: low priority",
-			eligiblePods: labelIsEligible("preemptible", "yes"),
+			eligiblePods: podLabelIsEligible("preemptible", "yes"),
 			nodeNames:    []string{"node1", "node2", "node3"},
 			pod:          st.MakePod().Name("p3").UID("p3").Priority(lowPriority).Req(largeRes).Obj(),
 			pods: []*v1.Pod{
@@ -1552,9 +1537,22 @@ func TestCustomSelection(t *testing.T) {
 				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// lowPriority can preempt higher priority
+			// lowPriority can preempt higher priority since we only look for matching pod label
 			expected: map[string][]string{"node2": {"v2"}},
 		},
+		{
+			name:         "only pods on specified node name are preemptable",
+			eligiblePods: nodeNameIsEligible("node1"),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p3").UID("p3").Priority(lowPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			// lowPriority can preempt higher priority since we only look for matching node name
+			expected: map[string][]string{"node1": {"v1"}},
+		},
 		{
 			name:         "only pods at or above specified priority can preempted: high priority",
 			eligiblePods: priorityBelowThresholdCannotPreempt(highPriority),
@@ -1701,10 +1699,10 @@ func TestCustomSelection(t *testing.T) {
 			}
 			// Override selection logic
 			if tt.eligiblePods != nil {
-				pl.EligiblePods = tt.eligiblePods
+				pl.EligiblePod = tt.eligiblePods
 			}
 			if tt.orderPods != nil {
-				pl.OrderPods = tt.orderPods
+				pl.MoreImportantPod = tt.orderPods
 			}
 			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
 			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)

From 224e6a3a355e9969821dd69b161e0cdd66574988 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Wed, 26 Feb 2025 13:59:48 +1300
Subject: [PATCH 07/18] Rename EligiblePod* to IsEligiblePod*

---
 .../defaultpreemption/default_preemption.go        | 14 +++++++-------
 .../defaultpreemption/default_preemption_test.go   | 12 ++++++------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 0c657caaf79..89a24deb99a 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -46,10 +46,10 @@ import (
 // Name of the plugin used in the plugin registry and configurations.
 const Name = names.DefaultPreemption
 
-// EligiblePodFunc is a function which may be assigned to the DefaultPreemption plugin.
+// IsEligiblePodFunc is a function which may be assigned to the DefaultPreemption plugin.
 // This function returns whether a given victim pod on the provided nodeInfo is eligible
 // to be preempted by the provided preemptor.
-type EligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
+type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
 
 // MoreImportantPodFunc is a function which may be assigned to the DefaultPreemption plugin.
 // This function returns true if the priority of the first pod is higher than the second pod.
@@ -65,10 +65,10 @@ type DefaultPreemption struct {
 	pdbLister policylisters.PodDisruptionBudgetLister
 	Evaluator *preemption.Evaluator
 
-	// EligiblePod returns whether a victim pod is allowed to be preempted by a preemptor pod.
+	// IsEligiblePod returns whether a victim pod is allowed to be preempted by a preemptor pod.
 	// The default behavior is to allow any pods of lower priority to be preempted by any pods
 	// of higher priority.
-	EligiblePod EligiblePodFunc
+	IsEligiblePod IsEligiblePodFunc
 
 	// MoreImportantPod is used to sort eligible victims in-place in descending order of highest to
 	// lowest importance. Pods with higher importance are less likely to be preempted.
@@ -111,7 +111,7 @@ func NewDefaultPreemption(dpArgs runtime.Object, fh framework.Handle, fts featur
 	pl.Evaluator = preemption.NewEvaluator(Name, fh, &pl, fts.EnableAsyncPreemption)
 
 	// Default behavior: Any pods of lower priority may be preempted by any pods of higher priority.
-	pl.EligiblePod = func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
+	pl.IsEligiblePod = func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
 		return corev1helpers.PodPriority(victim.Pod) < corev1helpers.PodPriority(preemptor)
 	}
 
@@ -219,7 +219,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	// As the first step, remove all pods eligible for preemption from the node and
 	// check if the given pod can be scheduled without them present.
 	for _, pi := range nodeInfo.Pods {
-		if pl.EligiblePod(nodeInfo, pi, pod) {
+		if pl.IsEligiblePod(nodeInfo, pi, pod) {
 			potentialVictims = append(potentialVictims, pi)
 			if err := removePod(pi); err != nil {
 				return nil, 0, framework.AsStatus(err)
@@ -314,7 +314,7 @@ func (pl *DefaultPreemption) PodEligibleToPreemptOthers(_ context.Context, pod *
 
 		if nodeInfo, _ := nodeInfos.Get(nomNodeName); nodeInfo != nil {
 			for _, p := range nodeInfo.Pods {
-				if pl.EligiblePod(nodeInfo, p, pod) && podTerminatingByPreemption(p.Pod) {
+				if pl.IsEligiblePod(nodeInfo, p, pod) && podTerminatingByPreemption(p.Pod) {
 					// There is a terminating pod on the nominated node.
 					return false, "not eligible due to a terminating pod on the nominated node."
 				}
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index d1f4cd9e316..e8f3b903eb5 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -1460,7 +1460,7 @@ func TestSelectBestCandidate(t *testing.T) {
 }
 
 func TestCustomSelection(t *testing.T) {
-	podLabelIsEligible := func(key, val string) EligiblePodFunc {
+	podLabelIsEligible := func(key, val string) IsEligiblePodFunc {
 		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
 			pval, ok := victim.Pod.Labels[key]
 			if !ok {
@@ -1469,17 +1469,17 @@ func TestCustomSelection(t *testing.T) {
 			return pval == val
 		}
 	}
-	nodeNameIsEligible := func(name string) EligiblePodFunc {
+	nodeNameIsEligible := func(name string) IsEligiblePodFunc {
 		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
 			return nodeInfo.Node().Name == name
 		}
 	}
-	priorityBelowThresholdCannotPreempt := func(minPreempting int32) EligiblePodFunc {
+	priorityBelowThresholdCannotPreempt := func(minPreempting int32) IsEligiblePodFunc {
 		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
 			return corev1helpers.PodPriority(preemptor) >= minPreempting
 		}
 	}
-	priorityAboveThresholdCannotBePreempted := func(maxPreemptible int32) EligiblePodFunc {
+	priorityAboveThresholdCannotBePreempted := func(maxPreemptible int32) IsEligiblePodFunc {
 		return func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
 			return corev1helpers.PodPriority(victim.Pod) <= maxPreemptible
 		}
@@ -1494,7 +1494,7 @@ func TestCustomSelection(t *testing.T) {
 
 	tests := []struct {
 		name         string
-		eligiblePods EligiblePodFunc
+		eligiblePods IsEligiblePodFunc
 		orderPods    MoreImportantPodFunc
 		nodeNames    []string
 		pod          *v1.Pod
@@ -1699,7 +1699,7 @@ func TestCustomSelection(t *testing.T) {
 			}
 			// Override selection logic
 			if tt.eligiblePods != nil {
-				pl.EligiblePod = tt.eligiblePods
+				pl.IsEligiblePod = tt.eligiblePods
 			}
 			if tt.orderPods != nil {
 				pl.MoreImportantPod = tt.orderPods

From b8ac1714370f9d2d195c6e9af3cad35ae2a9d421 Mon Sep 17 00:00:00 2001
From: Nicholas Parker <nick@nickbp.com>
Date: Thu, 27 Feb 2025 23:38:44 +1300
Subject: [PATCH 08/18] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Dominik Marciński <gmidon@gmail.com>
---
 .../framework/plugins/defaultpreemption/default_preemption.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 89a24deb99a..bfd22297e37 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -47,12 +47,12 @@ import (
 const Name = names.DefaultPreemption
 
 // IsEligiblePodFunc is a function which may be assigned to the DefaultPreemption plugin.
-// This function returns whether a given victim pod on the provided nodeInfo is eligible
+// Implemenmtations should return whether a given victim pod on the provided nodeInfo is eligible
 // to be preempted by the provided preemptor.
 type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
 
 // MoreImportantPodFunc is a function which may be assigned to the DefaultPreemption plugin.
-// This function returns true if the priority of the first pod is higher than the second pod.
+// The implementations should return true if the first pod is more important than the second pod and the second one should be considered for preemption before the first one.
 // If the two pods are of equal priority then this will return false.
 type MoreImportantPodFunc func(pod1, pod2 *v1.Pod) bool
 

From 137da6a48821dbe29e7c8e13904b3564bfc3816d Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Thu, 27 Feb 2025 23:40:09 +1300
Subject: [PATCH 09/18] Remove line about equal priority, fix typo

---
 .../framework/plugins/defaultpreemption/default_preemption.go  | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index bfd22297e37..fbe76bc89e2 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -47,13 +47,12 @@ import (
 const Name = names.DefaultPreemption
 
 // IsEligiblePodFunc is a function which may be assigned to the DefaultPreemption plugin.
-// Implemenmtations should return whether a given victim pod on the provided nodeInfo is eligible
+// Implementations should return whether a given victim pod on the provided nodeInfo is eligible
 // to be preempted by the provided preemptor.
 type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
 
 // MoreImportantPodFunc is a function which may be assigned to the DefaultPreemption plugin.
 // The implementations should return true if the first pod is more important than the second pod and the second one should be considered for preemption before the first one.
-// If the two pods are of equal priority then this will return false.
 type MoreImportantPodFunc func(pod1, pod2 *v1.Pod) bool
 
 // DefaultPreemption is a PostFilter plugin implements the preemption logic.

From 7f57c6e52dc862c22feca1c2d7fb4f8fc77d0765 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Thu, 27 Feb 2025 23:55:17 +1300
Subject: [PATCH 10/18] Update factory to use generics, keep single New
 function

---
 .../plugins/defaultpreemption/default_preemption.go  |  9 ++-------
 .../defaultpreemption/default_preemption_test.go     | 12 ++++++------
 pkg/scheduler/framework/runtime/registry.go          |  4 ++--
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index fbe76bc89e2..0a145130416 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -82,13 +82,8 @@ func (pl *DefaultPreemption) Name() string {
 	return Name
 }
 
-// New initializes a new plugin and returns it.
-func New(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (framework.Plugin, error) {
-	return NewDefaultPreemption(dpArgs, fh, fts)
-}
-
-// NewDefaultPreemption initializes a new plugin and returns it. The plugin type is retained to allow modification.
-func NewDefaultPreemption(dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (*DefaultPreemption, error) {
+// New initializes a new plugin and returns it. The plugin type is retained to allow modification.
+func New(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feature.Features) (*DefaultPreemption, error) {
 	args, ok := dpArgs.(*config.DefaultPreemptionArgs)
 	if !ok {
 		return nil, fmt.Errorf("got args of type %T, want *DefaultPreemptionArgs", dpArgs)
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index e8f3b903eb5..9be143b479c 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -436,7 +436,7 @@ func TestPostFilter(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			p, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			p, err := New(ctx, getDefaultDefaultPreemptionArgs(), f, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1186,7 +1186,7 @@ func TestDryRunPreemption(t *testing.T) {
 			if tt.args == nil {
 				tt.args = getDefaultDefaultPreemptionArgs()
 			}
-			pl, err := NewDefaultPreemption(tt.args, fwk, feature.Features{})
+			pl, err := New(ctx, tt.args, fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1430,7 +1430,7 @@ func TestSelectBestCandidate(t *testing.T) {
 				t.Errorf("Unexpected PreFilter Status: %v", status)
 			}
 
-			pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			pl, err := New(ctx, getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1693,7 +1693,7 @@ func TestCustomSelection(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			pl, err := New(ctx, getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1809,7 +1809,7 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), f, feature.Features{})
+			pl, err := New(ctx, getDefaultDefaultPreemptionArgs(), f, feature.Features{})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -2115,7 +2115,7 @@ func TestPreempt(t *testing.T) {
 				features := feature.Features{
 					EnableAsyncPreemption: asyncPreemptionEnabled,
 				}
-				pl, err := NewDefaultPreemption(getDefaultDefaultPreemptionArgs(), fwk, features)
+				pl, err := New(ctx, getDefaultDefaultPreemptionArgs(), fwk, features)
 				if err != nil {
 					t.Fatal(err)
 				}
diff --git a/pkg/scheduler/framework/runtime/registry.go b/pkg/scheduler/framework/runtime/registry.go
index b6fca3c29cd..8c5901ae52a 100644
--- a/pkg/scheduler/framework/runtime/registry.go
+++ b/pkg/scheduler/framework/runtime/registry.go
@@ -31,11 +31,11 @@ import (
 type PluginFactory = func(ctx context.Context, configuration runtime.Object, f framework.Handle) (framework.Plugin, error)
 
 // PluginFactoryWithFts is a function that builds a plugin with certain feature gates.
-type PluginFactoryWithFts func(context.Context, runtime.Object, framework.Handle, plfeature.Features) (framework.Plugin, error)
+type PluginFactoryWithFts[T framework.Plugin] func(context.Context, runtime.Object, framework.Handle, plfeature.Features) (T, error)
 
 // FactoryAdapter can be used to inject feature gates for a plugin that needs
 // them when the caller expects the older PluginFactory method.
-func FactoryAdapter(fts plfeature.Features, withFts PluginFactoryWithFts) PluginFactory {
+func FactoryAdapter[T framework.Plugin](fts plfeature.Features, withFts PluginFactoryWithFts[T]) PluginFactory {
 	return func(ctx context.Context, plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) {
 		return withFts(ctx, plArgs, fh, fts)
 	}

From 95ebc2c10e9773aa5af08e6454cf45af0d663fa8 Mon Sep 17 00:00:00 2001
From: Nicholas Parker <nick@nickbp.com>
Date: Fri, 28 Feb 2025 17:43:20 +1300
Subject: [PATCH 11/18] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Dominik Marciński <gmidon@gmail.com>
---
 .../framework/plugins/defaultpreemption/default_preemption.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 0a145130416..445f2bd3195 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -242,7 +242,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	sort.Slice(potentialVictims, func(i, j int) bool { return pl.MoreImportantPod(potentialVictims[i].Pod, potentialVictims[j].Pod) })
 	// Try to reprieve as many pods as possible. We first try to reprieve the PDB
 	// violating victims and then other non-violating ones. In both cases, we start
-	// from the highest priority victims.
+	// from the highest importance victims.
 	violatingVictims, nonViolatingVictims := filterPodsWithPDBViolation(potentialVictims, pdbs)
 	reprievePod := func(pi *framework.PodInfo) (bool, error) {
 		if err := addPod(pi); err != nil {
@@ -273,7 +273,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 		}
 	}
 
-	// Sort victims after reprieving pods to keep the pods in the victims sorted in order of priority from high to low.
+	// Sort victims after reprieving pods to keep the pods in the victims sorted in order of importance from high to low.
 	if len(violatingVictims) != 0 && len(nonViolatingVictims) != 0 {
 		sort.Slice(victims, func(i, j int) bool { return pl.MoreImportantPod(victims[i].Pod, victims[j].Pod) })
 	}

From d4bc527a7b849e43d14f70edecb55bd163d144d0 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 28 Feb 2025 18:32:51 +1300
Subject: [PATCH 12/18] Update comments: affinity info, default behavior,
 priority->importance

---
 .../plugins/defaultpreemption/default_preemption.go  | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 445f2bd3195..b7bcca69c7d 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -52,7 +52,13 @@ const Name = names.DefaultPreemption
 type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
 
 // MoreImportantPodFunc is a function which may be assigned to the DefaultPreemption plugin.
-// The implementations should return true if the first pod is more important than the second pod and the second one should be considered for preemption before the first one.
+// Implementations should return true if the first pod is more important than the second pod
+// and the second one should be considered for preemption before the first one.
+// For performance reasons, the search for nodes eligible for preemption is done by omitting all
+// eligible victims from a node then checking whether the preemptor fits on the node without them.
+// The victims are then readded after the preemptor in order of highest to lowest importance.
+// The default behavior is to ignore the possibility of affinity between the preemptor and any of
+// the victims, as affinity between pods that are eligible to preempt each other isn't recommended.
 type MoreImportantPodFunc func(pod1, pod2 *v1.Pod) bool
 
 // DefaultPreemption is a PostFilter plugin implements the preemption logic.
@@ -71,6 +77,8 @@ type DefaultPreemption struct {
 
 	// MoreImportantPod is used to sort eligible victims in-place in descending order of highest to
 	// lowest importance. Pods with higher importance are less likely to be preempted.
+	// The default behavior is to order pods by descending priority, then descending runtime duration
+	// for pods with equal priority.
 	MoreImportantPod MoreImportantPodFunc
 }
 
@@ -231,7 +239,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	// condition that we could check is if the "pod" is failing to schedule due to
 	// inter-pod affinity to one or more victims, but we have decided not to
 	// support this case for performance reasons. Having affinity to lower
-	// priority pods is not a recommended configuration anyway.
+	// importance (priority) pods is not a recommended configuration anyway.
 	if status := pl.fh.RunFilterPluginsWithNominatedPods(ctx, state, pod, nodeInfo); !status.IsSuccess() {
 		return nil, 0, status
 	}

From 3ed73e058aee31a4ca1e9e18f1c1d3a16c5df168 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 28 Feb 2025 18:42:43 +1300
Subject: [PATCH 13/18] Clean up the affinity explanation, fix for new lint
 rule

---
 .../plugins/defaultpreemption/default_preemption.go       | 8 ++++----
 .../plugins/defaultpreemption/default_preemption_test.go  | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index b7bcca69c7d..e64e29cff0c 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -55,10 +55,10 @@ type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodI
 // Implementations should return true if the first pod is more important than the second pod
 // and the second one should be considered for preemption before the first one.
 // For performance reasons, the search for nodes eligible for preemption is done by omitting all
-// eligible victims from a node then checking whether the preemptor fits on the node without them.
-// The victims are then readded after the preemptor in order of highest to lowest importance.
-// The default behavior is to ignore the possibility of affinity between the preemptor and any of
-// the victims, as affinity between pods that are eligible to preempt each other isn't recommended.
+// eligible victims from a node then checking whether the preemptor fits on the node without them,
+// before adding back victims that still fit with the preemptor.
+// The default behavior is to not consider pod affinity between the preemptor and the victims,
+// as affinity between pods that are eligible to preempt each other isn't recommended.
 type MoreImportantPodFunc func(pod1, pod2 *v1.Pod) bool
 
 // DefaultPreemption is a PostFilter plugin implements the preemption logic.
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index 9be143b479c..f5e682dff37 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -1649,7 +1649,6 @@ func TestCustomSelection(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			rand.Seed(4)
 			nodes := make([]*v1.Node, len(tt.nodeNames))
 			for i, nodeName := range tt.nodeNames {
 				nodes[i] = st.MakeNode().Name(nodeName).Capacity(veryLargeRes).Obj()

From 283c5e6b61f0fe8b6d2e69cd9a7fe752fb8c6dc6 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 7 Mar 2025 17:02:30 +1300
Subject: [PATCH 14/18] Have IsEligiblePod be supplemental to priority check,
 update tests

---
 .../defaultpreemption/default_preemption.go   |  23 ++--
 .../default_preemption_test.go                | 103 +++++++++---------
 2 files changed, 68 insertions(+), 58 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index e64e29cff0c..291a18f9d4d 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -47,8 +47,8 @@ import (
 const Name = names.DefaultPreemption
 
 // IsEligiblePodFunc is a function which may be assigned to the DefaultPreemption plugin.
-// Implementations should return whether a given victim pod on the provided nodeInfo is eligible
-// to be preempted by the provided preemptor.
+// This may implement rules/filtering around preemption eligibility, which is in addition to
+// the internal requirement that the victim pod have lower priority than the preemptor pod.
 type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
 
 // MoreImportantPodFunc is a function which may be assigned to the DefaultPreemption plugin.
@@ -71,8 +71,8 @@ type DefaultPreemption struct {
 	Evaluator *preemption.Evaluator
 
 	// IsEligiblePod returns whether a victim pod is allowed to be preempted by a preemptor pod.
-	// The default behavior is to allow any pods of lower priority to be preempted by any pods
-	// of higher priority.
+	// This filtering is in addition to the internal requirement that the victim pod have lower
+	// priority than the preemptor pod.
 	IsEligiblePod IsEligiblePodFunc
 
 	// MoreImportantPod is used to sort eligible victims in-place in descending order of highest to
@@ -112,9 +112,10 @@ func New(_ context.Context, dpArgs runtime.Object, fh framework.Handle, fts feat
 	}
 	pl.Evaluator = preemption.NewEvaluator(Name, fh, &pl, fts.EnableAsyncPreemption)
 
-	// Default behavior: Any pods of lower priority may be preempted by any pods of higher priority.
+	// Default behavior: No additional filtering, beyond the internal requirement that the victim pod
+	// have lower priority than the preemptor pod.
 	pl.IsEligiblePod = func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
-		return corev1helpers.PodPriority(victim.Pod) < corev1helpers.PodPriority(preemptor)
+		return true
 	}
 
 	// Default behavior: Sort by descending priority, then by descending runtime duration as secondary ordering.
@@ -221,7 +222,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
 	// As the first step, remove all pods eligible for preemption from the node and
 	// check if the given pod can be scheduled without them present.
 	for _, pi := range nodeInfo.Pods {
-		if pl.IsEligiblePod(nodeInfo, pi, pod) {
+		if pl.isPreemptionAllowed(nodeInfo, pi, pod) {
 			potentialVictims = append(potentialVictims, pi)
 			if err := removePod(pi); err != nil {
 				return nil, 0, framework.AsStatus(err)
@@ -316,7 +317,7 @@ func (pl *DefaultPreemption) PodEligibleToPreemptOthers(_ context.Context, pod *
 
 		if nodeInfo, _ := nodeInfos.Get(nomNodeName); nodeInfo != nil {
 			for _, p := range nodeInfo.Pods {
-				if pl.IsEligiblePod(nodeInfo, p, pod) && podTerminatingByPreemption(p.Pod) {
+				if pl.isPreemptionAllowed(nodeInfo, p, pod) && podTerminatingByPreemption(p.Pod) {
 					// There is a terminating pod on the nominated node.
 					return false, "not eligible due to a terminating pod on the nominated node."
 				}
@@ -331,6 +332,12 @@ func (pl *DefaultPreemption) OrderedScoreFuncs(ctx context.Context, nodesToVicti
 	return nil
 }
 
+// isPreemptionAllowed returns whether the victim residing on nodeInfo can be preempted by the preemptor
+func (pl *DefaultPreemption) isPreemptionAllowed(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool {
+	// The victim must have lower priority than the preemptor, in addition to any filtering implemented by IsEligiblePod
+	return corev1helpers.PodPriority(victim.Pod) < corev1helpers.PodPriority(preemptor) && pl.IsEligiblePod(nodeInfo, victim, preemptor)
+}
+
 // podTerminatingByPreemption returns true if the pod is in the termination state caused by scheduler preemption.
 func podTerminatingByPreemption(p *v1.Pod) bool {
 	if p.DeletionTimestamp == nil {
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index f5e682dff37..5d0817a0d77 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -1502,56 +1502,85 @@ func TestCustomSelection(t *testing.T) {
 		expected     map[string][]string
 	}{
 		{
-			name:         "only pods with specified label are preemptable: high priority",
+			name:         "filter for matching pod label: high priority",
 			eligiblePods: podLabelIsEligible("preemptible", "yes"),
-			nodeNames:    []string{"node1", "node2", "node3"},
+			nodeNames:    []string{"node1", "node2", "node3", "node4"},
 			pod:          st.MakePod().Name("p1").UID("p1").Priority(highPriority).Req(largeRes).Obj(),
 			pods: []*v1.Pod{
 				st.MakePod().Name("v1").UID("v1").Label("preemptible", "no").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Label("preemptible", "yes").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v4").UID("v4").Node("node4").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// must have "preemptible"="yes" label, others are not preemptible despite high priority
-			expected: map[string][]string{"node2": {"v2"}},
+			expected: map[string][]string{"node2": {"v2"}, "node3": {"v3"}},
 		},
 		{
-			name:         "only pods with specified label are preemptable: mid priority",
+			name:         "filter for matching pod label: mid priority",
 			eligiblePods: podLabelIsEligible("preemptible", "yes"),
-			nodeNames:    []string{"node1", "node2", "node3"},
+			nodeNames:    []string{"node1", "node2", "node3", "node4"},
 			pod:          st.MakePod().Name("p2").UID("p2").Priority(midPriority).Req(largeRes).Obj(),
 			pods: []*v1.Pod{
 				st.MakePod().Name("v1").UID("v1").Label("preemptible", "no").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Label("preemptible", "yes").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v4").UID("v4").Node("node4").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// midPriority can preempt equal priority since we only look for matching pod label
-			expected: map[string][]string{"node2": {"v2"}},
+			expected: map[string][]string{"node3": {"v3"}},
 		},
 		{
-			name:         "only pods with specified label are preemptable: low priority",
+			name:         "filter for matching pod label: low priority",
 			eligiblePods: podLabelIsEligible("preemptible", "yes"),
-			nodeNames:    []string{"node1", "node2", "node3"},
+			nodeNames:    []string{"node1", "node2", "node3", "node4"},
 			pod:          st.MakePod().Name("p3").UID("p3").Priority(lowPriority).Req(largeRes).Obj(),
 			pods: []*v1.Pod{
 				st.MakePod().Name("v1").UID("v1").Label("preemptible", "no").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v2").UID("v2").Label("preemptible", "yes").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Label("preemptible", "yes").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v4").UID("v4").Node("node4").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// lowPriority can preempt higher priority since we only look for matching pod label
-			expected: map[string][]string{"node2": {"v2"}},
+			expected: map[string][]string{},
 		},
 		{
-			name:         "only pods on specified node name are preemptable",
+			name:         "filter for matching victim node: high priority",
+			eligiblePods: nodeNameIsEligible("node1"),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p3").UID("p3").Priority(highPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node1").Priority(midPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node1").Priority(lowPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v4").UID("v4").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v5").UID("v5").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			expected: map[string][]string{"node1": {"v2", "v3"}},
+		},
+		{
+			name:         "filter for matching victim node: mid priority",
+			eligiblePods: nodeNameIsEligible("node1"),
+			nodeNames:    []string{"node1", "node2", "node3"},
+			pod:          st.MakePod().Name("p3").UID("p3").Priority(midPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node1").Priority(midPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node1").Priority(lowPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v4").UID("v4").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v5").UID("v5").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+			},
+			expected: map[string][]string{"node1": {"v3"}},
+		},
+		{
+			name:         "filter for matching victim node: low priority",
 			eligiblePods: nodeNameIsEligible("node1"),
 			nodeNames:    []string{"node1", "node2", "node3"},
 			pod:          st.MakePod().Name("p3").UID("p3").Priority(lowPriority).Req(largeRes).Obj(),
 			pods: []*v1.Pod{
-				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node1").Priority(midPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node1").Priority(lowPriority).Req(smallRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v4").UID("v4").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v5").UID("v5").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// lowPriority can preempt higher priority since we only look for matching node name
-			expected: map[string][]string{"node1": {"v1"}},
+			expected: map[string][]string{},
 		},
 		{
 			name:         "only pods at or above specified priority can preempted: high priority",
@@ -1563,8 +1592,8 @@ func TestCustomSelection(t *testing.T) {
 				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// highPriority can preempt anything, including other highPriority
-			expected: map[string][]string{"node1": {"v1"}, "node2": {"v2"}, "node3": {"v3"}},
+			// highPriority can preempt anything, but not other highPriority
+			expected: map[string][]string{"node2": {"v2"}, "node3": {"v3"}},
 		},
 		{
 			name:         "only pods at or above specified priority can preempted: mid priority",
@@ -1579,19 +1608,6 @@ func TestCustomSelection(t *testing.T) {
 			// midPriority can't preempt anything
 			expected: map[string][]string{},
 		},
-		{
-			name:         "only pods at or above specified priority can preempted: low priority",
-			eligiblePods: priorityBelowThresholdCannotPreempt(highPriority),
-			nodeNames:    []string{"node1", "node2", "node3"},
-			pod:          st.MakePod().Name("p3").UID("p3").Priority(lowPriority).Req(largeRes).Obj(),
-			pods: []*v1.Pod{
-				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
-			},
-			// lowPriority can't preempt anything
-			expected: map[string][]string{},
-		},
 		{
 			name:         "only pods at or below specified priority can be preempted: high priority",
 			eligiblePods: priorityAboveThresholdCannotBePreempted(midPriority),
@@ -1602,20 +1618,7 @@ func TestCustomSelection(t *testing.T) {
 				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
 				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
-			// the lowPriority pod can always be preempted regardless of preemptor priority
-			expected: map[string][]string{"node3": {"v3"}},
-		},
-		{
-			name:         "only pods at or below specified priority can be preempted: low priority",
-			eligiblePods: priorityAboveThresholdCannotBePreempted(midPriority),
-			nodeNames:    []string{"node1", "node2", "node3"},
-			pod:          st.MakePod().Name("p2").UID("p2").Priority(lowPriority).Req(largeRes).Obj(),
-			pods: []*v1.Pod{
-				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(highPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v2").UID("v2").Node("node2").Priority(midPriority).Req(largeRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
-			},
-			// the lowPriority pod can always be preempted regardless of preemptor priority
+			// the lowPriority pod can be preempted but not the midPriority pod
 			expected: map[string][]string{"node3": {"v3"}},
 		},
 		{

From a507e64fe4b4bc0bf94509f02f1d97f45ad8ba25 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 21 Mar 2025 08:04:15 +1300
Subject: [PATCH 15/18] Have separate tests for custom selection vs ordering,
 add comments around system pod eligibility

---
 .../defaultpreemption/default_preemption.go   |   5 +-
 .../default_preemption_test.go                | 207 ++++++++++++------
 2 files changed, 150 insertions(+), 62 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 291a18f9d4d..2b121ad5fab 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -49,6 +49,8 @@ const Name = names.DefaultPreemption
 // IsEligiblePodFunc is a function which may be assigned to the DefaultPreemption plugin.
 // This may implement rules/filtering around preemption eligibility, which is in addition to
 // the internal requirement that the victim pod have lower priority than the preemptor pod.
+// Any customizations should always allow system services to preempt normal pods, to avoid
+// problems if system pods are unable to find space.
 type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodInfo, preemptor *v1.Pod) bool
 
 // MoreImportantPodFunc is a function which may be assigned to the DefaultPreemption plugin.
@@ -72,7 +74,8 @@ type DefaultPreemption struct {
 
 	// IsEligiblePod returns whether a victim pod is allowed to be preempted by a preemptor pod.
 	// This filtering is in addition to the internal requirement that the victim pod have lower
-	// priority than the preemptor pod.
+	// priority than the preemptor pod. Any customizations should always allow system services
+	// to preempt normal pods, to avoid problems if system pods are unable to find space.
 	IsEligiblePod IsEligiblePodFunc
 
 	// MoreImportantPod is used to sort eligible victims in-place in descending order of highest to
diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index 5d0817a0d77..55511b646e2 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -55,6 +55,7 @@ import (
 	internalqueue "k8s.io/kubernetes/pkg/scheduler/backend/queue"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 	"k8s.io/kubernetes/pkg/scheduler/framework/parallelize"
+	"k8s.io/kubernetes/pkg/scheduler/framework/preemption"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity"
@@ -1437,12 +1438,7 @@ func TestSelectBestCandidate(t *testing.T) {
 			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
 			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)
 			s := pl.Evaluator.SelectCandidate(ctx, candidates)
-			if len(tt.expected) == 0 {
-				if s != nil {
-					t.Errorf("expected no candidates, but got %v", s.Name())
-				}
-				return
-			} else if s == nil || len(s.Name()) == 0 {
+			if s == nil || len(s.Name()) == 0 {
 				t.Fatalf("expected any node in %v, but candidate is missing", tt.expected)
 			}
 			found := false
@@ -1485,17 +1481,9 @@ func TestCustomSelection(t *testing.T) {
 		}
 	}
 
-	orderByOldestStart := func(pod1, pod2 *v1.Pod) bool {
-		return util.GetPodStartTime(pod1).Before(util.GetPodStartTime(pod2))
-	}
-	orderByPodName := func(pod1, pod2 *v1.Pod) bool {
-		return pod1.Name < pod2.Name
-	}
-
 	tests := []struct {
 		name         string
 		eligiblePods IsEligiblePodFunc
-		orderPods    MoreImportantPodFunc
 		nodeNames    []string
 		pod          *v1.Pod
 		pods         []*v1.Pod
@@ -1619,35 +1607,7 @@ func TestCustomSelection(t *testing.T) {
 				st.MakePod().Name("v3").UID("v3").Node("node3").Priority(lowPriority).Req(largeRes).StartTime(epochTime).Obj(),
 			},
 			// the lowPriority pod can be preempted but not the midPriority pod
-			expected: map[string][]string{"node3": {"v3"}},
-		},
-		{
-			name:      "select newest pods",
-			orderPods: orderByOldestStart,
-			nodeNames: []string{"node1"},
-			pod:       st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
-			// size victims to require at least two to be preempted
-			pods: []*v1.Pod{
-				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime2).Obj(),
-				st.MakePod().Name("v2").UID("v2").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("v3").UID("v3").Node("node1").Priority(midPriority).Req(mediumRes).StartTime(epochTime1).Obj(),
-			},
-			// the newest two pods are selected, despite one with higher priority
-			expected: map[string][]string{"node1": {"v3", "v1"}},
-		},
-		{
-			name:      "select alphabetically-last pods",
-			orderPods: orderByPodName,
-			nodeNames: []string{"node1"},
-			pod:       st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
-			// size victims to require at least two to be preempted
-			pods: []*v1.Pod{
-				st.MakePod().Name("foo").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("bar").UID("v2").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
-				st.MakePod().Name("baz").UID("v3").Node("node1").Priority(midPriority).Req(mediumRes).StartTime(epochTime).Obj(),
-			},
-			// the last pods in alphabetic order are selected, despite one with higher priority
-			expected: map[string][]string{"node1": {"baz", "foo"}},
+			expected: map[string][]string{"node2": {"v2"}, "node3": {"v3"}},
 		},
 	}
 	for _, tt := range tests {
@@ -1699,34 +1659,159 @@ func TestCustomSelection(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			// Override selection logic
+			// Override eligibility logic
 			if tt.eligiblePods != nil {
 				pl.IsEligiblePod = tt.eligiblePods
 			}
+			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
+			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)
+			// check that the candidates match what's expected
+			if len(tt.expected) != len(candidates) {
+				candidateNames := []string{}
+				for _, c := range candidates {
+					candidateNames = append(candidateNames, c.Name())
+				}
+				t.Fatalf("expected %d candidates (%+v) but got %d: %+v", len(tt.expected), tt.expected, len(candidates), candidateNames)
+			}
+			for len(candidates) > 0 {
+				selected := pl.Evaluator.SelectCandidate(ctx, candidates)
+
+				expectVictims, ok := tt.expected[selected.Name()]
+				if !ok {
+					t.Fatalf("got unexpected candidate %+v, when expected is %+v", selected, tt.expected)
+				}
+
+				gotVictims := []string{}
+				for _, p := range selected.Victims().Pods {
+					gotVictims = append(gotVictims, p.Name)
+				}
+				if diff := cmp.Diff(expectVictims, gotVictims); diff != "" {
+					t.Errorf("expect victims %+v on node %s, but got pods %+v", expectVictims, selected.Name(), gotVictims)
+				}
+
+				// remove selected from candidates
+				notSelected := []preemption.Candidate{}
+				for _, c := range candidates {
+					if c.Name() != selected.Name() {
+						notSelected = append(notSelected, c)
+					}
+				}
+				candidates = notSelected
+			}
+		})
+	}
+}
+
+func TestCustomOrdering(t *testing.T) {
+	// Two arbitrary examples of custom selection ordering to check that they behave as expected
+	orderByOldestStart := func(pod1, pod2 *v1.Pod) bool {
+		return util.GetPodStartTime(pod1).Before(util.GetPodStartTime(pod2))
+	}
+	orderByPodName := func(pod1, pod2 *v1.Pod) bool {
+		return pod1.Name < pod2.Name
+	}
+
+	tests := []struct {
+		name         string
+		orderPods    MoreImportantPodFunc
+		nodeNames    []string
+		pod          *v1.Pod
+		pods         []*v1.Pod
+		expectedPods []string
+	}{
+		{
+			name:      "select newest pods",
+			orderPods: orderByOldestStart,
+			nodeNames: []string{"node1"},
+			pod:       st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
+			// size victims to require at least two to be preempted
+			pods: []*v1.Pod{
+				st.MakePod().Name("v1").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime2).Obj(),
+				st.MakePod().Name("v2").UID("v2").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("v3").UID("v3").Node("node1").Priority(midPriority).Req(mediumRes).StartTime(epochTime1).Obj(),
+			},
+			// the newest two pods are selected, despite one with higher priority
+			expectedPods: []string{"v3", "v1"},
+		},
+		{
+			name:      "select alphabetically-last pods",
+			orderPods: orderByPodName,
+			nodeNames: []string{"node1"},
+			pod:       st.MakePod().Name("p2").UID("p2").Priority(highPriority).Req(largeRes).Obj(),
+			// size victims to require at least two to be preempted
+			pods: []*v1.Pod{
+				st.MakePod().Name("foo").UID("v1").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("bar").UID("v2").Node("node1").Priority(lowPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+				st.MakePod().Name("baz").UID("v3").Node("node1").Priority(midPriority).Req(mediumRes).StartTime(epochTime).Obj(),
+			},
+			// the last pods in alphabetic order are selected, despite one with higher priority
+			expectedPods: []string{"baz", "foo"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			nodes := make([]*v1.Node, len(tt.nodeNames))
+			for i, nodeName := range tt.nodeNames {
+				nodes[i] = st.MakeNode().Name(nodeName).Capacity(veryLargeRes).Obj()
+			}
+
+			var objs []runtime.Object
+			objs = append(objs, tt.pod)
+			for _, pod := range tt.pods {
+				objs = append(objs, pod)
+			}
+			cs := clientsetfake.NewClientset(objs...)
+			informerFactory := informers.NewSharedInformerFactory(cs, 0)
+			snapshot := internalcache.NewSnapshot(tt.pods, nodes)
+			logger, ctx := ktesting.NewTestContext(t)
+			ctx, cancel := context.WithCancel(ctx)
+			defer cancel()
+			fwk, err := tf.NewFramework(
+				ctx,
+				[]tf.RegisterPluginFunc{
+					tf.RegisterPluginAsExtensions(noderesources.Name, nodeResourcesFitFunc, "Filter", "PreFilter"),
+					tf.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
+					tf.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
+				},
+				"",
+				frameworkruntime.WithPodNominator(internalqueue.NewSchedulingQueue(nil, informerFactory)),
+				frameworkruntime.WithSnapshotSharedLister(snapshot),
+				frameworkruntime.WithInformerFactory(informerFactory),
+				frameworkruntime.WithLogger(logger),
+			)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			state := framework.NewCycleState()
+			// Some tests rely on PreFilter plugin to compute its CycleState.
+			if _, status, _ := fwk.RunPreFilterPlugins(ctx, state, tt.pod); !status.IsSuccess() {
+				t.Errorf("Unexpected PreFilter Status: %v", status)
+			}
+			nodeInfos, err := snapshot.NodeInfos().List()
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			pl, err := New(ctx, getDefaultDefaultPreemptionArgs(), fwk, feature.Features{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			// Override ordering logic
 			if tt.orderPods != nil {
 				pl.MoreImportantPod = tt.orderPods
 			}
 			offset, numCandidates := pl.GetOffsetAndNumCandidates(int32(len(nodeInfos)))
 			candidates, _, _ := pl.Evaluator.DryRunPreemption(ctx, state, tt.pod, nodeInfos, nil, offset, numCandidates)
-			s := pl.Evaluator.SelectCandidate(ctx, candidates)
-			if len(tt.expected) == 0 {
-				if s != nil {
-					t.Errorf("expected no candidates, but got %v", s.Name())
-				}
-				return
-			} else if s == nil || len(s.Name()) == 0 {
-				t.Fatalf("expected any node in %v, but candidate is missing", tt.expected)
+			if len(candidates) != 1 {
+				t.Fatalf("expected exactly one node but got %+v", candidates)
 			}
-			expectPodNames, ok := tt.expected[s.Name()]
-			if !ok {
-				t.Errorf("expect any node in %v, but got %v", tt.expected, s.Name())
+			podNames := []string{}
+			for _, p := range candidates[0].Victims().Pods {
+				podNames = append(podNames, p.Name)
 			}
-			gotPodNames := []string{}
-			for _, p := range s.Victims().Pods {
-				gotPodNames = append(gotPodNames, p.Name)
-			}
-			if diff := cmp.Diff(expectPodNames, gotPodNames); diff != "" {
-				t.Errorf("expected pods %v, but got %v", expectPodNames, gotPodNames)
+			if diff := cmp.Diff(tt.expectedPods, podNames); diff != "" {
+				t.Errorf("expect pods %+v, but got pods %+v", tt.expectedPods, podNames)
 			}
 		})
 	}

From 52e7aa37daa8e7116b5dc7d3e68ddcec7b70fde6 Mon Sep 17 00:00:00 2001
From: Nick Parker <nickparker@netflix.com>
Date: Fri, 21 Mar 2025 12:04:27 +1300
Subject: [PATCH 16/18] gofmt

---
 .../plugins/defaultpreemption/default_preemption_test.go        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index 55511b646e2..fc6713e3768 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -55,7 +55,6 @@ import (
 	internalqueue "k8s.io/kubernetes/pkg/scheduler/backend/queue"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 	"k8s.io/kubernetes/pkg/scheduler/framework/parallelize"
-	"k8s.io/kubernetes/pkg/scheduler/framework/preemption"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity"
@@ -63,6 +62,7 @@ import (
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/podtopologyspread"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/queuesort"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration"
+	"k8s.io/kubernetes/pkg/scheduler/framework/preemption"
 	frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime"
 	"k8s.io/kubernetes/pkg/scheduler/metrics"
 	st "k8s.io/kubernetes/pkg/scheduler/testing"

From 28602c66fcf9720c8500507c641293f950994b83 Mon Sep 17 00:00:00 2001
From: Nicholas Parker <nick@nickbp.com>
Date: Thu, 10 Apr 2025 07:35:06 +1200
Subject: [PATCH 17/18] Update
 pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
---
 .../plugins/defaultpreemption/default_preemption_test.go        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
index fc6713e3768..5cc3ce976f0 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go
@@ -1686,7 +1686,7 @@ func TestCustomSelection(t *testing.T) {
 					gotVictims = append(gotVictims, p.Name)
 				}
 				if diff := cmp.Diff(expectVictims, gotVictims); diff != "" {
-					t.Errorf("expect victims %+v on node %s, but got pods %+v", expectVictims, selected.Name(), gotVictims)
+					t.Errorf("Unexpected victims on node %s (-want,+got):\n%s", selected.Name(), diff)
 				}
 
 				// remove selected from candidates

From 7bccb1acb5eb045ff5f4fcbb466924492970765f Mon Sep 17 00:00:00 2001
From: Nicholas Parker <nick@nickbp.com>
Date: Fri, 16 May 2025 13:12:13 +1200
Subject: [PATCH 18/18] Update
 pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Dominik Marciński <gmidon@gmail.com>
---
 .../framework/plugins/defaultpreemption/default_preemption.go   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
index 2b121ad5fab..9484fe952c7 100644
--- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
+++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go
@@ -58,7 +58,7 @@ type IsEligiblePodFunc func(nodeInfo *framework.NodeInfo, victim *framework.PodI
 // and the second one should be considered for preemption before the first one.
 // For performance reasons, the search for nodes eligible for preemption is done by omitting all
 // eligible victims from a node then checking whether the preemptor fits on the node without them,
-// before adding back victims that still fit with the preemptor.
+// before adding back victims (starting from the most important) that still fit with the preemptor.
 // The default behavior is to not consider pod affinity between the preemptor and the victims,
 // as affinity between pods that are eligible to preempt each other isn't recommended.
 type MoreImportantPodFunc func(pod1, pod2 *v1.Pod) bool