Repository: kubernetes-sigs/kueue

Issue:
Commonize the code for workload eviction
<!-- Please only use this template for submitting clean up requests -->

**What would you like to be cleaned**:

Commonize workload eviction by using the common workload.EvictWorkload function (or just workload.Evict).

**Why is this needed**:

Currently many different places handles workload eviction in slightly different ways, we should commonize to make bugs less likely.

Hints from issue comments:
@mimowo: /assign 
tentatively

Solution patch:
diff --git a/hack/e2e-test.sh b/hack/e2e-test.sh
index 83dd8d7f4..b8e0b87ca 100755
--- a/hack/e2e-test.sh
+++ b/hack/e2e-test.sh
@@ -51,10 +51,16 @@ wait
 kind_load "$KIND_CLUSTER_NAME" ""
 kueue_deploy
 
-if [ "$E2E_RUN_ONLY_ENV" == 'true' ]; then
-  read -rp "Press Enter to cleanup."
-else
-  # shellcheck disable=SC2086
-  $GINKGO $GINKGO_ARGS --junit-report=junit.xml --json-report=e2e.json --output-dir="$ARTIFACTS" -v ./test/e2e/$E2E_TARGET_FOLDER/...
-  "$ROOT_DIR/bin/ginkgo-top" -i "$ARTIFACTS/e2e.json" > "$ARTIFACTS/e2e-top.yaml"
+if [ "$E2E_RUN_ONLY_ENV" = "true" ]; then
+  read -rp "Do you want to cleanup? [Y/n] " reply
+  if [[ "$reply" =~ ^[nN]$ ]]; then
+    trap - EXIT
+    echo "Skipping cleanup for kind cluster."
+    echo -e "\nKind cluster cleanup:\n  kind delete cluster --name $KIND_CLUSTER_NAME"
+  fi
+  exit 0
 fi
+
+# shellcheck disable=SC2086
+$GINKGO $GINKGO_ARGS --junit-report=junit.xml --json-report=e2e.json --output-dir="$ARTIFACTS" -v ./test/e2e/$E2E_TARGET_FOLDER/...
+"$ROOT_DIR/bin/ginkgo-top" -i "$ARTIFACTS/e2e.json" > "$ARTIFACTS/e2e-top.yaml"
diff --git a/hack/multikueue-e2e-test.sh b/hack/multikueue-e2e-test.sh
index 9fe4389d7..addd2efb9 100755
--- a/hack/multikueue-e2e-test.sh
+++ b/hack/multikueue-e2e-test.sh
@@ -93,10 +93,18 @@ wait # for libraries installation
 export KUBECONFIG="$MANAGER_KUBECONFIG:$WORKER1_KUBECONFIG:$WORKER2_KUBECONFIG"
 kueue_deploy
 
-if [ "$E2E_RUN_ONLY_ENV" == 'true' ]; then
-  read -rp "Press Enter to cleanup."
-else
-  # shellcheck disable=SC2086
-  $GINKGO $GINKGO_ARGS --junit-report=junit.xml --json-report=e2e.json --output-dir="$ARTIFACTS" -v ./test/e2e/multikueue/...
-  "$ROOT_DIR/bin/ginkgo-top" -i "$ARTIFACTS/e2e.json" > "$ARTIFACTS/e2e-top.yaml"
+if [ "$E2E_RUN_ONLY_ENV" = "true" ]; then
+  read -rp "Do you want to cleanup? [Y/n] " reply
+  if [[ "$reply" =~ ^[nN]$ ]]; then
+    trap - EXIT
+    echo "Skipping cleanup for kind clusters."
+    echo -e "\nManager kind cluster cleanup:\n  kind delete cluster --name $MANAGER_KIND_CLUSTER_NAME"
+    echo -e "\nWorker1 kind cluster cleanup:\n  kind delete cluster --name $WORKER1_KIND_CLUSTER_NAME"
+    echo -e "\nWorker2 kind cluster cleanup:\n  kind delete cluster --name $WORKER2_KIND_CLUSTER_NAME"
+  fi
+  exit 0
 fi
+
+# shellcheck disable=SC2086
+$GINKGO $GINKGO_ARGS --junit-report=junit.xml --json-report=e2e.json --output-dir="$ARTIFACTS" -v ./test/e2e/multikueue/...
+"$ROOT_DIR/bin/ginkgo-top" -i "$ARTIFACTS/e2e.json" > "$ARTIFACTS/e2e-top.yaml"
diff --git a/pkg/controller/core/indexer/indexer.go b/pkg/controller/core/indexer/indexer.go
index 883fad67b..2c15b57d3 100644
--- a/pkg/controller/core/indexer/indexer.go
+++ b/pkg/controller/core/indexer/indexer.go
@@ -28,6 +28,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
+	"sigs.k8s.io/kueue/pkg/features"
 	"sigs.k8s.io/kueue/pkg/util/slices"
 )
 
@@ -188,5 +189,12 @@ func Setup(ctx context.Context, indexer client.FieldIndexer) error {
 	if err := indexer.IndexField(ctx, &kueue.Workload{}, OwnerReferenceUID, IndexOwnerUID); err != nil {
 		return fmt.Errorf("setting index on ownerReferences.uid for Workload: %w", err)
 	}
+	// Add pod index to be able to list pods for elastic-jobs, needed to remove scheduling gate on
+	// admitted workload slices.
+	if features.Enabled(features.ElasticJobsViaWorkloadSlices) {
+		if err := indexer.IndexField(ctx, &corev1.Pod{}, OwnerReferenceUID, IndexOwnerUID); err != nil {
+			return err
+		}
+	}
 	return nil
 }
diff --git a/pkg/controller/core/workload_controller.go b/pkg/controller/core/workload_controller.go
index 77c057d18..b8bc0afcc 100644
--- a/pkg/controller/core/workload_controller.go
+++ b/pkg/controller/core/workload_controller.go
@@ -253,7 +253,7 @@ func (r *WorkloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		}
 		if updated {
 			if evicted {
-				if err := workload.EvictWorkload(ctx, r.client, r.recorder, &wl, reason, "", message, r.clock); err != nil {
+				if err := workload.Evict(ctx, r.client, r.recorder, &wl, reason, "", message, r.clock); err != nil {
 					if !apierrors.IsNotFound(err) {
 						return ctrl.Result{}, fmt.Errorf("setting eviction: %w", err)
 					}
@@ -443,7 +443,7 @@ func (r *WorkloadReconciler) reconcileCheckBasedEviction(ctx context.Context, wl
 	}
 	// at this point we know a Workload has at least one Retry AdmissionCheck
 	message := "At least one admission check is false"
-	if err := workload.EvictWorkload(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByAdmissionCheck, "", message, r.clock); err != nil {
+	if err := workload.Evict(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByAdmissionCheck, "", message, r.clock); err != nil {
 		return false, err
 	}
 	return true, nil
@@ -476,7 +476,7 @@ func (r *WorkloadReconciler) reconcileOnLocalQueueActiveState(ctx context.Contex
 			return false, nil
 		}
 		log.V(3).Info("Workload is evicted because the LocalQueue is stopped", "localQueue", klog.KRef(wl.Namespace, string(wl.Spec.QueueName)))
-		err := workload.EvictWorkload(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByLocalQueueStopped, "", "The LocalQueue is stopped", r.clock)
+		err := workload.Evict(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByLocalQueueStopped, "", "The LocalQueue is stopped", r.clock)
 		return true, err
 	}
 
@@ -516,7 +516,7 @@ func (r *WorkloadReconciler) reconcileOnClusterQueueActiveState(ctx context.Cont
 		}
 		log.V(3).Info("Workload is evicted because the ClusterQueue is stopped", "clusterQueue", klog.KRef("", string(cqName)))
 		message := "The ClusterQueue is stopped"
-		err := workload.EvictWorkload(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByClusterQueueStopped, "", message, r.clock)
+		err := workload.Evict(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByClusterQueueStopped, "", message, r.clock)
 		return true, err
 	}
 
@@ -591,7 +591,7 @@ func (r *WorkloadReconciler) reconcileNotReadyTimeout(ctx context.Context, req c
 		return 0, err
 	}
 	message := fmt.Sprintf("Exceeded the PodsReady timeout %s", req.String())
-	err := workload.EvictWorkload(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByPodsReadyTimeout, underlyingCause, message, r.clock)
+	err := workload.Evict(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedByPodsReadyTimeout, underlyingCause, message, r.clock)
 	return 0, err
 }
 
diff --git a/pkg/controller/jobs/job/job_controller.go b/pkg/controller/jobs/job/job_controller.go
index e44067b37..c8cc26495 100644
--- a/pkg/controller/jobs/job/job_controller.go
+++ b/pkg/controller/jobs/job/job_controller.go
@@ -370,14 +370,6 @@ func SetupIndexes(ctx context.Context, fieldIndexer client.FieldIndexer) error {
 	if err := fieldIndexer.IndexField(ctx, &batchv1.Job{}, indexer.OwnerReferenceUID, indexer.IndexOwnerUID); err != nil {
 		return err
 	}
-	// Add pod index to be able to list pods for elastic-jobs, needed to remove scheduling gate on
-	// admitted workload slices.
-	if features.Enabled(features.ElasticJobsViaWorkloadSlices) {
-		if err := fieldIndexer.IndexField(ctx, &corev1.Pod{}, indexer.OwnerReferenceUID, indexer.IndexOwnerUID); err != nil {
-			return err
-		}
-	}
-
 	return jobframework.SetupWorkloadOwnerIndex(ctx, fieldIndexer, gvk)
 }
 
diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go
index 650e25dfb..9e5f4b005 100644
--- a/pkg/controller/jobs/pod/pod_webhook.go
+++ b/pkg/controller/jobs/pod/pod_webhook.go
@@ -200,11 +200,8 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error {
 			}
 			utilpod.Gate(&pod.pod, kueuealpha.TopologySchedulingGate)
 		}
-
-		if podGroupName(pod.pod) != "" {
-			if err := pod.addRoleHash(); err != nil {
-				return err
-			}
+		if err := pod.addRoleHash(); err != nil {
+			return err
 		}
 		// copy back changes to the object
 		pod.pod.DeepCopyInto(obj.(*corev1.Pod))
diff --git a/pkg/controller/tas/node_failure_controller.go b/pkg/controller/tas/node_failure_controller.go
index 4cdb3182c..5f95d88c3 100644
--- a/pkg/controller/tas/node_failure_controller.go
+++ b/pkg/controller/tas/node_failure_controller.go
@@ -225,7 +225,7 @@ func (r *nodeFailureReconciler) evictWorkload(ctx context.Context, log logr.Logg
 		log = log.WithValues("failedNode", failedNode)
 		log.V(3).Info("Evicting workload due to multiple node failures")
 		evictionMsg := fmt.Sprintf(nodeMultipleFailuresEvictionMessageFormat, failedNode, nodeName)
-		if evictionErr := workload.EvictWorkload(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedDueToNodeFailures, "", evictionMsg, r.clock); evictionErr != nil {
+		if evictionErr := workload.Evict(ctx, r.client, r.recorder, wl, kueue.WorkloadEvictedDueToNodeFailures, "", evictionMsg, r.clock); evictionErr != nil {
 			log.V(2).Error(evictionErr, "Failed to complete eviction process")
 			return false, evictionErr
 		} else {
diff --git a/pkg/scheduler/preemption/preemption.go b/pkg/scheduler/preemption/preemption.go
index 186256bc7..b7f9d1fef 100644
--- a/pkg/scheduler/preemption/preemption.go
+++ b/pkg/scheduler/preemption/preemption.go
@@ -185,7 +185,7 @@ func (p *Preemptor) IssuePreemptions(ctx context.Context, preemptor *workload.In
 func (p *Preemptor) applyPreemptionWithSSA(ctx context.Context, w *kueue.Workload, reason, message string) error {
 	w = w.DeepCopy()
 	workload.SetPreemptedCondition(w, reason, message)
-	return workload.EvictWorkload(ctx, p.client, p.recorder, w, kueue.WorkloadEvictedByPreemption, "", message, p.clock)
+	return workload.Evict(ctx, p.client, p.recorder, w, kueue.WorkloadEvictedByPreemption, "", message, p.clock)
 }
 
 type preemptionAttemptOpts struct {
diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go
index beabbf301..4aab6fa5c 100644
--- a/pkg/scheduler/scheduler.go
+++ b/pkg/scheduler/scheduler.go
@@ -566,7 +566,7 @@ func (s *Scheduler) getInitialAssignments(log logr.Logger, wl *workload.Info, sn
 func (s *Scheduler) evictWorkloadAfterFailedTASReplacement(ctx context.Context, log logr.Logger, wl *kueue.Workload) error {
 	log.V(3).Info("Evicting workload after failed try to find a node replacement; TASFailedNodeReplacementFailFast enabled")
 	msg := fmt.Sprintf("Workload was evicted as there was no replacement for a failed node: %s", workload.NodeToReplace(wl))
-	if err := workload.EvictWorkload(ctx, s.client, s.recorder, wl, kueue.WorkloadEvictedDueToNodeFailures, "", msg, s.clock); err != nil {
+	if err := workload.Evict(ctx, s.client, s.recorder, wl, kueue.WorkloadEvictedDueToNodeFailures, "", msg, s.clock); err != nil {
 		return err
 	}
 	if err := workload.RemoveAnnotation(ctx, s.client, wl, kueuealpha.NodeToReplaceAnnotation); err != nil {
diff --git a/pkg/workload/admissionchecks.go b/pkg/workload/admissionchecks.go
index 4472594e1..9e65dd087 100644
--- a/pkg/workload/admissionchecks.go
+++ b/pkg/workload/admissionchecks.go
@@ -92,8 +92,8 @@ func FindAdmissionCheck(checks []kueue.AdmissionCheckState, checkName kueue.Admi
 	return nil
 }
 
-// ResetChecksOnEviction sets all AdmissionChecks to Pending
-func ResetChecksOnEviction(w *kueue.Workload, now time.Time) {
+// resetChecksOnEviction sets all AdmissionChecks to Pending
+func resetChecksOnEviction(w *kueue.Workload, now time.Time) {
 	checks := w.Status.AdmissionChecks
 	for i := range checks {
 		if checks[i].State == kueue.CheckStatePending {
diff --git a/pkg/workload/workload.go b/pkg/workload/workload.go
index 51954f2c9..646747c29 100644
--- a/pkg/workload/workload.go
+++ b/pkg/workload/workload.go
@@ -827,7 +827,7 @@ func admissionStatusPatch(w *kueue.Workload, wlCopy *kueue.Workload) {
 	wlCopy.Status.NominatedClusterNames = w.Status.NominatedClusterNames
 }
 
-func AdmissionChecksStatusPatch(w *kueue.Workload, wlCopy *kueue.Workload, c clock.Clock) {
+func admissionChecksStatusPatch(w *kueue.Workload, wlCopy *kueue.Workload, c clock.Clock) {
 	if wlCopy.Status.AdmissionChecks == nil && w.Status.AdmissionChecks != nil {
 		wlCopy.Status.AdmissionChecks = make([]kueue.AdmissionCheckState, 0)
 	}
@@ -847,7 +847,7 @@ func ApplyAdmissionStatus(ctx context.Context, c client.Client, w *kueue.Workloa
 func PrepareWorkloadPatch(w *kueue.Workload, strict bool, clk clock.Clock) *kueue.Workload {
 	wlCopy := BaseSSAWorkload(w, strict)
 	admissionStatusPatch(w, wlCopy)
-	AdmissionChecksStatusPatch(w, wlCopy, clk)
+	admissionChecksStatusPatch(w, wlCopy, clk)
 	return wlCopy
 }
 
@@ -1080,9 +1080,9 @@ func AdmissionChecksForWorkload(log logr.Logger, wl *kueue.Workload, admissionCh
 	return acNames
 }
 
-func EvictWorkload(ctx context.Context, c client.Client, recorder record.EventRecorder, wl *kueue.Workload, reason, underlyingCause, msg string, clock clock.Clock) error {
-	PrepareForEviction(wl, clock.Now(), reason, msg)
-	reportWorkloadEvictedOnce := WorkloadEvictionStateInc(wl, reason, underlyingCause)
+func Evict(ctx context.Context, c client.Client, recorder record.EventRecorder, wl *kueue.Workload, reason, underlyingCause, msg string, clock clock.Clock) error {
+	prepareForEviction(wl, clock.Now(), reason, msg)
+	reportWorkloadEvictedOnce := workloadEvictionStateInc(wl, reason, underlyingCause)
 	if err := ApplyAdmissionStatus(ctx, c, wl, true, clock); err != nil {
 		return err
 	}
@@ -1094,25 +1094,25 @@ func EvictWorkload(ctx context.Context, c client.Client, recorder record.EventRe
 		log.V(3).Info("WARNING: unexpected eviction of workload without status.Admission", "workload", klog.KObj(wl))
 		return nil
 	}
-	ReportEvictedWorkload(recorder, wl, wl.Status.Admission.ClusterQueue, reason, msg)
+	reportEvictedWorkload(recorder, wl, wl.Status.Admission.ClusterQueue, reason, msg)
 	if reportWorkloadEvictedOnce {
 		metrics.ReportEvictedWorkloadsOnce(wl.Status.Admission.ClusterQueue, reason, underlyingCause)
 	}
 	return nil
 }
 
-func PrepareForEviction(w *kueue.Workload, now time.Time, reason, message string) {
+func prepareForEviction(w *kueue.Workload, now time.Time, reason, message string) {
 	SetEvictedCondition(w, reason, message)
-	ResetClusterNomination(w)
-	ResetChecksOnEviction(w, now)
+	resetClusterNomination(w)
+	resetChecksOnEviction(w, now)
 }
 
-func ResetClusterNomination(w *kueue.Workload) {
+func resetClusterNomination(w *kueue.Workload) {
 	w.Status.ClusterName = nil
 	w.Status.NominatedClusterNames = nil
 }
 
-func ReportEvictedWorkload(recorder record.EventRecorder, wl *kueue.Workload, cqName kueue.ClusterQueueReference, reason, message string) {
+func reportEvictedWorkload(recorder record.EventRecorder, wl *kueue.Workload, cqName kueue.ClusterQueueReference, reason, message string) {
 	metrics.ReportEvictedWorkloads(cqName, reason)
 	if podsReadyToEvictionTime := workloadsWithPodsReadyToEvictedTime(wl); podsReadyToEvictionTime != nil {
 		metrics.PodsReadyToEvictedTimeSeconds.WithLabelValues(string(cqName), reason).Observe(podsReadyToEvictionTime.Seconds())
@@ -1138,8 +1138,8 @@ func References(wls []*Info) []klog.ObjectRef {
 	return keys
 }
 
-func WorkloadEvictionStateInc(wl *kueue.Workload, reason, underlyingCause string) bool {
-	evictionState := FindSchedulingStatsEvictionByReason(wl, reason, underlyingCause)
+func workloadEvictionStateInc(wl *kueue.Workload, reason, underlyingCause string) bool {
+	evictionState := findSchedulingStatsEvictionByReason(wl, reason, underlyingCause)
 	if evictionState == nil {
 		evictionState = &kueue.WorkloadSchedulingStatsEviction{
 			Reason:          reason,
@@ -1148,11 +1148,11 @@ func WorkloadEvictionStateInc(wl *kueue.Workload, reason, underlyingCause string
 	}
 	report := evictionState.Count == 0
 	evictionState.Count++
-	SetSchedulingStatsEviction(wl, *evictionState)
+	setSchedulingStatsEviction(wl, *evictionState)
 	return report
 }
 
-func FindSchedulingStatsEvictionByReason(wl *kueue.Workload, reason, underlyingCause string) *kueue.WorkloadSchedulingStatsEviction {
+func findSchedulingStatsEvictionByReason(wl *kueue.Workload, reason, underlyingCause string) *kueue.WorkloadSchedulingStatsEviction {
 	if wl.Status.SchedulingStats != nil {
 		for i := range wl.Status.SchedulingStats.Evictions {
 			if wl.Status.SchedulingStats.Evictions[i].Reason == reason && wl.Status.SchedulingStats.Evictions[i].UnderlyingCause == underlyingCause {
@@ -1163,11 +1163,11 @@ func FindSchedulingStatsEvictionByReason(wl *kueue.Workload, reason, underlyingC
 	return nil
 }
 
-func SetSchedulingStatsEviction(wl *kueue.Workload, newEvictionState kueue.WorkloadSchedulingStatsEviction) bool {
+func setSchedulingStatsEviction(wl *kueue.Workload, newEvictionState kueue.WorkloadSchedulingStatsEviction) bool {
 	if wl.Status.SchedulingStats == nil {
 		wl.Status.SchedulingStats = &kueue.SchedulingStats{}
 	}
-	evictionState := FindSchedulingStatsEvictionByReason(wl, newEvictionState.Reason, newEvictionState.UnderlyingCause)
+	evictionState := findSchedulingStatsEvictionByReason(wl, newEvictionState.Reason, newEvictionState.UnderlyingCause)
 	if evictionState == nil {
 		wl.Status.SchedulingStats.Evictions = append(wl.Status.SchedulingStats.Evictions, newEvictionState)
 		return true
diff --git a/site/content/en/docs/contribution_guidelines/testing.md b/site/content/en/docs/contribution_guidelines/testing.md
index d8b080816..9c0109f07 100644
--- a/site/content/en/docs/contribution_guidelines/testing.md
+++ b/site/content/en/docs/contribution_guidelines/testing.md
@@ -110,7 +110,7 @@ and then you can use GUI of the Ginkgo Test Explorer to run individual tests, pr
 You can use the following approach to start up a kind cluster and then run e2e tests from commandline or VSCode,
 attaching them to the existing cluster. For example, suppose you want to test some of the multikueue-e2e tests.
 
-Run `E2E_RUN_ONLY_ENV=true make kind-image-build test-multikueue-e2e` and wait for the `Press Enter to cleanup.` to appear.
+Run `E2E_RUN_ONLY_ENV=true make kind-image-build test-multikueue-e2e` and wait for the `Do you want to cleanup? [Y/n] ` to appear.
 
 The cluster is ready, and now you can run tests from another terminal:
 ```shell
diff --git a/site/content/zh-CN/docs/contribution_guidelines/testing.md b/site/content/zh-CN/docs/contribution_guidelines/testing.md
index 240e820a8..c91c21b73 100644
--- a/site/content/zh-CN/docs/contribution_guidelines/testing.md
+++ b/site/content/zh-CN/docs/contribution_guidelines/testing.md
@@ -110,7 +110,7 @@ func TestValidateClusterQueue(t *testing.T) {
 你可以使用以下方法来启动 kind 集群，然后从命令行或 VSCode 运行 e2e 测试，
 将它们附加到现有集群。例如，假设你想测试一些 multikueue-e2e 测试。
 
-运行 `E2E_RUN_ONLY_ENV=true make kind-image-build test-multikueue-e2e` 并等待 `Press Enter to cleanup.` 出现。
+运行 `E2E_RUN_ONLY_ENV=true make kind-image-build test-multikueue-e2e` 并等待 `Do you want to cleanup? [Y/n] ` 出现。
 
 集群已准备就绪，现在你可以从另一个终端运行测试：
 ```shell



Test patch:
diff --git a/pkg/controller/jobs/pod/pod_webhook_test.go b/pkg/controller/jobs/pod/pod_webhook_test.go
index cc383995d..371d484d5 100644
--- a/pkg/controller/jobs/pod/pod_webhook_test.go
+++ b/pkg/controller/jobs/pod/pod_webhook_test.go
@@ -106,6 +106,7 @@ func TestDefault(t *testing.T) {
 				Queue("test-queue").
 				ManagedByKueueLabel().
 				KueueSchedulingGate().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				Obj(),
 		},
@@ -120,6 +121,7 @@ func TestDefault(t *testing.T) {
 				Queue("test-queue").
 				ManagedByKueueLabel().
 				KueueSchedulingGate().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				Obj(),
 		},
@@ -133,6 +135,7 @@ func TestDefault(t *testing.T) {
 			want: testingpod.MakePod("test-pod", defaultNamespace.Name).
 				ManagedByKueueLabel().
 				KueueSchedulingGate().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				Obj(),
 		},
@@ -151,6 +154,7 @@ func TestDefault(t *testing.T) {
 				Queue("test-queue").
 				ManagedByKueueLabel().
 				KueueSchedulingGate().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				OwnerReference("parent-job", batchv1.SchemeGroupVersion.WithKind("Job")).
 				Obj(),
@@ -361,6 +365,7 @@ func TestDefault(t *testing.T) {
 				Annotation(kueuealpha.PodSetRequiredTopologyAnnotation, "block").
 				ManagedByKueueLabel().
 				KueueFinalizer().
+				RoleHash("a9f06f3a").
 				KueueSchedulingGate().
 				TopologySchedulingGate().
 				Obj(),
@@ -383,6 +388,7 @@ func TestDefault(t *testing.T) {
 				Label("test-label", "test-value").
 				Label(kueuealpha.PodGroupPodIndexLabel, "test-value").
 				ManagedByKueueLabel().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				KueueSchedulingGate().
 				TopologySchedulingGate().
@@ -400,6 +406,7 @@ func TestDefault(t *testing.T) {
 				Queue("default").
 				ManagedByKueueLabel().
 				KueueSchedulingGate().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				Obj(),
 		},
@@ -427,6 +434,7 @@ func TestDefault(t *testing.T) {
 				Queue("queue").
 				ManagedByKueueLabel().
 				KueueSchedulingGate().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				Obj(),
 		},
@@ -440,6 +448,7 @@ func TestDefault(t *testing.T) {
 				Queue("queue").
 				ManagedByKueueLabel().
 				KueueSchedulingGate().
+				RoleHash("a9f06f3a").
 				KueueFinalizer().
 				Obj(),
 		},
@@ -464,6 +473,7 @@ func TestDefault(t *testing.T) {
 			want: testingpod.MakePod("test-pod", defaultNamespace.Name).
 				Queue("queue").
 				ManagedByKueueLabel().
+				RoleHash("a9f06f3a").
 				KueueSchedulingGate().
 				KueueFinalizer().
 				Obj(),
@@ -491,6 +501,7 @@ func TestDefault(t *testing.T) {
 				Label(corev1.LabelMetadataName, "test-pod").
 				Queue("queue").
 				ManagedByKueueLabel().
+				RoleHash("a9f06f3a").
 				KueueSchedulingGate().
 				KueueFinalizer().
 				Obj(),
diff --git a/test/e2e/singlecluster/pod_test.go b/test/e2e/singlecluster/pod_test.go
index b8d35ee72..ddb43b866 100644
--- a/test/e2e/singlecluster/pod_test.go
+++ b/test/e2e/singlecluster/pod_test.go
@@ -30,6 +30,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
+	"sigs.k8s.io/kueue/pkg/constants"
+	controllerconsts "sigs.k8s.io/kueue/pkg/controller/constants"
 	"sigs.k8s.io/kueue/pkg/controller/jobs/pod"
 	podconstants "sigs.k8s.io/kueue/pkg/controller/jobs/pod/constants"
 	"sigs.k8s.io/kueue/pkg/util/testing"
@@ -551,5 +553,68 @@ var _ = ginkgo.Describe("Pod groups", func() {
 				util.ExpectWorkloadToFinish(ctx, k8sClient, defaultGroupKey)
 			})
 		})
+
+		ginkgo.It("Pod should be admitted after the group labels are added", func() {
+			ginkgo.By("creating a pod", func() {
+				p := podtesting.MakePod("pod-0", ns.Name).
+					Image(util.GetAgnHostImage(), util.BehaviorExitFast).
+					RequestAndLimit(corev1.ResourceCPU, "1").
+					Label("app-role", "worker").
+					Annotation(podconstants.SuspendedByParentAnnotation, "OtherController").
+					KueueSchedulingGate().
+					Obj()
+				util.MustCreate(ctx, k8sClient, p)
+
+				gomega.Eventually(func(g gomega.Gomega) {
+					var createdPod corev1.Pod
+					g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(p), &createdPod)).To(gomega.Succeed())
+					g.Expect(createdPod.Spec.SchedulingGates).To(gomega.ContainElement(corev1.PodSchedulingGate{Name: podconstants.SchedulingGateName}))
+					g.Expect(createdPod.Annotations).To(gomega.HaveKey(podconstants.RoleHashAnnotation))
+				}, util.Timeout, util.Interval).Should(gomega.Succeed())
+			})
+			ginkgo.By("Add the group annotations and labels to the pod", func() {
+				var p corev1.Pod
+				pKey := client.ObjectKey{Namespace: ns.Name, Name: "pod-0"}
+				gomega.Expect(k8sClient.Get(ctx, pKey, &p)).To(gomega.Succeed())
+
+				p.Labels[podconstants.GroupNameLabel] = "test-group"
+				p.Annotations[podconstants.GroupTotalCountAnnotation] = "1"
+				p.Annotations[podconstants.SuspendedByParentAnnotation] = "OtherController"
+				p.Labels[controllerconsts.QueueLabel] = lq.Name
+				p.Labels[constants.ManagedByKueueLabelKey] = constants.ManagedByKueueLabelValue
+				gomega.Expect(k8sClient.Update(ctx, &p)).To(gomega.Succeed())
+
+				var podWithHash corev1.Pod
+				gomega.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&p), &podWithHash)).To(gomega.Succeed())
+				initialRoleHash := podWithHash.Annotations[podconstants.RoleHashAnnotation]
+				gomega.Expect(initialRoleHash).NotTo(gomega.BeEmpty())
+			})
+
+			ginkgo.By("Verify the pod is scheduled and runs", func() {
+				pKey := client.ObjectKey{Namespace: ns.Name, Name: "pod-0"}
+				gomega.Eventually(func(g gomega.Gomega) {
+					var runningPod corev1.Pod
+					g.Expect(k8sClient.Get(ctx, pKey, &runningPod)).To(gomega.Succeed())
+					g.Expect(runningPod.Spec.SchedulingGates).To(gomega.BeEmpty())
+				}, util.Timeout, util.Interval).Should(gomega.Succeed())
+
+				gKey := client.ObjectKey{Namespace: ns.Name, Name: "test-group"}
+				util.ExpectWorkloadToFinish(ctx, k8sClient, gKey)
+			})
+
+			ginkgo.By("Ensure the pod is deleted", func() {
+				var p corev1.Pod
+				pKey := client.ObjectKey{Namespace: ns.Name, Name: "pod-0"}
+				gomega.Expect(k8sClient.Get(ctx, pKey, &p)).To(gomega.Succeed())
+				gomega.Expect(k8sClient.Delete(ctx, &p)).To(gomega.Succeed())
+
+				gomega.Eventually(func(g gomega.Gomega) {
+					g.Expect(k8sClient.Get(ctx, pKey, &corev1.Pod{})).To(testing.BeNotFoundError())
+				}, util.Timeout, util.Interval).Should(gomega.Succeed())
+
+				gKey := client.ObjectKey{Namespace: ns.Name, Name: "test-group"}
+				util.ExpectWorkloadsFinalizedOrGone(ctx, k8sClient, gKey)
+			})
+		})
 	})
 })