temp

Author: ryanzhang-oss

.github/.copilot/breadcrumbs/2025-06-24-0900-rollout-controller-binding-interface-refactor.md

@@ -20,6 +20,8 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kubefleet-dev/kubefleet/apis"
 )
 
 const (
@@ -52,7 +54,7 @@ type BindingStatusGetterSetter interface {
 // A BindingObj offers an abstract way to work with fleet binding objects.
 // +kubebuilder:object:generate=false
 type BindingObj interface {
-	client.Object
+	apis.ConditionedObj
 	BindingSpecGetterSetter
 	BindingStatusGetterSetter
 }

apis/placement/v1beta1/binding_types.go

@@ -22,6 +22,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kubefleet-dev/kubefleet/apis"
 )
 
 const (
@@ -58,7 +60,7 @@ type PlacementStatusGetterSetter interface {
 // PlacementObj offers the functionality to work with fleet placement object.
 // +kubebuilder:object:generate=false
 type PlacementObj interface {
-	client.Object
+	apis.ConditionedObj
 	PlacementSpecGetterSetter
 	PlacementStatusGetterSetter
 }

apis/placement/v1beta1/clusterresourceplacement_types.go

@@ -20,6 +20,8 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kubefleet-dev/kubefleet/apis"
 )
 
 const (
@@ -57,7 +59,7 @@ type PolicySnapshotStatusGetterSetter interface {
 // A PolicySnapshotObj offers an abstract way to work with a fleet policy snapshot object.
 // +kubebuilder:object:generate=false
 type PolicySnapshotObj interface {
-	client.Object
+	apis.ConditionedObj
 	PolicySnapshotSpecGetterSetter
 	PolicySnapshotStatusGetterSetter
 }

apis/placement/v1beta1/policysnapshot_types.go

@@ -21,6 +21,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kubefleet-dev/kubefleet/apis"
 )
 
 const (
@@ -74,7 +76,7 @@ type ResourceSnapshotStatusGetterSetter interface {
 // A ResourceSnapshotObj offers an abstract way to work with a resource snapshot object.
 // +kubebuilder:object:generate=false
 type ResourceSnapshotObj interface {
-	client.Object
+	apis.ConditionedObj
 	ResourceSnapshotSpecGetterSetter
 	ResourceSnapshotStatusGetterSetter
 }

apis/placement/v1beta1/resourcesnapshot_types.go

@@ -118,7 +118,7 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1
 	}
 
 	// set default values for CRP.
-	defaulter.SetDefaultsClusterResourcePlacement(&crp)
+	defaulter.SetPlacementDefaults(&crp)
 
 	if crp.DeletionTimestamp != nil {
 		klog.V(2).InfoS(condition.EvictionInvalidDeletingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)

pkg/controllers/clusterresourceplacementeviction/controller.go

@@ -258,7 +258,7 @@ func TestValidateEviction(t *testing.T) {
 			// Since default values are applied to the affected CRP in the eviction controller; the
 			// the same must be done on the expected result as well.
 			if tc.wantValidationResult.crp != nil {
-				defaulter.SetDefaultsClusterResourcePlacement(tc.wantValidationResult.crp)
+				defaulter.SetPlacementDefaults(tc.wantValidationResult.crp)
 			}
 
 			if diff := cmp.Diff(tc.wantValidationResult, gotValidationResult, validationResultCmpOptions...); diff != "" {

pkg/controllers/clusterresourceplacementeviction/controller_test.go

@@ -264,7 +264,7 @@ func TestWaitForResourcesToCleanUp(t *testing.T) {
 	for name, tt := range tests {
 		crp := &fleetv1beta1.ClusterResourcePlacement{}
 		t.Run(name, func(t *testing.T) {
-			gotWait, err := waitForResourcesToCleanUp(tt.allBindings, crp)
+			gotWait, err := waitForResourcesToCleanUp(controller.ConvertCRBArrayToBindingObjs(tt.allBindings), crp)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("waitForResourcesToCleanUp test `%s` error = %v, wantErr %v", name, err, tt.wantErr)
 				return
@@ -618,7 +618,7 @@ func TestUpdateBindings(t *testing.T) {
 				}
 				if tt.desiredBindingsSpec != nil {
 					inputs[i].desiredBinding = tt.bindings[i].DeepCopy()
-					inputs[i].desiredBinding.Spec = tt.desiredBindingsSpec[i]
+					inputs[i].desiredBinding.SetBindingSpec(tt.desiredBindingsSpec[i])
 				}
 			}
 			err := r.updateBindings(ctx, inputs)
@@ -2172,7 +2172,7 @@ func TestPickBindingsToRoll(t *testing.T) {
 					Name: tt.latestResourceSnapshotName,
 				},
 			}
-			gotUpdatedBindings, gotStaleUnselectedBindings, gotUpToDateBoundBindings, gotNeedRoll, gotWaitTime, err := r.pickBindingsToRoll(context.Background(), tt.allBindings, resourceSnapshot, tt.crp, tt.matchedCROs, tt.matchedROs)
+			gotUpdatedBindings, gotStaleUnselectedBindings, gotUpToDateBoundBindings, gotNeedRoll, gotWaitTime, err := r.pickBindingsToRoll(context.Background(), controller.ConvertCRBArrayToBindingObjs(tt.allBindings), resourceSnapshot, tt.crp, tt.matchedCROs, tt.matchedROs)
 			if (err != nil) != (tt.wantErr != nil) || err != nil && !errors.Is(err, tt.wantErr) {
 				t.Fatalf("pickBindingsToRoll() error = %v, wantErr %v", err, tt.wantErr)
 			}
@@ -2183,21 +2183,23 @@ func TestPickBindingsToRoll(t *testing.T) {
 			wantTobeUpdatedBindings := make([]toBeUpdatedBinding, len(tt.wantTobeUpdatedBindings))
 			for i, index := range tt.wantTobeUpdatedBindings {
 				// Unscheduled bindings are only removed in a single rollout cycle.
-				if tt.allBindings[index].Spec.State != fleetv1beta1.BindingStateUnscheduled {
+				bindingSpec := tt.allBindings[index].GetBindingSpec()
+				if bindingSpec.State != fleetv1beta1.BindingStateUnscheduled {
 					wantTobeUpdatedBindings[i].currentBinding = tt.allBindings[index]
 					wantTobeUpdatedBindings[i].desiredBinding = tt.allBindings[index].DeepCopy()
-					wantTobeUpdatedBindings[i].desiredBinding.Spec = tt.wantDesiredBindingsSpec[index]
+					wantTobeUpdatedBindings[i].desiredBinding.SetBindingSpec(tt.wantDesiredBindingsSpec[index])
 				} else {
 					wantTobeUpdatedBindings[i].currentBinding = tt.allBindings[index]
 				}
 			}
 			wantStaleUnselectedBindings := make([]toBeUpdatedBinding, len(tt.wantStaleUnselectedBindings))
 			for i, index := range tt.wantStaleUnselectedBindings {
 				// Unscheduled bindings are only removed in a single rollout cycle.
-				if tt.allBindings[index].Spec.State != fleetv1beta1.BindingStateUnscheduled {
+				bindingSpec := tt.allBindings[index].GetBindingSpec()
+				if bindingSpec.State != fleetv1beta1.BindingStateUnscheduled {
 					wantStaleUnselectedBindings[i].currentBinding = tt.allBindings[index]
 					wantStaleUnselectedBindings[i].desiredBinding = tt.allBindings[index].DeepCopy()
-					wantStaleUnselectedBindings[i].desiredBinding.Spec = tt.wantDesiredBindingsSpec[index]
+					wantStaleUnselectedBindings[i].desiredBinding.SetBindingSpec(tt.wantDesiredBindingsSpec[index])
 				} else {
 					wantStaleUnselectedBindings[i].currentBinding = tt.allBindings[index]
 				}
@@ -2892,7 +2894,7 @@ func TestCheckAndUpdateStaleBindingsStatus(t *testing.T) {
 				Client: fakeClient,
 			}
 			ctx := context.Background()
-			if err := r.checkAndUpdateStaleBindingsStatus(ctx, tt.bindings); err != nil {
+			if err := r.checkAndUpdateStaleBindingsStatus(ctx, controller.ConvertCRBArrayToBindingObjs(tt.bindings)); err != nil {
 				t.Fatalf("checkAndUpdateStaleBindingsStatus() got error %v, want no err", err)
 			}
 			bindingList := &fleetv1beta1.ClusterResourceBindingList{}
@@ -3168,7 +3170,7 @@ func TestProcessApplyStrategyUpdates(t *testing.T) {
 				Client: fakeClient,
 			}
 
-			applyStrategyUpdated, err := r.processApplyStrategyUpdates(ctx, tc.crp, tc.allBindings)
+			applyStrategyUpdated, err := r.processApplyStrategyUpdates(ctx, tc.crp, controller.ConvertCRBArrayToBindingObjs(tc.allBindings))
 			if err != nil {
 				t.Errorf("processApplyStrategyUpdates() error = %v, want no error", err)
 			}

pkg/controllers/rollout/controller.go

@@ -432,7 +432,7 @@ func (r *Reconciler) recordOverrideSnapshots(ctx context.Context, placementName
 		// no more retries here.
 		return fmt.Errorf("%w: %s", errInitializedFailed, err.Error())
 	}
-
+	// TODO: use the lib to fetch the master resource snapshot using interface instead of concrete type
 	var masterResourceSnapshot *placementv1beta1.ClusterResourceSnapshot
 	labelMatcher := client.MatchingLabels{
 		placementv1beta1.CRPTrackingLabel:   placementName,