test

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

Author: ryanzhang-oss

pkg/controllers/rollout/controller.go

@@ -65,7 +65,7 @@ type Reconciler struct {
 // Reconcile triggers a single binding reconcile round.
 func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtime.Result, error) {
 	startTime := time.Now()
-	placementKey := controller.GetPlacementKeyFromRequest(req)
+	placementKey := controller.GetObjectKeyFromRequest(req)
 	klog.V(2).InfoS("Start to rollout the bindings", "placementKey", placementKey)
 
 	// add latency log
@@ -147,7 +147,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 	}
 
 	// find the master resourceSnapshot.
-	masterResourceSnapshot, err := controller.FetchMasterResourceSnapshot(ctx, r.UncachedReader, string(placementKey))
+	masterResourceSnapshot, err := controller.FetchLatestMasterResourceSnapshot(ctx, r.UncachedReader, string(placementKey))
 	if err != nil {
 		klog.ErrorS(err, "Failed to find the masterResourceSnapshot for the placement",
 			"placement", placementObjRef)

pkg/controllers/workgenerator/controller.go

@@ -95,7 +95,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req controllerruntime.Reques
 	}()
 
 	// Get the binding using the utility function
-	placementKey := controller.GetPlacementKeyFromRequest(req)
+	placementKey := controller.GetObjectKeyFromRequest(req)
 	resourceBinding, err := controller.FetchBindingFromKey(ctx, r.Client, placementKey)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
@@ -280,7 +280,7 @@ func (r *Reconciler) updateBindingStatusWithRetry(ctx context.Context, resourceB
 		klog.ErrorS(err, "Failed to update the binding status, will retry", "binding", bindingRef, "bindingStatus", resourceBinding.GetBindingStatus())
 		errAfterRetries := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 			// Get the latest binding object using the utility function
-			placementKey := controller.GetPlacementKeyFromObj(resourceBinding)
+			placementKey := controller.GetObjectKeyFromObj(resourceBinding)
 			latestBinding, err := controller.FetchBindingFromKey(ctx, r.Client, placementKey)
 			if err != nil {
 				return err

pkg/controllers/workgenerator/envelope.go

@@ -39,36 +39,36 @@ func (r *Reconciler) createOrUpdateEnvelopeCRWorkObj(
 	ctx context.Context,
 	envelopeReader fleetv1beta1.EnvelopeReader,
 	workNamePrefix string,
-	resourceBinding fleetv1beta1.BindingObj,
+	binding fleetv1beta1.BindingObj,
 	resourceSnapshot fleetv1beta1.ResourceSnapshotObj,
 	resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash string,
 ) (*fleetv1beta1.Work, error) {
 	manifests, err := extractManifestsFromEnvelopeCR(envelopeReader)
 	if err != nil {
 		klog.ErrorS(err, "Failed to extract manifests from the envelope spec",
-			"resourceBinding", klog.KObj(resourceBinding),
+			"resourceBinding", klog.KObj(binding),
 			"resourceSnapshot", klog.KObj(resourceSnapshot),
 			"envelope", envelopeReader.GetEnvelopeObjRef())
 		return nil, err
 	}
 	klog.V(2).InfoS("Successfully extracted wrapped manifests from the envelope",
 		"numOfResources", len(manifests),
-		"resourceBinding", klog.KObj(resourceBinding),
+		"resourceBinding", klog.KObj(binding),
 		"resourceSnapshot", klog.KObj(resourceSnapshot),
 		"envelope", envelopeReader.GetEnvelopeObjRef())
 
 	// Check to see if a corresponding work object has been created for the envelope.
 	labelMatcher := client.MatchingLabels{
-		fleetv1beta1.ParentBindingLabel:     resourceBinding.GetName(),
-		fleetv1beta1.CRPTrackingLabel:       resourceBinding.GetLabels()[fleetv1beta1.CRPTrackingLabel],
+		fleetv1beta1.ParentBindingLabel:     binding.GetName(),
+		fleetv1beta1.CRPTrackingLabel:       binding.GetLabels()[fleetv1beta1.CRPTrackingLabel],
 		fleetv1beta1.EnvelopeTypeLabel:      envelopeReader.GetEnvelopeType(),
 		fleetv1beta1.EnvelopeNameLabel:      envelopeReader.GetName(),
 		fleetv1beta1.EnvelopeNamespaceLabel: envelopeReader.GetNamespace(),
 	}
 	workList := &fleetv1beta1.WorkList{}
 	if err = r.Client.List(ctx, workList, labelMatcher); err != nil {
 		klog.ErrorS(err, "Failed to list work objects when finding the work object for an envelope",
-			"resourceBinding", klog.KObj(resourceBinding),
+			"resourceBinding", klog.KObj(binding),
 			"resourceSnapshot", klog.KObj(resourceSnapshot),
 			"envelope", envelopeReader.GetEnvelopeObjRef())
 		wrappedErr := fmt.Errorf("failed to list work objects when finding the work object for an envelope %v: %w", envelopeReader.GetEnvelopeObjRef(), err)
@@ -81,25 +81,25 @@ func (r *Reconciler) createOrUpdateEnvelopeCRWorkObj(
 		// Multiple matching work objects found; this should never occur under normal conditions.
 		wrappedErr := fmt.Errorf("%d work objects found for the same envelope %v, only one expected", len(workList.Items), envelopeReader.GetEnvelopeObjRef())
 		klog.ErrorS(wrappedErr, "Failed to create or update work object for envelope",
-			"resourceBinding", klog.KObj(resourceBinding),
+			"resourceBinding", klog.KObj(binding),
 			"resourceSnapshot", klog.KObj(resourceSnapshot),
 			"envelope", envelopeReader.GetEnvelopeObjRef())
 		return nil, controller.NewUnexpectedBehaviorError(wrappedErr)
 	case len(workList.Items) == 1:
 		klog.V(2).InfoS("Found existing work object for the envelope; updating it",
 			"work", klog.KObj(&workList.Items[0]),
-			"resourceBinding", klog.KObj(resourceBinding),
+			"resourceBinding", klog.KObj(binding),
 			"resourceSnapshot", klog.KObj(resourceSnapshot),
 			"envelope", envelopeReader.GetEnvelopeObjRef())
 		work = &workList.Items[0]
-		refreshWorkForEnvelopeCR(work, resourceBinding, resourceSnapshot, manifests, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash)
+		refreshWorkForEnvelopeCR(work, binding, resourceSnapshot, manifests, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash)
 	case len(workList.Items) == 0:
 		// No matching work object found; create a new one.
 		klog.V(2).InfoS("No existing work object found for the envelope; creating a new one",
-			"resourceBinding", klog.KObj(resourceBinding),
+			"resourceBinding", klog.KObj(binding),
 			"resourceSnapshot", klog.KObj(resourceSnapshot),
 			"envelope", envelopeReader.GetEnvelopeObjRef())
-		work = buildNewWorkForEnvelopeCR(workNamePrefix, resourceBinding, resourceSnapshot, envelopeReader, manifests, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash)
+		work = buildNewWorkForEnvelopeCR(workNamePrefix, binding, resourceSnapshot, envelopeReader, manifests, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash)
 	}
 
 	return work, nil

pkg/controllers/workgenerator/override.go

@@ -36,6 +36,7 @@ import (
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/overrider"
 )
 
+// TODO: combine the following two functions into one, as they are very similar.
 func (r *Reconciler) fetchClusterResourceOverrideSnapshots(ctx context.Context, resourceBinding placementv1beta1.BindingObj) (map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot, error) {
 	croMap := make(map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot)
 
@@ -49,7 +50,7 @@ func (r *Reconciler) fetchClusterResourceOverrideSnapshots(ctx context.Context,
 				// It could be caused by that the user updates the override too frequently and the snapshot has been replaced
 				// by the new one.
 				// TODO: support customized revision history limit
-				return nil, controller.NewUserError(fmt.Errorf("clusterResourceSnapshot %s is not found", name))
+				return nil, controller.NewUserError(fmt.Errorf("clusterResourceOverrideSnapshot %s is not found", name))
 			}
 			klog.ErrorS(err, "Failed to get the clusterResourceOverrideSnapshot",
 				"binding", klog.KObj(resourceBinding), "clusterResourceOverrideSnapshot", name)
@@ -83,7 +84,7 @@ func (r *Reconciler) fetchResourceOverrideSnapshots(ctx context.Context, resourc
 				// by the new one.
 				// TODO: support customized revision history limit
 				klog.ErrorS(err, "The resourceOverrideSnapshot is deleted", "binding", klog.KObj(resourceBinding), "resourceOverrideSnapshot", namespacedName)
-				return nil, controller.NewUserError(fmt.Errorf("resourceSnapshot %s is not found", namespacedName))
+				return nil, controller.NewUserError(fmt.Errorf("resourceOverrideSnapshot %s is not found", namespacedName))
 			}
 			klog.ErrorS(err, "Failed to get the resourceOverrideSnapshot",
 				"binding", klog.KObj(resourceBinding), "resourceOverrideSnapshot", namespacedName)

pkg/scheduler/queue/queue.go

@@ -29,6 +29,7 @@ import (
 // ClusterResourcePlacement's name in the form of "name".
 // If the Placement is a NamespaceResourcePlacement, the PlacementKey is the
 // NamespaceResourcePlacement's name in the form of "namespace/name".
+// TODO: rename this to be ObjectKey or something similar, as it is not specific to Placement.
 type PlacementKey string
 
 // PlacementSchedulingQueueWriter is an interface which allows sources, such as controllers, to add

pkg/scheduler/scheduler.go

@@ -210,7 +210,7 @@ func (s *Scheduler) scheduleOnce(ctx context.Context, worker int) {
 	// Note that the scheduler will enter this cycle as long as the placement is active and an active
 	// policy snapshot has been produced.
 	cycleStartTime := time.Now()
-	res, err := s.framework.RunSchedulingCycleFor(ctx, controller.GetPlacementKeyFromObj(placement), latestPolicySnapshot)
+	res, err := s.framework.RunSchedulingCycleFor(ctx, controller.GetObjectKeyFromObj(placement), latestPolicySnapshot)
 	if err != nil {
 		if errors.Is(err, controller.ErrUnexpectedBehavior) {
 			// The placement is in an unexpected state; this is a scheduler-side error, and
@@ -299,7 +299,7 @@ func (s *Scheduler) cleanUpAllBindingsFor(ctx context.Context, placement fleetv1
 	placementRef := klog.KObj(placement)
 
 	// Get the placement key which handles both cluster-scoped and namespaced placements
-	placementKey := controller.GetPlacementKeyFromObj(placement)
+	placementKey := controller.GetObjectKeyFromObj(placement)
 
 	// List all bindings derived from the placement.
 	//

pkg/utils/controller/binding_resolver.go

@@ -27,10 +27,10 @@ import (
 	"github.com/kubefleet-dev/kubefleet/pkg/scheduler/queue"
 )
 
-// FetchBindingFromKey resolves a PlacementKey to a concrete binding object that implements BindingObj.
-func FetchBindingFromKey(ctx context.Context, c client.Reader, placementKey queue.PlacementKey) (placementv1beta1.BindingObj, error) {
+// FetchBindingFromKey resolves a bindingKey to a concrete binding object that implements BindingObj.
+func FetchBindingFromKey(ctx context.Context, c client.Reader, bindingKey queue.PlacementKey) (placementv1beta1.BindingObj, error) {
 	// Extract namespace and name from the placement key
-	namespace, name, err := ExtractNamespaceNameFromKey(placementKey)
+	namespace, name, err := ExtractNamespaceNameFromKey(bindingKey)
 	if err != nil {
 		return nil, err
 	}

pkg/utils/controller/binding_resolver_test.go

@@ -368,9 +368,32 @@ func TestFetchBindingFromKey(t *testing.T) {
 		{
 			name:         "cluster-scoped placement key - ClusterResourceBinding not found",
 			placementKey: queue.PlacementKey("test-placement"),
-			objects:      []client.Object{},
-			wantErr:      true,
-			wantBinding:  nil,
+			objects: []client.Object{
+				&placementv1beta1.ClusterResourceBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-binding-1",
+						Labels: map[string]string{
+							placementv1beta1.CRPTrackingLabel: "test-placement",
+						},
+					},
+					Spec: placementv1beta1.ResourceBindingSpec{
+						TargetCluster: "cluster-1",
+					},
+				},
+				&placementv1beta1.ClusterResourceBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "other-binding",
+						Labels: map[string]string{
+							placementv1beta1.CRPTrackingLabel: "other-placement",
+						},
+					},
+					Spec: placementv1beta1.ResourceBindingSpec{
+						TargetCluster: "cluster-2",
+					},
+				},
+			},
+			wantErr:     true,
+			wantBinding: nil,
 		},
 		{
 			name:         "namespaced placement key - ResourceBinding found",
@@ -385,6 +408,24 @@ func TestFetchBindingFromKey(t *testing.T) {
 						TargetCluster: "cluster-1",
 					},
 				},
+				&placementv1beta1.ResourceBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-placement",
+						Namespace: "other-namespace",
+					},
+					Spec: placementv1beta1.ResourceBindingSpec{
+						TargetCluster: "cluster-1",
+					},
+				},
+				&placementv1beta1.ResourceBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "other-placement",
+						Namespace: "test-namespace",
+					},
+					Spec: placementv1beta1.ResourceBindingSpec{
+						TargetCluster: "cluster-1",
+					},
+				},
 			},
 			wantErr: false,
 			wantBinding: &placementv1beta1.ResourceBinding{
@@ -400,9 +441,28 @@ func TestFetchBindingFromKey(t *testing.T) {
 		{
 			name:         "namespaced placement key - ResourceBinding not found",
 			placementKey: queue.PlacementKey("test-namespace/test-placement"),
-			objects:      []client.Object{},
-			wantErr:      true,
-			wantBinding:  nil,
+			objects: []client.Object{
+				&placementv1beta1.ResourceBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-placement",
+						Namespace: "other-namespace",
+					},
+					Spec: placementv1beta1.ResourceBindingSpec{
+						TargetCluster: "cluster-1",
+					},
+				},
+				&placementv1beta1.ResourceBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "other-placement",
+						Namespace: "test-namespace",
+					},
+					Spec: placementv1beta1.ResourceBindingSpec{
+						TargetCluster: "cluster-1",
+					},
+				},
+			},
+			wantErr:     true,
+			wantBinding: nil,
 		},
 		{
 			name:         "invalid placement key format - too many separators",

pkg/utils/controller/placement_resolver.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"strings"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -66,8 +67,8 @@ func FetchPlacementFromKey(ctx context.Context, c client.Reader, placementKey qu
 	}
 }
 
-// GetPlacementKeyFromObj generates a PlacementKey from a placement object.
-func GetPlacementKeyFromObj(obj client.Object) queue.PlacementKey {
+// GetObjectKeyFromObj generates a PlacementKey from an meta object.
+func GetObjectKeyFromObj(obj metav1.Object) queue.PlacementKey {
 	if obj.GetNamespace() == "" {
 		// Cluster-scoped placement
 		return queue.PlacementKey(obj.GetName())
@@ -77,8 +78,8 @@ func GetPlacementKeyFromObj(obj client.Object) queue.PlacementKey {
 	}
 }
 
-// GetPlacementKeyFromObj generates a PlacementKey from a placement object.
-func GetPlacementKeyFromRequest(req ctrl.Request) queue.PlacementKey {
+// GetObjectKeyFromRequest generates an object key from a controller runtime request.
+func GetObjectKeyFromRequest(req ctrl.Request) queue.PlacementKey {
 	if req.Namespace == "" {
 		// Cluster-scoped placement
 		return queue.PlacementKey(req.Name)
@@ -89,8 +90,8 @@ func GetPlacementKeyFromRequest(req ctrl.Request) queue.PlacementKey {
 }
 
 // ExtractNamespaceNameFromKey resolves a PlacementKey to a (namespace, name) tuple of the placement object.
-func ExtractNamespaceNameFromKey(placementKey queue.PlacementKey) (string, string, error) {
-	keyStr := string(placementKey)
+func ExtractNamespaceNameFromKey(key queue.PlacementKey) (string, string, error) {
+	keyStr := string(key)
 	// Check if the key contains a namespace separator
 	if strings.Contains(keyStr, namespaceSeparator) {
 		// This is a namespaced ResourcePlacement

pkg/utils/controller/placement_resolver_test.go

@@ -194,7 +194,7 @@ func TestGetPlacementKeyFromObj(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			key := GetPlacementKeyFromObj(tt.placement)
+			key := GetObjectKeyFromObj(tt.placement)
 			if key != tt.wantKey {
 				t.Errorf("GetPlacementKeyFromObj() = %v, want %v", key, tt.wantKey)
 			}