wip3

Author: mszadkow

pkg/controller/admissionchecks/multikueue/workload.go

@@ -365,31 +365,35 @@ func (w *wlReconciler) reconcileGroup(ctx context.Context, group *wlGroup) (reco
 				// We'll retry this in the next reconciling.
 				return reconcile.Result{}, err
 			}
-		} else {
-			// workload eviction on worker cluster
-			log.V(3).Info("Workload gets evicted in the remote cluster", "cluster", evictedRemote)
+			return reconcile.Result{}, nil
+		}
 
-			for cluster := range group.remotes {
-				err := group.RemoveRemoteObjects(ctx, cluster)
-				if err != nil {
-					return reconcile.Result{}, err
-				}
+		// workload eviction on worker cluster
+		log.V(3).Info("Workload gets evicted in the remote cluster", "cluster", evictedRemote)
+
+		needsACUpdate := acs.State == kueue.CheckStateReady
+		if err := workload.PatchAdmissionStatus(ctx, w.client, group.local, w.clock, func(wl *kueue.Workload) (bool, error) {
+			if needsACUpdate {
+				acs.State = kueue.CheckStatePending
+				acs.Message = fmt.Sprintf("Workload evicted on worker cluster: %q, resetting for re-admission", *group.local.Status.ClusterName)
+				acs.LastTransitionTime = metav1.NewTime(w.clock.Now())
+				workload.SetAdmissionCheckState(&wl.Status.AdmissionChecks, *acs, w.clock)
+				wl.Status.ClusterName = nil
+				wl.Status.NominatedClusterNames = nil
 			}
-			needsACUpdate := acs.State == kueue.CheckStateReady
-			if err := workload.PatchAdmissionStatus(ctx, w.client, group.local, w.clock, func(wl *kueue.Workload) (bool, error) {
-				if needsACUpdate {
-					acs.State = kueue.CheckStatePending
-					// update the message
-					acs.Message = fmt.Sprintf("Workload evicted on worker cluster: %q, resetting for re-admission", *group.local.Status.ClusterName)
-					// update the transition time since is used to detect the lost worker state.
-					acs.LastTransitionTime = metav1.NewTime(w.clock.Now())
-					workload.SetAdmissionCheckState(&group.local.Status.AdmissionChecks, *acs, w.clock)
-					group.local.Status.ClusterName = nil
-					group.local.Status.NominatedClusterNames = nil
-				}
-				return true, nil
-			}); err != nil {
-				log.Error(err, "Failed to patch workload status")
+			return true, nil
+		}); err != nil {
+			log.Error(err, "Failed to patch workload status")
+			return reconcile.Result{}, err
+		}
+
+		if needsACUpdate {
+			w.recorder.Eventf(group.local, corev1.EventTypeNormal, "MultiKueue", acs.Message)
+		}
+
+		for cluster := range group.remotes {
+			if err := client.IgnoreNotFound(group.RemoveRemoteObjects(ctx, cluster)); err != nil {
+				log.Error(err, "Failed to remove cluster remote objects", "cluster", cluster)
 				return reconcile.Result{}, err
 			}
 		}

pkg/controller/admissionchecks/multikueue/workload_test.go

@@ -697,8 +697,8 @@ func TestWlReconcile(t *testing.T) {
 				*baseWorkloadBuilder.Clone().
 					AdmissionCheck(kueue.AdmissionCheckState{
 						Name:    "ac1",
-						State:   kueue.CheckStateReady,
-						Message: `The workload got reservation on "worker1"`,
+						State:   kueue.CheckStatePending,
+						Message: `Workload evicted on worker cluster: "worker1", resetting for re-admission`,
 					}).
 					ControllerReference(batchv1.SchemeGroupVersion.WithKind("Job"), "job1", "uid1").
 					ReserveQuotaAt(utiltestingapi.MakeAdmission("q1").Obj(), now).
@@ -711,6 +711,14 @@ func TestWlReconcile(t *testing.T) {
 			wantWorker1Workloads: []kueue.Workload{},
 			wantWorker1Jobs:      []batchv1.Job{},
 			wantWorker2Workloads: []kueue.Workload{},
+			wantEvents: []utiltesting.EventRecord{
+				{
+					Key:       client.ObjectKeyFromObject(baseWorkloadBuilder.Clone().Obj()),
+					EventType: "Normal",
+					Reason:    "MultiKueue",
+					Message:   `Workload evicted on worker cluster: "worker1", resetting for re-admission`,
+				},
+			},
 		},
 		"remote wl with reservation (withoutJobManagedBy)": {
 			features:     map[featuregate.Feature]bool{features.MultiKueueBatchJobWithManagedBy: false},
@@ -748,7 +756,6 @@ func TestWlReconcile(t *testing.T) {
 					}).
 					ControllerReference(batchv1.SchemeGroupVersion.WithKind("Job"), "job1", "uid1").
 					ReserveQuotaAt(utiltestingapi.MakeAdmission("q1").Obj(), now).
-					ClusterName("worker1").
 					Obj(),
 			},
 			wantManagersJobs: []batchv1.Job{
@@ -1760,7 +1767,7 @@ func TestWlReconcile(t *testing.T) {
 				// However, other important Status fields (e.g. Conditions) still reflect the change,
 				// so we deliberately ignore the Admission field here.
 				if features.Enabled(features.WorkloadRequestUseMergePatch) {
-					objCheckOpts = append(objCheckOpts, cmpopts.IgnoreFields(kueue.WorkloadStatus{}, "Admission"))
+					objCheckOpts = append(objCheckOpts, cmpopts.IgnoreFields(kueue.WorkloadStatus{}, "Admission", "ClusterName"))
 				}
 
 				gotManagersWorkloads := &kueue.WorkloadList{}

test/integration/multikueue/jobs_test.go

@@ -1964,13 +1964,9 @@ var _ = ginkgo.Describe("MultiKueue", ginkgo.Label("area:multikueue", "feature:m
 			gomega.Expect(managerTestCluster.client.Get(managerTestCluster.ctx, wlLookupKey, managerWl)).To(gomega.Succeed())
 			gomega.Eventually(func(g gomega.Gomega) {
 				createdWorkload := &kueue.Workload{}
-				err := worker1TestCluster.client.Get(worker1TestCluster.ctx, wlLookupKey, createdWorkload)
-				ginkgo.GinkgoLogr.Info("Checking worker1 workload", "workload", createdWorkload != nil, "err", err)
-				// g.Expect(err).To(gomega.Succeed())
-				// g.Expect(createdWorkload.Spec).To(gomega.BeComparableTo(managerWl.Spec))
-				err = worker2TestCluster.client.Get(worker2TestCluster.ctx, wlLookupKey, createdWorkload)
-				ginkgo.GinkgoLogr.Info("Checking worker2 workload", "workload", createdWorkload != nil, "err", err)
-				g.Expect(err).To(gomega.Succeed())
+				g.Expect(worker1TestCluster.client.Get(worker1TestCluster.ctx, wlLookupKey, createdWorkload)).To(gomega.Succeed())
+				g.Expect(createdWorkload.Spec).To(gomega.BeComparableTo(managerWl.Spec))
+				g.Expect(worker2TestCluster.client.Get(worker2TestCluster.ctx, wlLookupKey, createdWorkload)).To(gomega.Succeed())
 				g.Expect(createdWorkload.Spec).To(gomega.BeComparableTo(managerWl.Spec))
 			}, util.LongTimeout, util.Interval).Should(gomega.Succeed())
 		})