Bind PreemptionNoCandidates exclusion to effective AFS ordering

The exclusion only checked AdmissionScope.AdmissionMode, but what actually
reorders the queue is enableAdmissionFs, which needs admissionFairSharing
configured and the feature gate on.

Switched to testing enableAdmissionFs directly since that's the same value
the heap comparator uses, so the exclusion can't drift out of sync with
actual pop order. Also dropped the now-unused AdmissionScope assignment in
Update().

TestRequeueHashTriggerByReason now passes a real config and enables the gate;
its UBAFS cases previously passed only because of this gap.

This in turn replaces the earlier assumption that AdmissionMode alone
was enough to gate this.

See Kueue#14231

Author: Viswalahiri

pkg/cache/queue/cluster_queue.go

@@ -193,6 +193,13 @@ type ClusterQueue struct {
 
 	AdmissionScope *kueue.AdmissionScope
 
+	// enableAdmissionFs reports whether this ClusterQueue actually pops pending
+	// workloads by LocalQueue usage rather than by the queue-order timestamp.
+	// Set once at construction from afs.ResourceWeights, the same value the heap
+	// comparator captured, so the two can never disagree. Immutable afterwards,
+	// so it is read without holding rwm.
+	enableAdmissionFs bool
+
 	afsUsageLedger *queueafs.AfsUsageLedger
 
 	// lqWeights holds the LocalQueues that belong to this ClusterQueue, mapped to
@@ -315,6 +322,7 @@ func newClusterQueueImpl(ctx context.Context, client client.Client, cl *metrics.
 		snapshotSort:           snapshotSort,
 		rwm:                    sync.RWMutex{},
 		clock:                  clock,
+		enableAdmissionFs:      options.enableAdmissionFs,
 		afsUsageLedger:         options.afsUsageLedger,
 		lqWeights:              lqWeights,
 		pw:                     &pw,
@@ -333,7 +341,6 @@ func (c *ClusterQueue) Update(apiCQ *kueue.ClusterQueue) error {
 	}
 	c.namespaceSelector = nsSelector
 	c.active = apimeta.IsStatusConditionTrue(apiCQ.Status.Conditions, kueue.ClusterQueueActive)
-	c.AdmissionScope = apiCQ.Spec.AdmissionScope
 	if features.Enabled(features.ConcurrentAdmission) {
 		c.ConcurrentAdmissionPolicy = apiCQ.Spec.ConcurrentAdmissionPolicy
 	}
@@ -590,8 +597,14 @@ func (c *ClusterQueue) requeueIfNotPresent(log logr.Logger, wInfo *workload.Info
 	}
 	log.V(2).Info(logMsg, "clusterQueue", c.name, "workload", key)
 
+	// PreemptionNoCandidates is excluded when this queue pops by LocalQueue usage:
+	// the scheduling-equivalence shape does not encode the queue-order timestamp,
+	// which under a LowerOrNewerEqualPriority preemption policy decides whether a
+	// workload may preempt an equal-priority victim. Two workloads can therefore
+	// share a hash but not a preemption verdict, so a class-wide conclusion would
+	// defer workloads that are in fact eligible. See Kueue#14231.
 	if features.Enabled(features.SchedulingEquivalenceHashing) && wInfo.SchedulingHash != workload.SchedulingHashUnknown &&
-		(reason == RequeueReasonNoFit || (reason == RequeueReasonPreemptionNoCandidates && !c.usesUsageBasedAdmissionFairSharing())) {
+		(reason == RequeueReasonNoFit || (reason == RequeueReasonPreemptionNoCandidates && !c.enableAdmissionFs)) {
 		if moved := c.handleInadmissibleHash(wInfo.SchedulingHash, resolveQuotaReservedReason(quotaReservedReason)); moved > 0 {
 			log.V(2).Info("Bulk-moved equivalent workloads to inadmissible", "hash", wInfo.SchedulingHash, "movedCount", moved)
 		}
@@ -600,16 +613,6 @@ func (c *ClusterQueue) requeueIfNotPresent(log logr.Logger, wInfo *workload.Info
 	return true
 }
 
-// usesUsageBasedAdmissionFairSharing reports whether this ClusterQueue pops
-// pending workloads ordered by LocalQueue usage instead of the queue-order
-// timestamp. The scheduling-equivalence shape does not encode that timestamp,
-// so a class-wide PreemptionNoCandidates conclusion reached under this mode
-// can incorrectly defer an equivalent workload that is actually eligible to
-// preempt under a LowerOrNewerEqualPriority preemption policy. See Kueue#14231.
-func (c *ClusterQueue) usesUsageBasedAdmissionFairSharing() bool {
-	return c.AdmissionScope != nil && c.AdmissionScope.AdmissionMode == kueue.UsageBasedAdmissionFairSharing
-}
-
 // handleInadmissibleHash bulk-moves all heap workloads matching the given
 // scheduling hash to inadmissibleWorkloads. Returns the number moved.
 // Only applies to BestEffortFIFO queues; in StrictFIFO the head workload

pkg/cache/queue/cluster_queue_test.go

@@ -2041,9 +2041,14 @@ func TestQueueInadmissibleWorkloadsClearsHashes(t *testing.T) {
 }
 
 func TestRequeueHashTriggerByReason(t *testing.T) {
+	usageBasedAFS := &kueue.AdmissionScope{AdmissionMode: kueue.UsageBasedAdmissionFairSharing}
+
 	tests := map[string]struct {
-		reason         RequeueReason
+		reason RequeueReason
+		// admissionScope and afsConfig must both be set for the ClusterQueue to
+		// actually order by LocalQueue usage; see afs.ResourceWeights.
 		admissionScope *kueue.AdmissionScope
+		afsConfig      *config.AdmissionFairSharing
 		wantHash       bool
 	}{
 		"nofit triggers hash": {
@@ -2067,28 +2072,36 @@ func TestRequeueHashTriggerByReason(t *testing.T) {
 			wantHash: false,
 		},
 		"nofit triggers hash under usage-based admission fair sharing": {
-			reason: RequeueReasonNoFit,
-			admissionScope: &kueue.AdmissionScope{
-				AdmissionMode: kueue.UsageBasedAdmissionFairSharing,
-			},
-			wantHash: true,
+			reason:         RequeueReasonNoFit,
+			admissionScope: usageBasedAFS,
+			afsConfig:      &config.AdmissionFairSharing{},
+			wantHash:       true,
 		},
 		"preempt no candidates does not trigger hash under usage-based admission fair sharing": {
 			// The scheduling shape omits the queue-order timestamp that
 			// UsageBasedAdmissionFairSharing's LocalQueue-usage-first pop order can
 			// reorder relative to LowerOrNewerEqualPriority preemption eligibility, so a
 			// class-wide PreemptionNoCandidates conclusion is unsafe here. See Kueue#14231.
-			reason: RequeueReasonPreemptionNoCandidates,
-			admissionScope: &kueue.AdmissionScope{
-				AdmissionMode: kueue.UsageBasedAdmissionFairSharing,
-			},
-			wantHash: false,
+			reason:         RequeueReasonPreemptionNoCandidates,
+			admissionScope: usageBasedAFS,
+			afsConfig:      &config.AdmissionFairSharing{},
+			wantHash:       false,
+		},
+		"preempt no candidates triggers hash when admission mode is set but admission fair sharing is not configured": {
+			// Without admissionFairSharing in the Configuration the queue still pops
+			// oldest-first, so the queue-order timestamp gap does not apply and the
+			// optimization must be retained.
+			reason:         RequeueReasonPreemptionNoCandidates,
+			admissionScope: usageBasedAFS,
+			afsConfig:      nil,
+			wantHash:       true,
 		},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			features.SetFeatureGateDuringTest(t, features.SchedulingEquivalenceHashing, true)
+			features.SetFeatureGateDuringTest(t, features.AdmissionFairSharing, true)
 			ctx, _ := utiltesting.ContextWithLog(t)
 			cq, _ := newClusterQueue(ctx, nil,
 				&kueue.ClusterQueue{
@@ -2098,7 +2111,7 @@ func TestRequeueHashTriggerByReason(t *testing.T) {
 					},
 				}, nil,
 				workload.Ordering{PodsReadyRequeuingTimestamp: config.EvictionTimestamp},
-				nil, nil)
+				tc.afsConfig, nil)
 
 			wl := utiltestingapi.MakeWorkload("workload-1", defaultNamespace).
 				Request(corev1.ResourceCPU, "1").Obj()