fix validation related functions

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

Author: Arvind Thirumurugan

pkg/controllers/updaterun/initialization.go

@@ -83,24 +83,13 @@ func (r *Reconciler) initialize(
 	}
 
 	// TODO (arvindth): remove this conversion step after refactoring other functions to use interface types.
-	// Convert interfaces back to concrete types for compatibility with other functions that haven't been refactored yet.
-	scheduledBindings := make([]*placementv1beta1.ClusterResourceBinding, len(scheduledBindingObjs))
-	toBeDeletedBindings := make([]*placementv1beta1.ClusterResourceBinding, len(toBeDeletedBindingObjs))
-
-	for i, bindingObj := range scheduledBindingObjs {
-		if crb, ok := bindingObj.(*placementv1beta1.ClusterResourceBinding); ok {
-			scheduledBindings[i] = crb
-		} else {
-			return nil, nil, fmt.Errorf("expected ClusterResourceBinding but got %T - initialize function needs further refactoring for namespace-scoped resources", bindingObj)
-		}
+	scheduledBindings, err := controller.ConvertBindingObjsToConcreteCRBArray(scheduledBindingObjs)
+	if err != nil {
+		return nil, nil, err
 	}
-
-	for i, bindingObj := range toBeDeletedBindingObjs {
-		if crb, ok := bindingObj.(*placementv1beta1.ClusterResourceBinding); ok {
-			toBeDeletedBindings[i] = crb
-		} else {
-			return nil, nil, fmt.Errorf("expected ClusterResourceBinding but got %T - initialize function needs further refactoring for namespace-scoped resources", bindingObj)
-		}
+	toBeDeletedBindings, err := controller.ConvertBindingObjsToConcreteCRBArray(toBeDeletedBindingObjs)
+	if err != nil {
+		return nil, nil, err
 	}
 
 	return scheduledBindings, toBeDeletedBindings, r.recordInitializationSucceeded(ctx, updateRun)

pkg/controllers/updaterun/validation.go

@@ -75,16 +75,27 @@ func (r *Reconciler) validate(
 	}
 
 	// Collect the clusters by the corresponding ClusterResourcePlacement with the latest policy snapshot.
-	scheduledBindings, toBeDeletedBindings, err := r.collectScheduledClusters(ctx, placementName, latestPolicySnapshot, updateRunCopy)
+	scheduledBindingObjs, toBeDeletedBindingObjs, err := r.collectScheduledClusters(ctx, placementName, latestPolicySnapshot, updateRunCopy)
 	if err != nil {
 		return -1, nil, nil, err
 	}
 
 	// Validate the stages and return the updating stage index.
-	updatingStageIndex, err := r.validateStagesStatus(ctx, scheduledBindings, toBeDeletedBindings, updateRun, updateRunCopy)
+	updatingStageIndex, err := r.validateStagesStatus(ctx, scheduledBindingObjs, toBeDeletedBindingObjs, updateRun, updateRunCopy)
 	if err != nil {
 		return -1, nil, nil, err
 	}
+
+	// TODO (arvindth): remove this conversion step after refactoring other functions to use interface types.
+	scheduledBindings, err := controller.ConvertBindingObjsToConcreteCRBArray(scheduledBindingObjs)
+	if err != nil {
+		return -1, nil, nil, err
+	}
+	toBeDeletedBindings, err := controller.ConvertBindingObjsToConcreteCRBArray(toBeDeletedBindingObjs)
+	if err != nil {
+		return -1, nil, nil, err
+	}
+
 	return updatingStageIndex, scheduledBindings, toBeDeletedBindings, nil
 }
 
@@ -95,7 +106,7 @@ func (r *Reconciler) validate(
 // If the updating stage index is len(updateRun.Status.StagesStatus), the next stage to be updated will be the delete stage.
 func (r *Reconciler) validateStagesStatus(
 	ctx context.Context,
-	scheduledBindings, toBeDeletedBindings []*placementv1beta1.ClusterResourceBinding,
+	scheduledBindingObjs, toBeDeletedBindingObjs []placementv1beta1.BindingObj,
 	updateRun, updateRunCopy *placementv1beta1.ClusterStagedUpdateRun,
 ) (int, error) {
 	updateRunRef := klog.KObj(updateRun)
@@ -108,7 +119,7 @@ func (r *Reconciler) validateStagesStatus(
 		klog.ErrorS(unexpectedErr, "Failed to find the stagedUpdateStrategySnapshot in the clusterStagedUpdateRun", "clusterStagedUpdateRun", updateRunRef)
 		return -1, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error())
 	}
-	if err := r.computeRunStageStatus(ctx, scheduledBindings, updateRunCopy); err != nil {
+	if err := r.computeRunStageStatus(ctx, scheduledBindingObjs, updateRunCopy); err != nil {
 		return -1, err
 	}
 
@@ -124,7 +135,7 @@ func (r *Reconciler) validateStagesStatus(
 		return -1, validateErr
 	}
 
-	return validateDeleteStageStatus(updatingStageIndex, lastFinishedStageIndex, len(existingStageStatus), toBeDeletedBindings, updateRunCopy)
+	return validateDeleteStageStatus(updatingStageIndex, lastFinishedStageIndex, len(existingStageStatus), toBeDeletedBindingObjs, updateRunCopy)
 }
 
 // validateUpdateStagesStatus is a helper function to validate the updating stages in the clusterStagedUpdateRun.
@@ -256,7 +267,7 @@ func validateClusterUpdatingStatus(
 // It returns the updating stage index, or any error encountered.
 func validateDeleteStageStatus(
 	updatingStageIndex, lastFinishedStageIndex, totalStages int,
-	toBeDeletedBindings []*placementv1beta1.ClusterResourceBinding,
+	toBeDeletedBindingObjs []placementv1beta1.BindingObj,
 	updateRun *placementv1beta1.ClusterStagedUpdateRun,
 ) (int, error) {
 	updateRunRef := klog.KObj(updateRun)
@@ -274,10 +285,11 @@ func validateDeleteStageStatus(
 	for _, cluster := range existingDeleteStageStatus.Clusters {
 		deletingClusterMap[cluster.ClusterName] = struct{}{}
 	}
-	for _, binding := range toBeDeletedBindings {
-		if _, ok := deletingClusterMap[binding.Spec.TargetCluster]; !ok {
-			unexpectedErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("the cluster `%s` to be deleted is not in the delete stage", binding.Spec.TargetCluster))
-			klog.ErrorS(unexpectedErr, "Detect new cluster to be unscheduled", "clusterResourceBinding", klog.KObj(binding), "clusterStagedUpdateRun", updateRunRef)
+	for _, bindingObj := range toBeDeletedBindingObjs {
+		bindingSpec := bindingObj.GetBindingSpec()
+		if _, ok := deletingClusterMap[bindingSpec.TargetCluster]; !ok {
+			unexpectedErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("the cluster `%s` to be deleted is not in the delete stage", bindingSpec.TargetCluster))
+			klog.ErrorS(unexpectedErr, "Detect new cluster to be unscheduled", "binding", klog.KObj(bindingObj), "clusterStagedUpdateRun", updateRunRef)
 			return -1, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error())
 		}
 	}

pkg/utils/controller/binding_resolver.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"context"
+	"fmt"
 
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -122,3 +123,17 @@ func ConvertRBArrayToBindingObjs(bindings []*placementv1beta1.ResourceBinding) [
 	}
 	return result
 }
+
+// ConvertBindingObjsToConcreteCRBArray converts a slice of BindingObj interfaces to a slice of concrete ClusterResourceBinding pointers.
+func ConvertBindingObjsToConcreteCRBArray(bindingObjs []placementv1beta1.BindingObj) ([]*placementv1beta1.ClusterResourceBinding, error) {
+	bindings := make([]*placementv1beta1.ClusterResourceBinding, len(bindingObjs))
+
+	for i, bindingObj := range bindingObjs {
+		if crb, ok := bindingObj.(*placementv1beta1.ClusterResourceBinding); ok {
+			bindings[i] = crb
+		} else {
+			return nil, fmt.Errorf("expected ClusterResourceBinding but got %T - initialize function needs further refactoring for namespace-scoped resources", bindingObj)
+		}
+	}
+	return bindings, nil
+}