address comment

Signed-off-by: Ryan Zhang <[email protected]>

Author: ryanzhang-oss

pkg/scheduler/framework/frameworkutils.go

@@ -207,7 +207,7 @@ func crossReferencePickedClustersAndDeDupBindings(
 			}
 			binding, err := generateBinding(placementKey, scored.Cluster.Name)
 			if err != nil {
-				return nil, nil, nil, err
+				return nil, nil, nil, fmt.Errorf("failed to generate binding for cluster %q: %w", scored.Cluster.Name, err)
 			}
 			// Set the binding spec.
 			binding.SetBindingSpec(bindingSpec)
@@ -740,7 +740,7 @@ func crossReferenceValidTargetsWithBindings(
 			binding, err := generateBinding(placementKey, cluster.Name)
 			if err != nil {
 				// Cannot generate a binding; normally this should never happen.
-				return nil, nil, nil, controller.NewUnexpectedBehaviorError(fmt.Errorf("failed to generate binding: %w", err))
+				return nil, nil, nil, fmt.Errorf("failed to generate binding: %w", err)
 			}
 			// Set the binding spec.
 			spec := placementv1beta1.ResourceBindingSpec{
@@ -763,14 +763,9 @@ func crossReferenceValidTargetsWithBindings(
 
 	// Perform the cross-reference to find out bindings that should be deleted.
 	for _, binding := range obsolete {
-		spec := binding.GetBindingSpec()
-		if _, ok := validTargetMap[spec.TargetCluster]; !ok {
+		if _, ok := validTargetMap[binding.GetBindingSpec().TargetCluster]; !ok {
 			// The cluster is no longer a valid target; mark the binding as unscheduled.
-			if crb, ok := binding.(*placementv1beta1.ClusterResourceBinding); ok {
-				toDelete = append(toDelete, crb)
-			} else {
-				return nil, nil, nil, controller.NewUnexpectedBehaviorError(fmt.Errorf("unexpected binding type: %T", binding))
-			}
+			toDelete = append(toDelete, binding)
 		}
 	}
 

pkg/scheduler/scheduler.go

@@ -167,7 +167,7 @@ func (s *Scheduler) scheduleOnce(ctx context.Context, worker int) {
 			if err := s.cleanUpAllBindingsFor(ctx, placement); err != nil {
 				klog.ErrorS(err, "Failed to clean up all bindings for placement", "placement", placementRef)
 				if errors.Is(err, controller.ErrUnexpectedBehavior) {
-					// The placement is in an unexpected state; this is a scheduler-side error, and
+					// The placement is in an unexpected state, don't requeue it.
 					return
 				}
 				// Requeue for later processing.
@@ -324,7 +324,7 @@ func (s *Scheduler) cleanUpAllBindingsFor(ctx context.Context, placement fleetv1
 		binding := bindings[idx]
 		controllerutil.RemoveFinalizer(binding, fleetv1beta1.SchedulerCRBCleanupFinalizer)
 		if err := s.client.Update(ctx, binding); err != nil {
-			klog.ErrorS(err, "Failed to remove scheduler reconcile finalizer from resource binding", "binding", klog.KObj(binding))
+			klog.ErrorS(err, "Failed to remove scheduler reconcile finalizer from binding", "binding", klog.KObj(binding))
 			return controller.NewUpdateIgnoreConflictError(err)
 		}
 		// Delete the binding if it has not been marked for deletion yet.

pkg/utils/controller/binding_resolver.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"context"
+	"fmt"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -33,7 +34,7 @@ func ListBindingsFromKey(ctx context.Context, c client.Reader, placementKey queu
 	// Extract namespace and name from the placement key
 	namespace, name, err := ExtractNamespaceNameFromKey(placementKey)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to list binding for a placement: %w", err)
 	}
 	var listOptions []client.ListOption
 	var bindingList placementv1beta1.BindingObjList

pkg/utils/controller/placement_resolver.go

@@ -87,7 +87,7 @@ func ExtractNamespaceNameFromKey(placementKey queue.PlacementKey) (string, strin
 			return "", "", NewUnexpectedBehaviorError(fmt.Errorf("invalid placement key format: %s", keyStr))
 		}
 		if len(parts[0]) == 0 || len(parts[1]) == 0 {
-			return "", "", NewUnexpectedBehaviorError(fmt.Errorf("empty placement key"))
+			return "", "", NewUnexpectedBehaviorError(fmt.Errorf("empty placement key <namespace/name>: %s", keyStr))
 		}
 		return parts[0], parts[1], nil
 	} else {