Skip to content

Commit d83c930

Browse files
feat: use bindingObj interface instead of ClusterResourceBinding in the scheduler (#111)
1 parent dcd82cb commit d83c930

26 files changed

+2200
-443
lines changed

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

Lines changed: 537 additions & 0 deletions
Large diffs are not rendered by default.

.github/.copilot/breadcrumbs/2025-06-19-0800-scheduler-patch-functions-unified-refactor.md

Lines changed: 486 additions & 0 deletions
Large diffs are not rendered by default.

pkg/controllers/clusterresourceplacement/resource_selector.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,10 @@ func (r *Reconciler) selectResourcesForPlacement(placement *fleetv1beta1.Cluster
525525
return 0, nil, nil, err
526526
}
527527
uGVK := unstructuredObj.GetObjectKind().GroupVersionKind().GroupKind()
528-
switch {
529-
case uGVK == utils.ClusterResourceEnvelopeGK:
528+
switch uGVK {
529+
case utils.ClusterResourceEnvelopeGK:
530530
envelopeObjCount++
531-
case uGVK == utils.ResourceEnvelopeGK:
531+
case utils.ResourceEnvelopeGK:
532532
envelopeObjCount++
533533
}
534534
resources[i] = *rc

pkg/controllers/rollout/controller.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -606,14 +606,14 @@ func (r *Reconciler) calculateRealTarget(crp *fleetv1beta1.ClusterResourcePlacem
606606
targetNumber := 0
607607

608608
// note that if the policy will be overwritten if it is nil in this controller.
609-
switch {
610-
case crp.Spec.Policy.PlacementType == fleetv1beta1.PickAllPlacementType:
609+
switch crp.Spec.Policy.PlacementType {
610+
case fleetv1beta1.PickAllPlacementType:
611611
// we use the scheduler picked bindings as the target number since there is no target in the CRP
612612
targetNumber = len(schedulerTargetedBinds)
613-
case crp.Spec.Policy.PlacementType == fleetv1beta1.PickFixedPlacementType:
613+
case fleetv1beta1.PickFixedPlacementType:
614614
// we use the length of the given cluster names are targets
615615
targetNumber = len(crp.Spec.Policy.ClusterNames)
616-
case crp.Spec.Policy.PlacementType == fleetv1beta1.PickNPlacementType:
616+
case fleetv1beta1.PickNPlacementType:
617617
// we use the given number as the target
618618
targetNumber = int(*crp.Spec.Policy.NumberOfClusters)
619619
default:

pkg/controllers/rollout/controller_integration_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,10 @@ var _ = Describe("Test the rollout Controller", func() {
125125

126126
// Prepare bindings of various states.
127127
var binding *fleetv1beta1.ClusterResourceBinding
128-
switch {
129-
case i%3 == 0:
128+
switch i % 3 {
129+
case 0:
130130
binding = generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, resourceSnapshot.Name, clusters[i])
131-
case i%3 == 1:
131+
case 1:
132132
binding = generateClusterResourceBinding(fleetv1beta1.BindingStateBound, resourceSnapshot.Name, clusters[i])
133133
default:
134134
binding = generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, resourceSnapshot.Name, clusters[i])

pkg/controllers/workgenerator/controller.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,8 @@ func (r *Reconciler) processOneSelectedResource(
588588
}
589589

590590
uGVK := uResource.GetObjectKind().GroupVersionKind().GroupKind()
591-
switch {
592-
case uGVK == utils.ClusterResourceEnvelopeGK:
591+
switch uGVK {
592+
case utils.ClusterResourceEnvelopeGK:
593593
// The resource is a ClusterResourceEnvelope; extract its contents.
594594
var clusterResourceEnvelope fleetv1beta1.ClusterResourceEnvelope
595595
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uResource.Object, &clusterResourceEnvelope); err != nil {
@@ -609,7 +609,7 @@ func (r *Reconciler) processOneSelectedResource(
609609
}
610610
activeWork[work.Name] = work
611611
newWork = append(newWork, work)
612-
case uGVK == utils.ResourceEnvelopeGK:
612+
case utils.ResourceEnvelopeGK:
613613
// The resource is a ResourceEnvelope; extract its contents.
614614
var resourceEnvelope fleetv1beta1.ResourceEnvelope
615615
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uResource.Object, &resourceEnvelope); err != nil {

pkg/scheduler/framework/cyclestate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func (c *CycleState) HasObsoleteBindingFor(clusterName string) bool {
140140
// IsClusterObsolete
141141

142142
// NewCycleState creates a CycleState.
143-
func NewCycleState(clusters []clusterv1beta1.MemberCluster, obsoleteBindings []*placementv1beta1.ClusterResourceBinding, scheduledOrBoundBindings ...[]*placementv1beta1.ClusterResourceBinding) *CycleState {
143+
func NewCycleState(clusters []clusterv1beta1.MemberCluster, obsoleteBindings []placementv1beta1.BindingObj, scheduledOrBoundBindings ...[]placementv1beta1.BindingObj) *CycleState {
144144
return &CycleState{
145145
store: sync.Map{},
146146
clusters: clusters,

pkg/scheduler/framework/cyclestate_test.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1"
2626
placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
27+
"github.com/kubefleet-dev/kubefleet/pkg/utils/controller"
2728
)
2829

2930
// TestCycleStateBasicOps tests the basic ops of a CycleState.
@@ -67,7 +68,9 @@ func TestCycleStateBasicOps(t *testing.T) {
6768
},
6869
}
6970

70-
cs := NewCycleState(clusters, obsoleteBindings, scheduledOrBoundBindings)
71+
// Convert concrete bindings to interface slices
72+
73+
cs := NewCycleState(clusters, controller.ConvertCRBArrayToBindingObjs(obsoleteBindings), controller.ConvertCRBArrayToBindingObjs(scheduledOrBoundBindings))
7174

7275
k, v := "key", "value"
7376
cs.Write(StateKey(k), StateValue(v))
@@ -125,7 +128,8 @@ func TestPrepareScheduledOrBoundBindingsMap(t *testing.T) {
125128
altClusterName: true,
126129
}
127130

128-
scheduleOrBoundBindingsMap := prepareScheduledOrBoundBindingsMap(scheduled, bound)
131+
// Convert concrete bindings to interface slices
132+
scheduleOrBoundBindingsMap := prepareScheduledOrBoundBindingsMap(controller.ConvertCRBArrayToBindingObjs(scheduled), controller.ConvertCRBArrayToBindingObjs(bound))
129133
if diff := cmp.Diff(scheduleOrBoundBindingsMap, want); diff != "" {
130134
t.Errorf("preparedScheduledOrBoundBindingsMap() scheduledOrBoundBindingsMap diff (-got, +want): %s", diff)
131135
}
@@ -157,7 +161,8 @@ func TestPrepareObsoleteBindingsMap(t *testing.T) {
157161
altClusterName: true,
158162
}
159163

160-
obsoleteBindingsMap := prepareObsoleteBindingsMap(obsolete)
164+
// Convert concrete bindings to interface slice
165+
obsoleteBindingsMap := prepareObsoleteBindingsMap(controller.ConvertCRBArrayToBindingObjs(obsolete))
161166
if diff := cmp.Diff(obsoleteBindingsMap, want); diff != "" {
162167
t.Errorf("prepareObsoleteBindingsMap() obsoleteBindingsMap diff (-got, +want): %s", diff)
163168
}

pkg/scheduler/framework/cyclestateutils.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ import (
2323
// prepareScheduledOrBoundBindingsMap returns a map that allows quick lookup of whether a cluster
2424
// already has a binding of the scheduled or bound state relevant to the current scheduling
2525
// cycle.
26-
func prepareScheduledOrBoundBindingsMap(scheduledOrBoundBindings ...[]*fleetv1beta1.ClusterResourceBinding) map[string]bool {
26+
func prepareScheduledOrBoundBindingsMap(scheduledOrBoundBindings ...[]fleetv1beta1.BindingObj) map[string]bool {
2727
bm := make(map[string]bool)
2828

2929
for _, bindingSet := range scheduledOrBoundBindings {
3030
for _, binding := range bindingSet {
31-
bm[binding.Spec.TargetCluster] = true
31+
bm[binding.GetBindingSpec().TargetCluster] = true
3232
}
3333
}
3434

@@ -37,11 +37,11 @@ func prepareScheduledOrBoundBindingsMap(scheduledOrBoundBindings ...[]*fleetv1be
3737

3838
// prepareObsoleteBindingsMap returns a map that allows quick lookup of whether a cluster
3939
// already has an obsolete binding relevant to the current scheduling cycle.
40-
func prepareObsoleteBindingsMap(obsoleteBindings []*fleetv1beta1.ClusterResourceBinding) map[string]bool {
40+
func prepareObsoleteBindingsMap(obsoleteBindings []fleetv1beta1.BindingObj) map[string]bool {
4141
bm := make(map[string]bool)
4242

4343
for _, binding := range obsoleteBindings {
44-
bm[binding.Spec.TargetCluster] = true
44+
bm[binding.GetBindingSpec().TargetCluster] = true
4545
}
4646

4747
return bm

0 commit comments

Comments
 (0)