test

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

Author: Ryan Zhang

.github/.copilot/breadcrumbs/2025-06-13-1500-scheduler-binding-interface-refactor.md

@@ -140,7 +140,7 @@ func (c *CycleState) HasObsoleteBindingFor(clusterName string) bool {
 // IsClusterObsolete
 
 // NewCycleState creates a CycleState.
-func NewCycleState(clusters []clusterv1beta1.MemberCluster, obsoleteBindings []*placementv1beta1.ClusterResourceBinding, scheduledOrBoundBindings ...[]*placementv1beta1.ClusterResourceBinding) *CycleState {
+func NewCycleState(clusters []clusterv1beta1.MemberCluster, obsoleteBindings []placementv1beta1.BindingObj, scheduledOrBoundBindings ...[]placementv1beta1.BindingObj) *CycleState {
 	return &CycleState{
 		store:                    sync.Map{},
 		clusters:                 clusters,

pkg/scheduler/framework/cyclestate.go

@@ -67,7 +67,18 @@ func TestCycleStateBasicOps(t *testing.T) {
 		},
 	}
 
-	cs := NewCycleState(clusters, obsoleteBindings, scheduledOrBoundBindings)
+	// Convert concrete bindings to interface slices
+	obsoleteBindingObjs := make([]placementv1beta1.BindingObj, len(obsoleteBindings))
+	for i, binding := range obsoleteBindings {
+		obsoleteBindingObjs[i] = binding
+	}
+
+	scheduledOrBoundBindingObjs := make([]placementv1beta1.BindingObj, len(scheduledOrBoundBindings))
+	for i, binding := range scheduledOrBoundBindings {
+		scheduledOrBoundBindingObjs[i] = binding
+	}
+
+	cs := NewCycleState(clusters, obsoleteBindingObjs, scheduledOrBoundBindingObjs)
 
 	k, v := "key", "value"
 	cs.Write(StateKey(k), StateValue(v))
@@ -125,7 +136,17 @@ func TestPrepareScheduledOrBoundBindingsMap(t *testing.T) {
 		altClusterName: true,
 	}
 
-	scheduleOrBoundBindingsMap := prepareScheduledOrBoundBindingsMap(scheduled, bound)
+	// Convert concrete bindings to interface slices
+	scheduledObjs := make([]placementv1beta1.BindingObj, len(scheduled))
+	for i, binding := range scheduled {
+		scheduledObjs[i] = binding
+	}
+	boundObjs := make([]placementv1beta1.BindingObj, len(bound))
+	for i, binding := range bound {
+		boundObjs[i] = binding
+	}
+
+	scheduleOrBoundBindingsMap := prepareScheduledOrBoundBindingsMap(scheduledObjs, boundObjs)
 	if diff := cmp.Diff(scheduleOrBoundBindingsMap, want); diff != "" {
 		t.Errorf("preparedScheduledOrBoundBindingsMap() scheduledOrBoundBindingsMap diff (-got, +want): %s", diff)
 	}
@@ -157,7 +178,13 @@ func TestPrepareObsoleteBindingsMap(t *testing.T) {
 		altClusterName: true,
 	}
 
-	obsoleteBindingsMap := prepareObsoleteBindingsMap(obsolete)
+	// Convert concrete bindings to interface slice
+	obsoleteObjs := make([]placementv1beta1.BindingObj, len(obsolete))
+	for i, binding := range obsolete {
+		obsoleteObjs[i] = binding
+	}
+
+	obsoleteBindingsMap := prepareObsoleteBindingsMap(obsoleteObjs)
 	if diff := cmp.Diff(obsoleteBindingsMap, want); diff != "" {
 		t.Errorf("prepareObsoleteBindingsMap() obsoleteBindingsMap diff (-got, +want): %s", diff)
 	}

pkg/scheduler/framework/cyclestate_test.go

@@ -23,12 +23,12 @@ import (
 // prepareScheduledOrBoundBindingsMap returns a map that allows quick lookup of whether a cluster
 // already has a binding of the scheduled or bound state relevant to the current scheduling
 // cycle.
-func prepareScheduledOrBoundBindingsMap(scheduledOrBoundBindings ...[]*fleetv1beta1.ClusterResourceBinding) map[string]bool {
+func prepareScheduledOrBoundBindingsMap(scheduledOrBoundBindings ...[]fleetv1beta1.BindingObj) map[string]bool {
 	bm := make(map[string]bool)
 
 	for _, bindingSet := range scheduledOrBoundBindings {
 		for _, binding := range bindingSet {
-			bm[binding.Spec.TargetCluster] = true
+			bm[binding.GetBindingSpec().TargetCluster] = true
 		}
 	}
 
@@ -37,11 +37,11 @@ func prepareScheduledOrBoundBindingsMap(scheduledOrBoundBindings ...[]*fleetv1be
 
 // prepareObsoleteBindingsMap returns a map that allows quick lookup of whether a cluster
 // already has an obsolete binding relevant to the current scheduling cycle.
-func prepareObsoleteBindingsMap(obsoleteBindings []*fleetv1beta1.ClusterResourceBinding) map[string]bool {
+func prepareObsoleteBindingsMap(obsoleteBindings []fleetv1beta1.BindingObj) map[string]bool {
 	bm := make(map[string]bool)
 
 	for _, binding := range obsoleteBindings {
-		bm[binding.Spec.TargetCluster] = true
+		bm[binding.GetBindingSpec().TargetCluster] = true
 	}
 
 	return bm

pkg/scheduler/framework/cyclestateutils.go

@@ -357,25 +357,25 @@ func (f *framework) collectClusters(ctx context.Context) ([]clusterv1beta1.Membe
 }
 
 // collectBindings lists all bindings associated with a CRP **using the uncached client**.
-func (f *framework) collectBindings(ctx context.Context, crpName string) ([]placementv1beta1.ClusterResourceBinding, error) {
+func (f *framework) collectBindings(ctx context.Context, crpName string) ([]placementv1beta1.BindingObj, error) {
 	bindingList := &placementv1beta1.ClusterResourceBindingList{}
 	labelSelector := labels.SelectorFromSet(labels.Set{placementv1beta1.CRPTrackingLabel: crpName})
 	// List bindings directly from the API server.
 	if err := f.uncachedReader.List(ctx, bindingList, &client.ListOptions{LabelSelector: labelSelector}); err != nil {
 		return nil, controller.NewAPIServerError(false, err)
 	}
-	return bindingList.Items, nil
+	return controller.ConvertCRBArrayToBindingObjs(controller.ConvertCRBItemsToPointers(bindingList.Items)), nil
 }
 
 // markAsUnscheduledForAndUpdate marks a binding as unscheduled and updates it.
-var markUnscheduledForAndUpdate = func(ctx context.Context, hubClient client.Client, binding *placementv1beta1.ClusterResourceBinding) error {
+var markUnscheduledForAndUpdate = func(ctx context.Context, hubClient client.Client, binding placementv1beta1.BindingObj) error {
 	// Remember the previous unscheduledBinding state so that we might be able to revert this change if this
 	// cluster is being selected again before the resources are removed from it. Need to do a get and set if
 	// we add more annotations to the binding.
-	binding.SetAnnotations(map[string]string{placementv1beta1.PreviousBindingStateAnnotation: string(binding.Spec.State)})
+	binding.SetAnnotations(map[string]string{placementv1beta1.PreviousBindingStateAnnotation: string(binding.GetBindingSpec().State)})
 	// Mark the unscheduledBinding as unscheduled which can conflict with the rollout controller which also changes the state of a
 	// unscheduledBinding from "scheduled" to "bound".
-	binding.Spec.State = placementv1beta1.BindingStateUnscheduled
+	binding.GetBindingSpec().State = placementv1beta1.BindingStateUnscheduled
 	err := hubClient.Update(ctx, binding, &client.UpdateOptions{})
 	if err == nil {
 		klog.V(2).InfoS("Marked binding as unscheduled", "clusterResourceBinding", klog.KObj(binding))
@@ -384,7 +384,7 @@ var markUnscheduledForAndUpdate = func(ctx context.Context, hubClient client.Cli
 }
 
 // removeFinalizerAndUpdate removes scheduler CRB cleanup finalizer from ClusterResourceBinding and updates it.
-var removeFinalizerAndUpdate = func(ctx context.Context, hubClient client.Client, binding *placementv1beta1.ClusterResourceBinding) error {
+var removeFinalizerAndUpdate = func(ctx context.Context, hubClient client.Client, binding placementv1beta1.BindingObj) error {
 	controllerutil.RemoveFinalizer(binding, placementv1beta1.SchedulerCRBCleanupFinalizer)
 	err := hubClient.Update(ctx, binding, &client.UpdateOptions{})
 	if err == nil {
@@ -394,7 +394,7 @@ var removeFinalizerAndUpdate = func(ctx context.Context, hubClient client.Client
 }
 
 // updateBindings iterates over bindings and updates them using the update function provided.
-func (f *framework) updateBindings(ctx context.Context, bindings []*placementv1beta1.ClusterResourceBinding, updateFn func(ctx context.Context, client client.Client, binding *placementv1beta1.ClusterResourceBinding) error) error {
+func (f *framework) updateBindings(ctx context.Context, bindings []placementv1beta1.BindingObj, updateFn func(ctx context.Context, client client.Client, binding placementv1beta1.BindingObj) error) error {
 	// issue all the update requests in parallel
 	errs, cctx := errgroup.WithContext(ctx)
 	for _, binding := range bindings {
@@ -428,7 +428,7 @@ func (f *framework) runSchedulingCycleForPickAllPlacementType(
 	crpName string,
 	policy placementv1beta1.PolicySnapshotObj,
 	clusters []clusterv1beta1.MemberCluster,
-	bound, scheduled, unscheduled, obsolete []*placementv1beta1.ClusterResourceBinding,
+	bound, scheduled, unscheduled, obsolete []placementv1beta1.BindingObj,
 ) (result ctrl.Result, err error) {
 	policyRef := klog.KObj(policy)
 
@@ -490,8 +490,11 @@ func (f *framework) runSchedulingCycleForPickAllPlacementType(
 
 	// With the PickAll placement type, the desired number of clusters to select always matches
 	// with the count of scheduled + bound bindings.
+	// Extract updated bindings from patch structs
+	patchedBindings := controller.ConvertCRBArrayToBindingObjs(patched)
+
 	numOfClusters := len(toCreate) + len(patched) + len(scheduled) + len(bound)
-	if err := f.updatePolicySnapshotStatusFromBindings(ctx, policy, numOfClusters, nil, filtered, toCreate, patched, scheduled, bound); err != nil {
+	if err := f.updatePolicySnapshotStatusFromBindings(ctx, policy, numOfClusters, nil, filtered, toCreate, patchedBindings, scheduled, bound); err != nil {
 		klog.ErrorS(err, "Failed to update latest scheduling decisions and condition", "clusterSchedulingPolicySnapshot", policyRef)
 		return ctrl.Result{}, err
 	}
@@ -666,7 +669,7 @@ func (f *framework) runFilterPlugins(ctx context.Context, state *CycleState, pol
 func (f *framework) manipulateBindings(
 	ctx context.Context,
 	policy placementv1beta1.PolicySnapshotObj,
-	toCreate, toDelete []*placementv1beta1.ClusterResourceBinding,
+	toCreate, toDelete []placementv1beta1.BindingObj,
 	toPatch []*bindingWithPatch,
 ) error {
 	policyRef := klog.KObj(policy)
@@ -703,7 +706,7 @@ func (f *framework) manipulateBindings(
 }
 
 // createBindings creates a list of new bindings.
-func (f *framework) createBindings(ctx context.Context, toCreate []*placementv1beta1.ClusterResourceBinding) error {
+func (f *framework) createBindings(ctx context.Context, toCreate []placementv1beta1.BindingObj) error {
 	// issue all the create requests in parallel
 	errs, cctx := errgroup.WithContext(ctx)
 	for _, binding := range toCreate {
@@ -720,7 +723,7 @@ func (f *framework) createBindings(ctx context.Context, toCreate []*placementv1b
 							// The binding already exists, which is fine.
 							return nil
 						}
-						klog.ErrorS(err, "Failed to create a new binding", "clusterResourceBinding", klog.KObj(newBinding))
+						klog.ErrorS(err, "Failed to create a new binding", "binding", klog.KObj(newBinding))
 					}
 					return err
 				})
@@ -762,7 +765,7 @@ func (f *framework) updatePolicySnapshotStatusFromBindings(
 	numOfClusters int,
 	notPicked ScoredClusters,
 	filtered []*filteredClusterWithStatus,
-	existing ...[]*placementv1beta1.ClusterResourceBinding,
+	existing ...[]placementv1beta1.BindingObj,
 ) error {
 	policyRef := klog.KObj(policy)
 
@@ -811,7 +814,7 @@ func (f *framework) runSchedulingCycleForPickNPlacementType(
 	crpName string,
 	policy placementv1beta1.PolicySnapshotObj,
 	clusters []clusterv1beta1.MemberCluster,
-	bound, scheduled, unscheduled, obsolete []*placementv1beta1.ClusterResourceBinding,
+	bound, scheduled, unscheduled, obsolete []placementv1beta1.BindingObj,
 ) (result ctrl.Result, err error) {
 	policyRef := klog.KObj(policy)
 
@@ -976,9 +979,15 @@ func (f *framework) runSchedulingCycleForPickNPlacementType(
 		patched = append(patched, p.updated)
 	}
 
+	// Extract updated bindings from patch structs for status update
+	patchedForStatus := make([]placementv1beta1.BindingObj, len(toPatch))
+	for i, p := range toPatch {
+		patchedForStatus[i] = p.updated
+	}
+
 	// Update policy snapshot status with the latest scheduling decisions and condition.
 	klog.V(2).InfoS("Updating policy snapshot status", "clusterSchedulingPolicySnapshot", policyRef)
-	if err := f.updatePolicySnapshotStatusFromBindings(ctx, policy, numOfClusters, notPicked, filtered, toCreate, patched, scheduled, bound); err != nil {
+	if err := f.updatePolicySnapshotStatusFromBindings(ctx, policy, numOfClusters, notPicked, filtered, toCreate, patchedForStatus, scheduled, bound); err != nil {
 		klog.ErrorS(err, "Failed to update latest scheduling decisions and condition", "clusterSchedulingPolicySnapshot", policyRef)
 		return ctrl.Result{}, err
 	}
@@ -992,7 +1001,7 @@ func (f *framework) runSchedulingCycleForPickNPlacementType(
 // To minimize interruptions, the scheduler picks scheduled bindings first (in any order); if there
 // are still more bindings to trim, the scheduler will move onto bound bindings, and it prefers
 // ones with a lower cluster score and a smaller name (in alphabetical order) .
-func (f *framework) downscale(ctx context.Context, scheduled, bound []*placementv1beta1.ClusterResourceBinding, count int) (updatedScheduled, updatedBound []*placementv1beta1.ClusterResourceBinding, err error) {
+func (f *framework) downscale(ctx context.Context, scheduled, bound []placementv1beta1.BindingObj, count int) (updatedScheduled, updatedBound []placementv1beta1.BindingObj, err error) {
 	if count == 0 {
 		// Skip if the downscale count is zero.
 		return scheduled, bound, nil
@@ -1020,18 +1029,15 @@ func (f *framework) downscale(ctx context.Context, scheduled, bound []*placement
 		sortedScheduled := sortByClusterScoreAndName(scheduled)
 
 		// Trim scheduled bindings.
-		bindingsToDelete := make([]*placementv1beta1.ClusterResourceBinding, 0, count)
-		for i := 0; i < len(sortedScheduled) && i < count; i++ {
-			bindingsToDelete = append(bindingsToDelete, sortedScheduled[i])
-		}
+		bindingsToDelete := sortedScheduled[:count]
 
 		return sortedScheduled[count:], bound, f.updateBindings(ctx, bindingsToDelete, markUnscheduledForAndUpdate)
 	case count == len(scheduled):
 		// Trim all scheduled bindings.
 		return nil, bound, f.updateBindings(ctx, scheduled, markUnscheduledForAndUpdate)
 	case count < len(scheduled)+len(bound):
 		// Trim all scheduled bindings and part of bound bindings.
-		bindingsToDelete := make([]*placementv1beta1.ClusterResourceBinding, 0, count)
+		bindingsToDelete := make([]placementv1beta1.BindingObj, 0, count)
 		bindingsToDelete = append(bindingsToDelete, scheduled...)
 
 		left := count - len(bindingsToDelete)
@@ -1045,14 +1051,12 @@ func (f *framework) downscale(ctx context.Context, scheduled, bound []*placement
 		// Note that this is at best an approximation, as the cluster score assigned earlier might
 		// no longer apply, due to the ever-changing state in the fleet.
 		sortedBound := sortByClusterScoreAndName(bound)
-		for i := 0; i < left && i < len(sortedBound); i++ {
-			bindingsToDelete = append(bindingsToDelete, sortedBound[i])
-		}
+		bindingsToDelete = append(bindingsToDelete, sortedBound[:left]...)
 
 		return nil, sortedBound[left:], f.updateBindings(ctx, bindingsToDelete, markUnscheduledForAndUpdate)
 	case count == len(scheduled)+len(bound):
 		// Trim all scheduled and bound bindings.
-		bindingsToDelete := make([]*placementv1beta1.ClusterResourceBinding, 0, count)
+		bindingsToDelete := make([]placementv1beta1.BindingObj, 0, count)
 		bindingsToDelete = append(bindingsToDelete, scheduled...)
 		bindingsToDelete = append(bindingsToDelete, bound...)
 		return nil, nil, f.updateBindings(ctx, bindingsToDelete, markUnscheduledForAndUpdate)
@@ -1404,7 +1408,7 @@ func (f *framework) runSchedulingCycleForPickFixedPlacementType(
 	crpName string,
 	policy placementv1beta1.PolicySnapshotObj,
 	clusters []clusterv1beta1.MemberCluster,
-	bound, scheduled, unscheduled, obsolete []*placementv1beta1.ClusterResourceBinding,
+	bound, scheduled, unscheduled, obsolete []placementv1beta1.BindingObj,
 ) (ctrl.Result, error) {
 	policyRef := klog.KObj(policy)