update validatePlacement function

Signed-off-by: Arvind Thirumurugan <[email protected]>

Author: Arvind Thirumurugan

pkg/controllers/placement/controller.go

@@ -72,7 +72,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key controller.QueueKey) (ct
 		klog.V(2).InfoS("Placement reconciliation ends", "placementKey", placementKey, "latency", latency)
 	}()
 
-	placementObj, err := controller.FetchPlacementFromKey(ctx, r.Client, queue.PlacementKey(placementKey))
+	placementObj, err := controller.FetchPlacementFromQueueKey(ctx, r.Client, queue.PlacementKey(placementKey))
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			klog.V(4).InfoS("Ignoring NotFound placement", "placementKey", placementKey)

pkg/controllers/rollout/controller.go

@@ -72,7 +72,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 	}()
 
 	// Get the placement object (either ClusterResourcePlacement or ResourcePlacement)
-	placementObj, err := controller.FetchPlacementFromKey(ctx, r.Client, controller.GetObjectKeyFromRequest(req))
+	placementObj, err := controller.FetchPlacementFromQueueKey(ctx, r.Client, controller.GetObjectKeyFromRequest(req))
 	if err != nil {
 		if errors.IsNotFound(err) {
 			klog.V(4).InfoS("Ignoring NotFound placement", "placementKey", placementKey)

pkg/controllers/updaterun/initialization.go

@@ -27,6 +27,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -44,8 +45,8 @@ func (r *Reconciler) initialize(
 	ctx context.Context,
 	updateRun *placementv1beta1.ClusterStagedUpdateRun,
 ) ([]*placementv1beta1.ClusterResourceBinding, []*placementv1beta1.ClusterResourceBinding, error) {
-	// Validate the ClusterResourcePlace object referenced by the ClusterStagedUpdateRun.
-	placementName, err := r.validateCRP(ctx, updateRun)
+	// Validate the Placement object referenced by the StagedUpdateRun.
+	placementName, err := r.validatePlacement(ctx, updateRun)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -71,29 +72,48 @@ func (r *Reconciler) initialize(
 	return scheduledBindings, toBeDeletedBindings, r.recordInitializationSucceeded(ctx, updateRun)
 }
 
-// validateCRP validates the ClusterResourcePlacement object referenced by the ClusterStagedUpdateRun.
-func (r *Reconciler) validateCRP(ctx context.Context, updateRun *placementv1beta1.ClusterStagedUpdateRun) (string, error) {
+// validatePlacement validates the Placement object (CRP or RP) referenced by the StagedUpdateRun.
+func (r *Reconciler) validatePlacement(ctx context.Context, updateRun placementv1beta1.StagedUpdateRunObj) (string, error) {
 	updateRunRef := klog.KObj(updateRun)
-	// Fetch the ClusterResourcePlacement object.
-	placementName := updateRun.Spec.PlacementName
-	var crp placementv1beta1.ClusterResourcePlacement
-	if err := r.Client.Get(ctx, client.ObjectKey{Name: placementName}, &crp); err != nil {
-		klog.ErrorS(err, "Failed to get ClusterResourcePlacement", "clusterResourcePlacement", placementName, "clusterStagedUpdateRun", updateRunRef)
+
+	// Get placement name from the updateRun spec using interface methods
+	placementName := updateRun.GetStagedUpdateRunSpec().PlacementName
+
+	// Create NamespacedName for the placement
+	// For ClusterStagedUpdateRun, namespace will be empty (cluster-scoped)
+	// For StagedUpdateRun, namespace will be the actual namespace (namespace-scoped)
+	namespacedName := types.NamespacedName{
+		Name:      placementName,
+		Namespace: updateRun.GetNamespace(),
+	}
+
+	// Fetch the placement object using the utility function
+	// This automatically determines whether to fetch CRP or RP based on namespace presence
+	placement, err := controller.FetchPlacementFromNamespacedName(ctx, r.Client, namespacedName)
+	if err != nil {
 		if apierrors.IsNotFound(err) {
-			// we won't continue or retry the initialization if the ClusterResourcePlacement is not found.
-			crpNotFoundErr := controller.NewUserError(fmt.Errorf("parent clusterResourcePlacement not found"))
-			return "", fmt.Errorf("%w: %s", errInitializedFailed, crpNotFoundErr.Error())
+			placementNotFoundErr := controller.NewUserError(fmt.Errorf("parent placement not found"))
+			klog.ErrorS(err, "Failed to get placement", "placement", placementName, "namespace", updateRun.GetNamespace(), "stagedUpdateRun", updateRunRef)
+			return "", fmt.Errorf("%w: %s", errInitializedFailed, placementNotFoundErr.Error())
 		}
+		klog.ErrorS(err, "Failed to get placement", "placement", placementName, "namespace", updateRun.GetNamespace(), "stagedUpdateRun", updateRunRef)
 		return "", controller.NewAPIServerError(true, err)
 	}
-	// Check if the ClusterResourcePlacement has an external rollout strategy.
-	if crp.Spec.Strategy.Type != placementv1beta1.ExternalRolloutStrategyType {
-		klog.V(2).InfoS("The clusterResourcePlacement does not have an external rollout strategy", "clusterResourcePlacement", placementName, "clusterStagedUpdateRun", updateRunRef)
-		wrongRolloutTypeErr := controller.NewUserError(errors.New("parent clusterResourcePlacement does not have an external rollout strategy, current strategy: " + string(crp.Spec.Strategy.Type)))
+
+	// Check if the Placement has an external rollout strategy using interface methods
+	placementSpec := placement.GetPlacementSpec()
+	if placementSpec.Strategy.Type != placementv1beta1.ExternalRolloutStrategyType {
+		klog.V(2).InfoS("The placement does not have an external rollout strategy", "placement", placementName, "namespace", updateRun.GetNamespace(), "stagedUpdateRun", updateRunRef)
+		wrongRolloutTypeErr := controller.NewUserError(errors.New("parent placement does not have an external rollout strategy, current strategy: " + string(placementSpec.Strategy.Type)))
 		return "", fmt.Errorf("%w: %s", errInitializedFailed, wrongRolloutTypeErr.Error())
 	}
-	updateRun.Status.ApplyStrategy = crp.Spec.Strategy.ApplyStrategy
-	return crp.Name, nil
+
+	// Update the apply strategy in the updateRun status using interface methods
+	updateRunStatus := updateRun.GetStagedUpdateRunStatus()
+	updateRunStatus.ApplyStrategy = placementSpec.Strategy.ApplyStrategy
+	updateRun.SetStagedUpdateRunStatus(*updateRunStatus)
+
+	return placementName, nil
 }
 
 // determinePolicySnapshot retrieves the latest policy snapshot associated with the ClusterResourcePlacement,

pkg/controllers/updaterun/validation.go

@@ -44,7 +44,7 @@ func (r *Reconciler) validate(
 	klog.V(2).InfoS("Start to validate the clusterStagedUpdateRun", "clusterStagedUpdateRun", updateRunRef)
 
 	// Validate the ClusterResourcePlacement object referenced by the ClusterStagedUpdateRun.
-	placementName, err := r.validateCRP(ctx, updateRunCopy)
+	placementName, err := r.validatePlacement(ctx, updateRunCopy)
 	if err != nil {
 		return -1, nil, nil, err
 	}

pkg/scheduler/scheduler.go

@@ -134,7 +134,7 @@ func (s *Scheduler) scheduleOnce(ctx context.Context, worker int) {
 	}()
 
 	// Retrieve the placement object (either ClusterResourcePlacement or ResourcePlacement).
-	placement, err := controller.FetchPlacementFromKey(ctx, s.client, placementKey)
+	placement, err := controller.FetchPlacementFromQueueKey(ctx, s.client, placementKey)
 	if err != nil {
 		if apiErrors.IsNotFound(err) {
 			// The placement has been gone before the scheduler gets a chance to

pkg/scheduler/watchers/placement/watcher.go

@@ -55,7 +55,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	// Retrieve the placement object (either ClusterResourcePlacement or ResourcePlacement).
 	placementKey := controller.GetObjectKeyFromRequest(req)
-	placement, err := controller.FetchPlacementFromKey(ctx, r.Client, placementKey)
+	placement, err := controller.FetchPlacementFromQueueKey(ctx, r.Client, placementKey)
 	if err != nil {
 		klog.ErrorS(err, "Failed to get placement", "placement", placementRef)
 		return ctrl.Result{}, controller.NewAPIServerError(true, client.IgnoreNotFound(err))

pkg/utils/controller/placement_resolver.go

@@ -35,27 +35,29 @@ const (
 	namespaceSeparator = "/"
 )
 
-// FetchPlacementFromKey resolves a PlacementKey to a concrete placement object that implements PlacementObj.
-func FetchPlacementFromKey(ctx context.Context, c client.Reader, placementKey queue.PlacementKey) (fleetv1beta1.PlacementObj, error) {
+// FetchPlacementFromQueueKey resolves a Queue PlacementKey to a concrete placement object that implements PlacementObj.
+func FetchPlacementFromQueueKey(ctx context.Context, c client.Reader, placementKey queue.PlacementKey) (fleetv1beta1.PlacementObj, error) {
 	// Extract namespace and name from the placement key
 	namespace, name, err := ExtractNamespaceNameFromKey(placementKey)
 	if err != nil {
 		return nil, err
 	}
+	return FetchPlacementFromNamespacedName(ctx, c, types.NamespacedName{Namespace: namespace, Name: name})
+}
+
+// FetchPlacementFromNamespacedName resolves a NamespacedName to a concrete placement object that implements PlacementObj.
+func FetchPlacementFromNamespacedName(ctx context.Context, c client.Reader, nn types.NamespacedName) (fleetv1beta1.PlacementObj, error) {
 	// Check if the key contains a namespace separator
 	var placement fleetv1beta1.PlacementObj
-	if namespace != "" {
+	if nn.Namespace != "" {
 		// This is a namespaced ResourcePlacement
 		placement = &fleetv1beta1.ResourcePlacement{}
 	} else {
 		// This is a cluster-scoped ClusterResourcePlacement
 		placement = &fleetv1beta1.ClusterResourcePlacement{}
 	}
-	key := types.NamespacedName{
-		Namespace: namespace,
-		Name:      name,
-	}
-	if err := c.Get(ctx, key, placement); err != nil {
+
+	if err := c.Get(ctx, nn, placement); err != nil {
 		return nil, err
 	}
 	return placement, nil
@@ -94,6 +96,14 @@ func GetObjectKeyFromNamespaceName(namespace, name string) string {
 	}
 }
 
+// GetNamespacedNameFromObject generates a NamespacedName from a meta object.
+func GetNamespacedNameFromObject(obj metav1.Object) types.NamespacedName {
+	return types.NamespacedName{
+		Namespace: obj.GetNamespace(),
+		Name:      obj.GetName(),
+	}
+}
+
 // ExtractNamespaceNameFromKey resolves a PlacementKey to a (namespace, name) tuple of the placement object.
 func ExtractNamespaceNameFromKey(key queue.PlacementKey) (string, string, error) {
 	return ExtractNamespaceNameFromKeyStr(string(key))

pkg/utils/controller/placement_resolver_test.go

@@ -32,7 +32,7 @@ import (
 	"github.com/kubefleet-dev/kubefleet/pkg/scheduler/queue"
 )
 
-func TestFetchPlacementFromKey(t *testing.T) {
+func TestFetchPlacementFromQueueKey(t *testing.T) {
 	scheme := runtime.NewScheme()
 	if err := fleetv1beta1.AddToScheme(scheme); err != nil {
 		t.Fatalf("Failed to add scheme: %v", err)
@@ -302,7 +302,7 @@ func TestFetchPlacementFromKey(t *testing.T) {
 				WithObjects(tt.objects...).
 				Build()
 
-			placement, err := FetchPlacementFromKey(context.Background(), fakeClient, tt.placementKey)
+			placement, err := FetchPlacementFromQueueKey(context.Background(), fakeClient, tt.placementKey)
 
 			if tt.wantErr {
 				if err == nil {
@@ -329,7 +329,7 @@ func TestFetchPlacementFromKey(t *testing.T) {
 			// Ignore resource version field for consistent comparison
 			if diff := cmp.Diff(placement, tt.wantPlacement,
 				cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion")); diff != "" {
-				t.Errorf("FetchPlacementFromKey() diff (-got +want):\n%s", diff)
+				t.Errorf("FetchPlacementFromQueueKey() diff (-got +want):\n%s", diff)
 			}
 
 			// Verify the concrete type matches expected