feat(controller): deprecation of propagation controller (#788)

* feat(controllers) Deprecation of propagation controller (#777) * feat(controllers) Remove filtering to propagate only changes in remote cluster (#777) * feat(controllers) Remove reconciliation intervsl (#777)
cloudoperators · Jan 28, 2025 · 156d206 · 156d206
1 parent 7583483
commit 156d206
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 81 deletions.
diff --git a/pkg/clientutil/predicates.go b/pkg/clientutil/predicates.go
@@ -8,6 +8,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	greenhousev1alpha1 "github.com/cloudoperators/greenhouse/pkg/apis/greenhouse/v1alpha1"
@@ -69,6 +70,12 @@ func PredicateByName(name string) predicate.Predicate {
 	})
 }
 
+func PredicateHasFinalizer(finalizer string) predicate.Predicate {
+	return predicate.NewPredicateFuncs(func(o client.Object) bool {
+		return controllerutil.ContainsFinalizer(o, finalizer)
+	})
+}
+
 func PredicateHasOICDConfigured() predicate.Predicate {
 	return predicate.NewPredicateFuncs(func(o client.Object) bool {
 		org, ok := o.(*greenhousev1alpha1.Organization)

diff --git a/pkg/controllers/propagation_reconciler.go b/pkg/controllers/propagation_reconciler.go
@@ -6,7 +6,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -25,10 +24,6 @@ import (
 	"github.com/cloudoperators/greenhouse/pkg/clientutil"
 )
 
-const (
-	DefaultRequeueInterval = 10 * time.Minute
-)
-
 // PropagationReconciler implements the basic functionality every resource propagation reconciler needs.
 type PropagationReconciler struct {
 	client.Client
@@ -50,15 +45,14 @@ func (r *PropagationReconciler) BaseSetupWithManager(name string, mgr ctrl.Manag
 		// Watch the respective CRD and enqueue all objects.
 		Watches(&apiextensionsv1.CustomResourceDefinition{},
 			handler.EnqueueRequestsFromMapFunc(r.HandlerFunc),
-			builder.WithPredicates(clientutil.PredicateByName(r.CRDName)),
+			builder.WithPredicates(
+				clientutil.PredicateByName(r.CRDName),
+				clientutil.PredicateHasFinalizer(greenhouseapis.FinalizerCleanupPropagatedResource)),
 		).
 		Complete(r)
 }
 
 func (r *PropagationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	// convenience var to collect successful deletions across all clusters
-	removeFinalizer := false
-
 	obj, ok := r.EmptyObj.DeepCopyObject().(client.Object)
 	if !ok {
 		return ctrl.Result{}, fmt.Errorf("object %T is not a client.Object", obj)
@@ -68,8 +62,9 @@ func (r *PropagationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if err := clientutil.EnsureFinalizer(ctx, r.Client, obj, greenhouseapis.FinalizerCleanupPropagatedResource); err != nil {
-		return ctrl.Result{}, err
+	if !controllerutil.ContainsFinalizer(obj, greenhouseapis.FinalizerCleanupPropagatedResource) {
+		fmt.Printf("Skip resource because it does not contain the cleanup finalizer")
+		return ctrl.Result{}, nil
 	}
 
 	clusterList := new(greenhousev1alpha1.ClusterList)
@@ -95,102 +90,42 @@ func (r *PropagationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			return ctrl.Result{}, err
 		}
 
-		if err := r.ReconcileCRD(ctx, remoteRestClient, cluster.GetName(), obj.GetNamespace()); err != nil {
-			return ctrl.Result{}, err
-		}
-
-		if err, removeFinalizer = r.reconcileObject(ctx, remoteRestClient, obj, cluster.GetName()); err != nil {
+		if err = r.reconcileObject(ctx, remoteRestClient, obj, cluster.GetName()); err != nil {
 			return ctrl.Result{}, err
 		}
 	}
-	if removeFinalizer {
-		if err := clientutil.RemoveFinalizer(ctx, r.Client, obj, greenhouseapis.FinalizerCleanupPropagatedResource); err != nil {
-			return ctrl.Result{}, err
-		}
-	}
-
-	return ctrl.Result{RequeueAfter: DefaultRequeueInterval}, nil
-}
-
-func (r *PropagationReconciler) ReconcileCRD(ctx context.Context, remoteClient client.Client, clusterName, namespace string) error {
-	SrcCRD := &apiextensionsv1.CustomResourceDefinition{}
-	if err := r.Client.Get(ctx, types.NamespacedName{Namespace: "", Name: r.CRDName}, SrcCRD); err != nil {
-		return err
-	}
-
-	var remoteNamespace = new(corev1.Namespace)
-	if err := remoteClient.Get(ctx, types.NamespacedName{Namespace: "", Name: namespace}, remoteNamespace); err != nil {
-		log.FromContext(ctx).Error(err, "failed getting remote namespace for CRD owner reference", "CRD", SrcCRD, "cluster", clusterName)
-		return err
+	if err := clientutil.RemoveFinalizer(ctx, r.Client, obj, greenhouseapis.FinalizerCleanupPropagatedResource); err != nil {
+		return ctrl.Result{}, err
 	}
 
-	var remoteCRD = &apiextensionsv1.CustomResourceDefinition{}
-	remoteCRD.SetName(SrcCRD.GetName())
-
-	result, err := clientutil.CreateOrPatch(ctx, remoteClient, remoteCRD, func() error {
-		remoteCRD.Spec = SrcCRD.Spec
-		return controllerutil.SetOwnerReference(remoteNamespace, remoteCRD, remoteClient.Scheme())
-	})
-	if err != nil {
-		return err
-	}
-	message := fmt.Sprintf("%s CRD on target cluster", result)
-	switch result {
-	case clientutil.OperationResultCreated, clientutil.OperationResultUpdated:
-		log.FromContext(ctx).Info(message, "CRD", SrcCRD, "cluster", clusterName)
-	case clientutil.OperationResultNone:
-		log.FromContext(ctx).V(5).Info(message, "CRD", SrcCRD, "cluster", clusterName)
-	}
-	return nil
+	return ctrl.Result{}, nil
 }
 
-func (r *PropagationReconciler) reconcileObject(ctx context.Context, restClient client.Client, obj client.Object, clusterName string) (err error, removeFinalizer bool) {
+func (r *PropagationReconciler) reconcileObject(ctx context.Context, restClient client.Client, obj client.Object, clusterName string) error {
 	remoteObject := obj.DeepCopyObject().(client.Object) //nolint:errcheck
 	remoteObjectExists := true
 	if err := restClient.Get(ctx, client.ObjectKeyFromObject(remoteObject), remoteObject); err != nil {
 		if apierrors.IsNotFound(err) {
 			remoteObjectExists = false
 		} else {
-			return err, false
+			return err
 		}
 	}
 
 	// cleanup
-	if obj.GetDeletionTimestamp() != nil && remoteObjectExists {
+	if remoteObjectExists {
 		if err := restClient.Delete(ctx, remoteObject); err != nil {
 			// might have been deleted by now
 			if apierrors.IsNotFound(err) {
 				log.FromContext(ctx).Info("object does not exist on target cluster", "object", obj, "cluster", clusterName)
-				return nil, true
+				return nil
 			} else {
-				return err, false
+				return err
 			}
 		}
 		log.FromContext(ctx).Info("deleted object on target cluster", "object", obj, "cluster", clusterName)
-		return nil, true
-	}
-
-	remoteObjectResource, err := r.StripObjectWrapper(obj)
-	if err != nil {
-		return err, false
 	}
-
-	// update
-	if remoteObjectExists {
-		remoteObjectResource.SetResourceVersion(remoteObject.GetResourceVersion())
-		if err = restClient.Update(ctx, remoteObjectResource); err != nil {
-			return err, false
-		}
-		log.FromContext(ctx).Info("updated object on target cluster", "object", obj, "cluster", clusterName)
-		return nil, false
-	}
-
-	// create
-	if err = restClient.Create(ctx, remoteObjectResource); err != nil {
-		return err, false
-	}
-	log.FromContext(ctx).Info("created object on target cluster", "object", obj, "cluster", clusterName)
-	return nil, false
+	return nil
 }
 
 func (r *PropagationReconciler) ListObjects(ctx context.Context) client.ObjectList {

diff --git a/pkg/controllers/propagation_reconciler_test.go b/pkg/controllers/propagation_reconciler_test.go
@@ -244,6 +244,7 @@ var _ = Describe("Propagation reconciler", Ordered, func() {
 	)
 
 	It("should have created the crd on the remote clusters with owner reference", func() {
+		Skip("Skipped because propagation reconciler is deprecated.")
 		CRDList := &apiextensionsv1.CustomResourceDefinitionList{}
 		otherCRDList := &apiextensionsv1.CustomResourceDefinitionList{}
 		Eventually(func(g Gomega) bool {
@@ -263,6 +264,7 @@ var _ = Describe("Propagation reconciler", Ordered, func() {
 	})
 
 	It("should have created the resource on the remote cluster", func() {
+		Skip("Skipped because propagation reconciler is deprecated.")
 		remoteObject := &fixtures.Dummy{}
 		otherRemoteObject := &fixtures.Dummy{}
 		Eventually(func(g Gomega) bool {
@@ -277,6 +279,7 @@ var _ = Describe("Propagation reconciler", Ordered, func() {
 	})
 
 	It("should reconcile CRD and object after CRD update", func() {
+		Skip("Skipped because propagation reconciler is deprecated.")
 		By("updating the crd in the local cluster")
 		currentCRD := &apiextensionsv1.CustomResourceDefinition{}
 		err := test.K8sClient.Get(test.Ctx, types.NamespacedName{Namespace: "", Name: dummyCRDName}, currentCRD)
@@ -319,6 +322,7 @@ var _ = Describe("Propagation reconciler", Ordered, func() {
 	})
 
 	It("should reconcile the remote resource after local resource update", func() {
+		Skip("Skipped because propagation reconciler is deprecated.")
 		By("getting the local resource")
 		localObject := dummy
 		updateLabel := map[string]string{"test": "test"}
@@ -347,6 +351,7 @@ var _ = Describe("Propagation reconciler", Ordered, func() {
 	})
 
 	It("should delete the remote resources after deletion", func() {
+		Skip("Skipped because propagation reconciler is deprecated.")
 		By("deleting the local resource")
 		err := test.K8sClient.Delete(test.Ctx, dummy)
 		Expect(err).ToNot(HaveOccurred(), "there should be no error deleting the resource")