From 6c656f80a5f35546b3dcb28b577fc7dfdfca9928 Mon Sep 17 00:00:00 2001 From: Guillermo Gaston Date: Mon, 9 Jan 2023 15:30:40 +0000 Subject: [PATCH] Implement top loevel apply methoid in resource scopes --- .../clusterresourceset_controller.go | 2 +- .../controllers/clusterresourceset_helpers.go | 14 -------- .../controllers/clusterresourceset_scope.go | 33 +++++++++++++++---- .../clusterresourceset_scope_test.go | 7 ++-- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/exp/addons/internal/controllers/clusterresourceset_controller.go b/exp/addons/internal/controllers/clusterresourceset_controller.go index 474abe60d4bf..2d3cd72a19da 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller.go +++ b/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()) diff --git a/exp/addons/internal/controllers/clusterresourceset_helpers.go b/exp/addons/internal/controllers/clusterresourceset_helpers.go index 2e5f1eeb948f..c0b8fa1a78e6 100644 --- a/exp/addons/internal/controllers/clusterresourceset_helpers.go +++ b/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( diff --git a/exp/addons/internal/controllers/clusterresourceset_scope.go b/exp/addons/internal/controllers/clusterresourceset_scope.go index d20b61811a0e..e4af7ff3d2c5 100644 --- a/exp/addons/internal/controllers/clusterresourceset_scope.go +++ b/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,7 +158,11 @@ 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) { @@ -164,3 +170,16 @@ func (r *reconcileApplyOnceScope) apply(ctx context.Context, c client.Client, ob } 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) +} diff --git a/exp/addons/internal/controllers/clusterresourceset_scope_test.go b/exp/addons/internal/controllers/clusterresourceset_scope_test.go index febc3e5471d1..624798294420 100644 --- a/exp/addons/internal/controllers/clusterresourceset_scope_test.go +++ b/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 {