resolve comments of review

cmd/kueue/main.go

@@ -226,7 +226,7 @@ func setupControllers(mgr ctrl.Manager, cCache *cache.Cache, queues *queue.Manag
 	// setup provision admission check controller
 	if features.Enabled(features.ProvisioningACC) && provisioning.ServerSupportsProvisioningRequest(mgr) {
 		// A info message is added in setupIndexes if autoscaling is not supported by the cluster
-		if err := provisioning.NewController(mgr.GetClient(), mgr.GetEventRecorderFor("admission-checks-controller")).SetupWithManager(mgr); err != nil {
+		if err := provisioning.NewController(mgr.GetClient(), mgr.GetEventRecorderFor("kueue-provisioning-request-controller")).SetupWithManager(mgr); err != nil {
 			setupLog.Error(err, "Could not setup provisioning controller")
 			os.Exit(1)
 		}

pkg/controller/admissionchecks/provisioning/admissioncheck_reconciler.go

@@ -19,10 +19,8 @@ package provisioning
 import (
 	"context"
 
-	corev1 "k8s.io/api/core/v1"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -33,7 +31,6 @@ import (
 type acReconciler struct {
 	client client.Client
 	helper *storeHelper
-	record record.EventRecorder
 }
 
 var _ reconcile.Reconciler = (*acReconciler)(nil)
@@ -64,11 +61,7 @@ func (a *acReconciler) Reconcile(ctx context.Context, req reconcile.Request) (re
 
 	if currentCondition.Status != newCondition.Status {
 		apimeta.SetStatusCondition(&ac.Status.Conditions, newCondition)
-		if err := a.client.Status().Update(ctx, ac); err != nil {
-			return reconcile.Result{}, err
-		}
-		a.record.Eventf(ac, corev1.EventTypeNormal, "UpdatedAdmissionChecks", "Admission checks status is changed to %s", newCondition.Status)
-		return reconcile.Result{}, nil
+		return reconcile.Result{}, a.client.Status().Update(ctx, ac)
 	}
 	return reconcile.Result{}, nil
 }

pkg/controller/admissionchecks/provisioning/admissioncheck_reconciler_test.go

@@ -20,11 +20,9 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	corev1 "k8s.io/api/core/v1"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
@@ -124,11 +122,9 @@ func TestReconcileAdmissionCheck(t *testing.T) {
 				client: k8sclient,
 			}
 
-			recorder := record.NewBroadcaster().NewRecorder(k8sclient.Scheme(), corev1.EventSource{Component: "admission-checks-controller"})
 			reconciler := acReconciler{
 				client: k8sclient,
 				helper: &helper,
-				record: recorder,
 			}
 
 			req := reconcile.Request{

pkg/controller/admissionchecks/provisioning/controller.go

@@ -435,19 +435,24 @@ func (c *Controller) syncCheckStates(ctx context.Context, wl *kueue.Workload, ch
 				}
 			}
 		}
-		workload.SetAdmissionCheckState(&wlPatch.Status.AdmissionChecks, checkState)
-		message := fmt.Sprintf("Admission check %s set state %s", checkState.Name, checkState.State)
-		if checkState.Message != "" {
-			message += fmt.Sprintf(" with message %s", checkState.Message)
+
+		existingCondition := workload.FindAdmissionCheck(wlPatch.Status.AdmissionChecks, checkState.Name)
+		if existingCondition != nil && existingCondition.State != checkState.State {
+			message := fmt.Sprintf("Admission check %s updated state from %s to %s", checkState.Name, existingCondition.State, checkState.State)
+			if checkState.Message != "" {
+				message += fmt.Sprintf(" with message %s", checkState.Message)
+			}
+			recorderMessages = append(recorderMessages, message)
 		}
-		recorderMessages = append(recorderMessages, message)
+
+		workload.SetAdmissionCheckState(&wlPatch.Status.AdmissionChecks, checkState)
 	}
 	if updated {
 		if err := c.client.Status().Patch(ctx, wlPatch, client.Apply, client.FieldOwner(ControllerName), client.ForceOwnership); err != nil {
 			return err
 		}
 		for i := range recorderMessages {
-			c.record.Eventf(wl, corev1.EventTypeNormal, "WorkloadAdmissionChecksUpdated", api.TruncateEventMessage(recorderMessages[i]))
+			c.record.Eventf(wl, corev1.EventTypeNormal, "AdmissionCheckUpdated", api.TruncateEventMessage(recorderMessages[i]))
 		}
 	}
 	return nil
@@ -633,7 +638,6 @@ func (c *Controller) SetupWithManager(mgr ctrl.Manager) error {
 	acReconciler := &acReconciler{
 		client: c.client,
 		helper: c.helper,
-		record: c.record,
 	}
 
 	return ctrl.NewControllerManagedBy(mgr).

pkg/controller/core/workload_controller.go

@@ -219,10 +219,10 @@ func (r *WorkloadReconciler) reconcileSyncAdmissionChecks(ctx context.Context, w
 		log := ctrl.LoggerFrom(ctx)
 		log.V(3).Info("The workload needs admission checks updates", "clusterQueue", klog.KRef("", cqName), "admissionChecks", queueAdmissionChecks)
 		wl.Status.AdmissionChecks = newChecks
-		if err := r.client.Status().Update(ctx, wl); !apierrors.IsNotFound(err) {
-			return true, err
+		if err := r.client.Status().Update(ctx, wl); err != nil {
+			return true, client.IgnoreNotFound(err)
 		}
-		r.record.Eventf(wl, corev1.EventTypeNormal, "WorkloadAdmissionChecksUpdated", "Updated admission checks of the workload %v for clusterQueue %s", workload.Key(wl), cqName)
+		r.record.Eventf(wl, corev1.EventTypeNormal, "AdmissionChecksChanged", "Updated the list of admission checks for the workload, based on the clusterQueue %s", cqName)
 		return true, nil
 	}
 	return false, nil