fix comments

Signed-off-by: Wantong Jiang <[email protected]>

Author: jwtty

pkg/controllers/clusterresourceplacement/controller.go

@@ -135,7 +135,7 @@ func (r *Reconciler) handleUpdate(ctx context.Context, placementObj fleetv1beta1
 
 	if placementSpec.RevisionHistoryLimit != nil {
 		if revisionLimit <= 0 {
-			err := fmt.Errorf("invalid placement %s: invalid revisionHistoryLimit %d", placementKObj, revisionLimit)
+			err := fmt.Errorf("invalid placement %s/%s: invalid revisionHistoryLimit %d", placementObj.GetNamespace(), placementObj.GetName(), revisionLimit)
 			klog.ErrorS(controller.NewUnexpectedBehaviorError(err), "Invalid revisionHistoryLimit value and using default value instead", "placement", placementKObj)
 		} else {
 			revisionLimit = *placementSpec.RevisionHistoryLimit
@@ -1053,10 +1053,9 @@ func (r *Reconciler) determineRolloutStateForPlacementWithExternalRolloutStrateg
 	allRPS []fleetv1beta1.PerClusterPlacementStatus,
 	selectedResourceIDs []fleetv1beta1.ResourceIdentifier,
 ) (bool, error) {
-	placementKObj := klog.KObj(placementObj)
 	if len(selected) == 0 {
 		// This should not happen as we already checked in setPlacementStatus.
-		err := controller.NewUnexpectedBehaviorError(fmt.Errorf("selected cluster list is empty for placement %s when checking per-cluster rollout state", placementKObj))
+		err := controller.NewUnexpectedBehaviorError(fmt.Errorf("selected cluster list is empty for placement %s/%s when checking per-cluster rollout state", placementObj.GetNamespace(), placementObj.GetName()))
 		klog.ErrorS(err, "Should not happen: selected cluster list is empty in determineRolloutStateForPlacementWithExternalRolloutStrategy()")
 		return false, err
 	}

pkg/controllers/rollout/controller.go

@@ -262,30 +262,30 @@ func waitForResourcesToCleanUp(allBindings []placementv1beta1.BindingObj, placem
 			if _, exist := bindingMap[bindingSpec.TargetCluster]; !exist {
 				bindingMap[bindingSpec.TargetCluster] = binding
 			} else {
-				return false, controller.NewUnexpectedBehaviorError(fmt.Errorf("the same cluster `%s` has bindings `%s` and `%s` pointing to it",
-					bindingSpec.TargetCluster, klog.KObj(bindingMap[bindingSpec.TargetCluster]), bindingKRef))
+				return false, controller.NewUnexpectedBehaviorError(fmt.Errorf("the same cluster `%s` has bindings `%s/%s` and `%s/%s` pointing to it",
+					bindingSpec.TargetCluster, bindingMap[bindingSpec.TargetCluster].GetNamespace(), bindingMap[bindingSpec.TargetCluster].GetName(),
+					binding.GetNamespace(), binding.GetName()))
 			}
 		}
 	}
 	// check if there are any cluster that has a binding that is both being deleted and scheduled
 	for cluster, binding := range bindingMap {
 		// check if there is a deleting binding on the same cluster
-		bindingKRef := klog.KObj(binding)
 		if deletingBinding[cluster] {
-			klog.V(2).InfoS("Find a binding assigned to a cluster with another deleting binding", "placement", placementObjRef, "binding", bindingKRef)
+			klog.V(2).InfoS("Find a binding assigned to a cluster with another deleting binding", "placement", placementObjRef, "binding", klog.KObj(binding))
 			bindingSpec := binding.GetBindingSpec()
 			if bindingSpec.State == placementv1beta1.BindingStateBound {
 				// the rollout controller won't move a binding from scheduled state to bound if there is a deleting binding on the same cluster.
 				return false, controller.NewUnexpectedBehaviorError(fmt.Errorf(
-					"find a cluster `%s` that has a bound binding `%s` and a deleting binding point to it", bindingSpec.TargetCluster, bindingKRef))
+					"find a cluster `%s` that has a bound binding `%s/%s` and a deleting binding point to it", bindingSpec.TargetCluster, binding.GetNamespace(), binding.GetName()))
 			}
 			if bindingSpec.State == placementv1beta1.BindingStateUnscheduled {
 				// this is a very rare case that the resource was in the middle of being removed from a member cluster after it is unselected.
 				// then the cluster get selected and unselected in two scheduling before the member agent is able to clean up all the resources.
 				if binding.GetAnnotations()[placementv1beta1.PreviousBindingStateAnnotation] == string(placementv1beta1.BindingStateBound) {
 					// its previous state can not be bound as rollout won't roll a binding with a deleting binding pointing to the same cluster.
 					return false, controller.NewUnexpectedBehaviorError(fmt.Errorf(
-						"find a cluster `%s` that has a unscheduled binding `%s` with previous state is `bound` and a deleting binding point to it", bindingSpec.TargetCluster, bindingKRef))
+						"find a cluster `%s` that has a unscheduled binding `%s/%s` with previous state is `bound` and a deleting binding point to it", bindingSpec.TargetCluster, binding.GetNamespace(), binding.GetName()))
 				}
 				return true, nil
 			}
@@ -764,11 +764,11 @@ func resourceSnapshotObjHandlerFuncs() handler.Funcs {
 func resourceOverrideSnapshotHandlerFuncs(enqueueCRP bool) handler.Funcs {
 	return handler.Funcs{
 		CreateFunc: func(ctx context.Context, e event.CreateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
-			klog.V(2).InfoS("Handling a resourceOverrideSnapshot create event", "resourceOverrideSnapshot", klog.KObj(e.Object))
+			klog.V(2).InfoS("Handling a resourceOverrideSnapshot create event", "resourceOverrideSnapshot", klog.KObj(e.Object), "enqueueCRP", enqueueCRP)
 			handleResourceOverrideSnapshot(e.Object, q, enqueueCRP)
 		},
 		GenericFunc: func(ctx context.Context, e event.GenericEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
-			klog.V(2).InfoS("Handling a resourceOverrideSnapshot generic event", "resourceOverrideSnapshot", klog.KObj(e.Object))
+			klog.V(2).InfoS("Handling a resourceOverrideSnapshot generic event", "resourceOverrideSnapshot", klog.KObj(e.Object), "enqueueCRP", enqueueCRP)
 			handleResourceOverrideSnapshot(e.Object, q, enqueueCRP)
 		},
 	}
@@ -778,7 +778,7 @@ func resourceOverrideSnapshotHandlerFuncs(enqueueCRP bool) handler.Funcs {
 func resourceOverrideHandlerFuncs(enqueueCRP bool) handler.Funcs {
 	return handler.Funcs{
 		DeleteFunc: func(ctx context.Context, e event.DeleteEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
-			klog.V(2).InfoS("Handling a resourceOverride delete event", "resourceOverride", klog.KObj(e.Object))
+			klog.V(2).InfoS("Handling a resourceOverride delete event", "resourceOverride", klog.KObj(e.Object), "enqueueCRP", enqueueCRP)
 			handleResourceOverride(e.Object, q, enqueueCRP)
 		},
 	}
@@ -880,18 +880,18 @@ func handleResourceOverrideSnapshot(o client.Object, q workqueue.TypedRateLimiti
 	namespace := ""
 	if enqueueCRP {
 		if placementInOverride.Scope == placementv1beta1.NamespaceScoped {
-			klog.V(2).InfoS("Skipping a resourceOverrideSnapshot event with resource placement", "resourceOverrideSnapshot", snapshotKRef)
+			klog.V(2).InfoS("Skipping a resourceOverrideSnapshot event with resource placement", "resourceOverrideSnapshot", snapshotKRef, "enqueueCRP", enqueueCRP)
 			return
 		}
 	} else {
 		if placementInOverride.Scope != placementv1beta1.NamespaceScoped {
-			klog.V(2).InfoS("Skipping a resourceOverrideSnapshot event with cluster resource placement", "resourceOverrideSnapshot", snapshotKRef)
+			klog.V(2).InfoS("Skipping a resourceOverrideSnapshot event with cluster resource placement", "resourceOverrideSnapshot", snapshotKRef, "enqueueCRP", enqueueCRP)
 			return
 		}
 		namespace = snapshot.GetNamespace()
 	}
 
-	klog.V(2).InfoS("Handling a resourceOverrideSnapshot event", "resourceOverrideSnapshot", snapshotKRef)
+	klog.V(2).InfoS("Handling a resourceOverrideSnapshot event", "resourceOverrideSnapshot", snapshotKRef, "enqueueCRP", enqueueCRP)
 	q.Add(reconcile.Request{
 		NamespacedName: types.NamespacedName{Name: placementInOverride.Name, Namespace: namespace},
 	})
@@ -951,18 +951,18 @@ func handleResourceOverride(o client.Object, q workqueue.TypedRateLimitingInterf
 	namespace := ""
 	if enqueueCRP {
 		if ro.Spec.Placement.Scope == placementv1beta1.NamespaceScoped {
-			klog.V(2).InfoS("Skipping a resourceOverride event with resource placement", "resourceOverride", klog.KObj(ro))
+			klog.V(2).InfoS("Skipping a resourceOverride event with resource placement", "resourceOverride", klog.KObj(ro), "enqueueCRP", enqueueCRP)
 			return
 		}
 	} else {
 		if ro.Spec.Placement.Scope != placementv1beta1.NamespaceScoped {
-			klog.V(2).InfoS("Skipping a resourceOverride event with cluster resource placement", "resourceOverride", klog.KObj(ro))
+			klog.V(2).InfoS("Skipping a resourceOverride event with cluster resource placement", "resourceOverride", klog.KObj(ro), "enqueueCRP", enqueueCRP)
 			return
 		}
 		namespace = ro.Namespace
 	}
 
-	klog.V(2).InfoS("Handling a resourceOverride event", "resourceOverride", klog.KObj(ro))
+	klog.V(2).InfoS("Handling a resourceOverride event", "resourceOverride", klog.KObj(ro), "enqueueCRP", enqueueCRP)
 	q.Add(reconcile.Request{
 		NamespacedName: types.NamespacedName{Name: ro.Spec.Placement.Name, Namespace: namespace},
 	})