Implement top loevel apply methoid in resource scopes

Author: g-gaston

exp/addons/internal/controllers/clusterresourceset_controller.go

@@ -320,7 +320,7 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 		// Apply all values in the key-value pair of the resource to the cluster.
 		// As there can be multiple key-value pairs in a resource, each value may have multiple objects in it.
 		isSuccessful := true
-		if err := apply(ctx, remoteClient, resourceScope); err != nil {
+		if err := resourceScope.apply(ctx, remoteClient); err != nil {
 			isSuccessful = false
 			log.Error(err, "failed to apply ClusterResourceSet resource", "Resource kind", resource.Kind, "Resource name", resource.Name)
 			conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.ApplyFailedReason, clusterv1.ConditionSeverityWarning, err.Error())

exp/addons/internal/controllers/clusterresourceset_helpers.go

@@ -33,7 +33,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
-	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -94,19 +93,6 @@ func isJSONList(data []byte) (bool, error) {
 	return bytes.HasPrefix(trim, jsonListPrefix), nil
 }
 
-// apply reconciles all objects defined by a resource from a ClusterResourceSet
-// delegating on the reconcileScope for the applying strategy.
-func apply(ctx context.Context, c client.Client, scope resourceReconcileScope) error {
-	errList := []error{}
-	objs := scope.objs()
-	for i := range objs {
-		if err := scope.apply(ctx, c, &objs[i]); err != nil {
-			errList = append(errList, err)
-		}
-	}
-	return kerrors.NewAggregate(errList)
-}
-
 func createUnstructured(ctx context.Context, c client.Client, obj *unstructured.Unstructured) error {
 	if err := c.Create(ctx, obj); err != nil {
 		return errors.Wrapf(

exp/addons/internal/controllers/clusterresourceset_scope.go

@@ -22,6 +22,7 @@ import (
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -34,11 +35,8 @@ type resourceReconcileScope interface {
 	// needsApply determines if a resource needs to be applied to the target cluster
 	// based on the strategy.
 	needsApply() bool
-	// apply reconciles the resource to the target cluster following a different process
-	// depending on the strategy.
-	apply(ctx context.Context, c client.Client, obj *unstructured.Unstructured) error
-	// objs returns all the Kubernetes objects defined in the resource.
-	objs() []unstructured.Unstructured
+	// apply reconciles all objects defined by the resource following the proper strategy for the CRS.
+	apply(ctx context.Context, c client.Client) error
 	// hash returns a computed hash of the defined objects in the resource. It is consistent
 	// between runs.
 	hash() string
@@ -116,7 +114,11 @@ func (r *reconcileStrategyScope) needsApply() bool {
 	return resourceBinding == nil || !resourceBinding.Applied || resourceBinding.Hash != r.computedHash
 }
 
-func (r *reconcileStrategyScope) apply(ctx context.Context, c client.Client, obj *unstructured.Unstructured) error {
+func (r *reconcileStrategyScope) apply(ctx context.Context, c client.Client) error {
+	return apply(ctx, c, r.applyObj, r.objs())
+}
+
+func (r *reconcileStrategyScope) applyObj(ctx context.Context, c client.Client, obj *unstructured.Unstructured) error {
 	currentObj := &unstructured.Unstructured{}
 	currentObj.SetAPIVersion(obj.GetAPIVersion())
 	currentObj.SetKind(obj.GetKind())
@@ -156,11 +158,28 @@ func (r *reconcileApplyOnceScope) needsApply() bool {
 	return !r.resourceSetBinding.IsApplied(r.resourceRef)
 }
 
-func (r *reconcileApplyOnceScope) apply(ctx context.Context, c client.Client, obj *unstructured.Unstructured) error {
+func (r *reconcileApplyOnceScope) apply(ctx context.Context, c client.Client) error {
+	return apply(ctx, c, r.applyObj, r.objs())
+}
+
+func (r *reconcileApplyOnceScope) applyObj(ctx context.Context, c client.Client, obj *unstructured.Unstructured) error {
 	// The create call is idempotent, so if the object already exists
 	// then do not consider it to be an error.
 	if err := createUnstructured(ctx, c, obj); !apierrors.IsAlreadyExists(err) {
 		return err
 	}
 	return nil
 }
+
+type applyObj func(ctx context.Context, c client.Client, obj *unstructured.Unstructured) error
+
+// apply reconciles unstructured objects using applyObj and aggreates the error if present.
+func apply(ctx context.Context, c client.Client, applyObj applyObj, objs []unstructured.Unstructured) error {
+	errList := []error{}
+	for i := range objs {
+		if err := applyObj(ctx, c, &objs[i]); err != nil {
+			errList = append(errList, err)
+		}
+	}
+	return kerrors.NewAggregate(errList)
+}

exp/addons/internal/controllers/clusterresourceset_scope_test.go

@@ -24,9 +24,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
 )
 
 func TestReconcileStrategyScopeNeedsApply(t *testing.T) {
@@ -191,7 +192,7 @@ func TestReconcileApplyOnceScopeNeedsApply(t *testing.T) {
 	}
 }
 
-func TestReconcileApplyOnceScopeApply(t *testing.T) {
+func TestReconcileApplyOnceScopeApplyObj(t *testing.T) {
 	tests := []struct {
 		name         string
 		existingObjs []client.Object
@@ -239,7 +240,7 @@ func TestReconcileApplyOnceScopeApply(t *testing.T) {
 			ctx := context.Background()
 			client := fake.NewClientBuilder().WithObjects(tt.existingObjs...).Build()
 			scope := &reconcileApplyOnceScope{}
-			err := scope.apply(ctx, client, tt.obj)
+			err := scope.applyObj(ctx, client, tt.obj)
 			if tt.wantErr == "" {
 				gs.Expect(err).NotTo(HaveOccurred())
 			} else {