Repository: kubernetes-sigs/kueue

Issue:
[release-0.12] Flaky Integration Test: Scheduler when Scheduling workloads on clusterQueues Should admit workloads with admission checks
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!

If the matter is security related, please disclose it privately via https://kubernetes.io/security/
-->


**What happened**:
```
Scheduler Suite: [It] Scheduler when Scheduling workloads on clusterQueues Should admit workloads with admission checks 

{Timed out after 10.001s.
The function passed to Eventually failed at /home/prow/go/src/sigs.k8s.io/kueue/test/util/util.go:478 with:
Expected object to be comparable, diff:   (*v1beta1.Admission)(
- 	&{
- 		ClusterQueue: "admission-check-cq",
- 		PodSetAssignments: []v1beta1.PodSetAssignment{
- 			{
- 				Name:          "main",
- 				Flavors:       map[v1.ResourceName]v1beta1.ResourceFlavorReference{...},
- 				ResourceUsage: v1.ResourceList{...},
- 				Count:         &1,
- 			},
- 		},
- 	},
+ 	nil,
  )
 failed [FAILED] Timed out after 10.001s.
The function passed to Eventually failed at /home/prow/go/src/sigs.k8s.io/kueue/test/util/util.go:478 with:
Expected object to be comparable, diff:   (*v1beta1.Admission)(
- 	&{
- 		ClusterQueue: "admission-check-cq",
- 		PodSetAssignments: []v1beta1.PodSetAssignment{
- 			{
- 				Name:          "main",
- 				Flavors:       map[v1.ResourceName]v1beta1.ResourceFlavorReference{...},
- 				ResourceUsage: v1.ResourceList{...},
- 				Count:         &1,
- 			},
- 		},
- 	},
+ 	nil,
  )
In [It] at: /home/prow/go/src/sigs.k8s.io/kueue/test/integration/singlecluster/scheduler/scheduler_test.go:477 @ 06/09/25 14:20:13.105
}
```

**What you expected to happen**:
No errors

**How to reproduce it (as minimally and precisely as possible)**:
https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/5586/pull-kueue-test-integration-baseline-release-0-12/1932077407703928832

**Anything else we need to know?**:

**Environment**:
- Kubernetes version (use `kubectl version`):
- Kueue version (use `git describe --tags --dirty --always`):
- Cloud provider or hardware configuration:
- OS (e.g: `cat /etc/os-release`):
- Kernel (e.g. `uname -a`):
- Install tools:
- Others:

Discussion:
@mbobrovskyi: /kind flake@mbobrovskyi: /assign

Solution patch:
diff --git a/Makefile-test.mk b/Makefile-test.mk
index fa00e2ed0..b27471795 100644
--- a/Makefile-test.mk
+++ b/Makefile-test.mk
@@ -50,8 +50,6 @@ E2E_RUN_ONLY_ENV ?= false
 # Default will delete default kind cluster
 KIND_CLUSTER_NAME ?= kind
 
-CYPRESS_IMAGE_NAME ?= cypress/base:22.14.0
-
 ##@ Tests
 
 # Periodic builds are tested with full ray image
diff --git a/hack/e2e-kueueviz-local.sh b/hack/e2e-kueueviz-local.sh
index 8915dce78..d694c07ea 100755
--- a/hack/e2e-kueueviz-local.sh
+++ b/hack/e2e-kueueviz-local.sh
@@ -21,9 +21,6 @@ ROOT_DIR="$SOURCE_DIR/.."
 # shellcheck source=hack/e2e-common.sh
 source "${SOURCE_DIR}/e2e-common.sh"
 
-# Set CYPRESS_IMAGE_NAME to the value of the env var or default to cypress/base
-CYPRESS_IMAGE_NAME="${CYPRESS_IMAGE_NAME:-cypress/base}"
-
 # Function to clean up background processes
 cleanup() {
   echo "Cleaning up kueueviz processes"
diff --git a/pkg/workload/workload.go b/pkg/workload/workload.go
index c0224350d..4ff46228a 100644
--- a/pkg/workload/workload.go
+++ b/pkg/workload/workload.go
@@ -644,21 +644,21 @@ func SetQuotaReservation(w *kueue.Workload, admission *kueue.Admission, clock cl
 // NeedsSecondPass checks if the second pass of scheduling is needed for the
 // workload.
 func NeedsSecondPass(w *kueue.Workload) bool {
+	if IsFinished(w) || IsEvicted(w) || !HasQuotaReservation(w) {
+		return false
+	}
 	return needsSecondPassForDelayedAssignment(w) || needsSecondPassAfterNodeFailure(w)
 }
 
 func needsSecondPassForDelayedAssignment(w *kueue.Workload) bool {
-	return HasQuotaReservation(w) &&
-		len(w.Status.AdmissionChecks) > 0 &&
+	return len(w.Status.AdmissionChecks) > 0 &&
 		HasAllChecksReady(w) &&
 		HasTopologyAssignmentsPending(w) &&
-		!IsAdmitted(w) &&
-		!IsFinished(w) &&
-		!IsEvicted(w)
+		!IsAdmitted(w)
 }
 
 func needsSecondPassAfterNodeFailure(w *kueue.Workload) bool {
-	return IsAdmitted(w) && HasNodeToReplace(w)
+	return HasTopologyAssignmentWithNodeToReplace(w)
 }
 
 // HasTopologyAssignmentsPending checks if the workload contains any



Test patch:
diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go
index ca0c839cb..8e41e6da6 100644
--- a/pkg/util/testing/wrappers.go
+++ b/pkg/util/testing/wrappers.go
@@ -218,6 +218,18 @@ func (w *WorkloadWrapper) Finished() *WorkloadWrapper {
 	return w
 }
 
+func (w *WorkloadWrapper) Evicted() *WorkloadWrapper {
+	cond := metav1.Condition{
+		Type:               kueue.WorkloadEvicted,
+		Status:             metav1.ConditionTrue,
+		LastTransitionTime: metav1.Now(),
+		Reason:             "ByTest",
+		Message:            "Evicted by test",
+	}
+	apimeta.SetStatusCondition(&w.Status.Conditions, cond)
+	return w
+}
+
 func (w *WorkloadWrapper) Creation(t time.Time) *WorkloadWrapper {
 	w.CreationTimestamp = metav1.NewTime(t)
 	return w
diff --git a/pkg/workload/workload_test.go b/pkg/workload/workload_test.go
index 2bc1b63c6..c28670a1d 100644
--- a/pkg/workload/workload_test.go
+++ b/pkg/workload/workload_test.go
@@ -31,10 +31,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	config "sigs.k8s.io/kueue/apis/config/v1beta1"
+	kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1"
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	"sigs.k8s.io/kueue/pkg/features"
 	"sigs.k8s.io/kueue/pkg/resources"
 	"sigs.k8s.io/kueue/pkg/util/admissioncheck"
+	utiltas "sigs.k8s.io/kueue/pkg/util/tas"
 	utiltesting "sigs.k8s.io/kueue/pkg/util/testing"
 )
 
@@ -1085,3 +1087,216 @@ func TestPropagateResourceRequests(t *testing.T) {
 		})
 	}
 }
+
+func TestNeedsSecondPass(t *testing.T) {
+	defaultSingleLevelTopology := *utiltesting.MakeDefaultOneLevelTopology("tas-single-level")
+	cases := map[string]struct {
+		wl   *kueue.Workload
+		want bool
+	}{
+		"admitted workload with NodeToReplace": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Annotations(map[string]string{kueuealpha.NodeToReplaceAnnotation: "x0"}).
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					PreferredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				ReserveQuota(
+					utiltesting.MakeAdmission("tas-main", "one").
+						Assignment(corev1.ResourceCPU, "tas-default", "1000m").
+						AssignmentPodCount(1).
+						TopologyAssignment(&kueue.TopologyAssignment{
+							Levels: utiltas.Levels(&defaultSingleLevelTopology),
+							Domains: []kueue.TopologyDomainAssignment{
+								{
+									Count: 1,
+									Values: []string{
+										"x0",
+									},
+								},
+							},
+						}).
+						Obj(),
+				).
+				Admitted(true).
+				Obj(),
+			want: true,
+		},
+		"admitted workload without NodeToReplace": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					PreferredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				ReserveQuota(
+					utiltesting.MakeAdmission("tas-main", "one").
+						Assignment(corev1.ResourceCPU, "tas-default", "1000m").
+						AssignmentPodCount(1).
+						TopologyAssignment(&kueue.TopologyAssignment{
+							Levels: utiltas.Levels(&defaultSingleLevelTopology),
+							Domains: []kueue.TopologyDomainAssignment{
+								{
+									Count: 1,
+									Values: []string{
+										"x0",
+									},
+								},
+							},
+						}).
+						Obj(),
+				).
+				Admitted(true).
+				Obj(),
+			want: false,
+		},
+		"admitted workload with NodeToReplace, but no node in the assignment": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Annotations(map[string]string{kueuealpha.NodeToReplaceAnnotation: "x0"}).
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					PreferredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				ReserveQuota(
+					utiltesting.MakeAdmission("tas-main", "one").
+						Assignment(corev1.ResourceCPU, "tas-default", "1000m").
+						AssignmentPodCount(1).
+						TopologyAssignment(&kueue.TopologyAssignment{
+							Levels: utiltas.Levels(&defaultSingleLevelTopology),
+							Domains: []kueue.TopologyDomainAssignment{
+								{
+									Count: 1,
+									Values: []string{
+										"x1",
+									},
+								},
+							},
+						}).
+						Obj(),
+				).
+				Admitted(true).
+				Obj(),
+			want: false,
+		},
+		"finished workload with NodeToReplace": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Annotations(map[string]string{kueuealpha.NodeToReplaceAnnotation: "x0"}).
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					PreferredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				ReserveQuota(
+					utiltesting.MakeAdmission("tas-main", "one").
+						Assignment(corev1.ResourceCPU, "tas-default", "1000m").
+						AssignmentPodCount(1).
+						TopologyAssignment(&kueue.TopologyAssignment{
+							Levels: utiltas.Levels(&defaultSingleLevelTopology),
+							Domains: []kueue.TopologyDomainAssignment{
+								{
+									Count: 1,
+									Values: []string{
+										"x0",
+									},
+								},
+							},
+						}).
+						Obj(),
+				).
+				Admitted(true).
+				Finished().
+				Obj(),
+			want: false,
+		},
+		"evicted workload with NodeToReplace": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Annotations(map[string]string{kueuealpha.NodeToReplaceAnnotation: "x0"}).
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					PreferredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				ReserveQuota(
+					utiltesting.MakeAdmission("tas-main", "one").
+						Assignment(corev1.ResourceCPU, "tas-default", "1000m").
+						AssignmentPodCount(1).
+						TopologyAssignment(&kueue.TopologyAssignment{
+							Levels: utiltas.Levels(&defaultSingleLevelTopology),
+							Domains: []kueue.TopologyDomainAssignment{
+								{
+									Count: 1,
+									Values: []string{
+										"x0",
+									},
+								},
+							},
+						}).
+						Obj(),
+				).
+				Admitted(true).
+				Evicted().
+				Obj(),
+			want: false,
+		},
+		"quotaReserved and admission checks Ready when workload delayedTopologyRequest=Pending": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					RequiredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				ReserveQuota(
+					utiltesting.MakeAdmission("tas-main", "one").
+						Assignment(corev1.ResourceCPU, "tas-default", "1000m").
+						DelayedTopologyRequest(kueue.DelayedTopologyRequestStatePending).
+						AssignmentPodCount(1).Obj(),
+				).
+				AdmissionCheck(kueue.AdmissionCheckState{
+					Name:  "prov-check",
+					State: kueue.CheckStateReady,
+				}).
+				Obj(),
+			want: true,
+		},
+		"quotaReserved and admission checks Pending": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					RequiredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				ReserveQuota(
+					utiltesting.MakeAdmission("tas-main", "one").
+						Assignment(corev1.ResourceCPU, "tas-default", "1000m").
+						DelayedTopologyRequest(kueue.DelayedTopologyRequestStatePending).
+						AssignmentPodCount(1).Obj(),
+				).
+				AdmissionCheck(kueue.AdmissionCheckState{
+					Name:  "prov-check",
+					State: kueue.CheckStatePending,
+				}).
+				Obj(),
+			want: false,
+		},
+		"workload without quota": {
+			wl: utiltesting.MakeWorkload("foo", "default").
+				Queue("tas-main").
+				PodSets(*utiltesting.MakePodSet("one", 1).
+					RequiredTopologyRequest(corev1.LabelHostname).
+					Request(corev1.ResourceCPU, "1").
+					Obj()).
+				Obj(),
+			want: false,
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := NeedsSecondPass(tc.wl)
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("Unexpected NeedsSecondPass() result (-want,+got):\n%s", diff)
+			}
+		})
+	}
+}