From 8b8d293d033e8f5dc4b18d22cc8f530270adcc9f Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 22 Oct 2024 17:51:44 +0200 Subject: [PATCH] refactor: uniform mark as ready logic Signed-off-by: Armando Ruocco --- api/v1/publication_funcs.go | 7 ++++++ api/v1/subscription_funcs.go | 7 ++++++ internal/management/controller/common.go | 18 ++++++++++++++- .../controller/publication_controller.go | 21 +----------------- .../controller/subscription_controller.go | 22 ++----------------- 5 files changed, 34 insertions(+), 41 deletions(-) diff --git a/api/v1/publication_funcs.go b/api/v1/publication_funcs.go index e7a29d1b48..57f73184a3 100644 --- a/api/v1/publication_funcs.go +++ b/api/v1/publication_funcs.go @@ -21,3 +21,10 @@ func (pub *Publication) SetAsFailed(err error) { pub.Status.Ready = false pub.Status.Error = err.Error() } + +// SetAsReady sets the subscription as working correctly +func (pub *Publication) SetAsReady() { + pub.Status.Error = "" + pub.Status.Ready = true + pub.Status.ObservedGeneration = pub.Generation +} diff --git a/api/v1/subscription_funcs.go b/api/v1/subscription_funcs.go index 8459b8a157..40511e1380 100644 --- a/api/v1/subscription_funcs.go +++ b/api/v1/subscription_funcs.go @@ -21,3 +21,10 @@ func (sub *Subscription) SetAsFailed(err error) { sub.Status.Ready = false sub.Status.Error = err.Error() } + +// SetAsReady sets the subscription as working correctly +func (sub *Subscription) SetAsReady() { + sub.Status.Error = "" + sub.Status.Ready = true + sub.Status.ObservedGeneration = sub.Generation +} diff --git a/internal/management/controller/common.go b/internal/management/controller/common.go index f70ca44229..76d5cee083 100644 --- a/internal/management/controller/common.go +++ b/internal/management/controller/common.go @@ -18,7 +18,6 @@ package controller import ( "context" - "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -38,3 +37,20 @@ func markAsFailed( resource.SetAsFailed(err) return cli.Status().Patch(ctx, resource, client.MergeFrom(oldResource)) } + +type markableAsReady interface { + client.Object + SetAsReady() +} + +// markAsReady marks the reconciliation as succeeded inside the resource +func markAsReady( + ctx context.Context, + cli client.Client, + resource markableAsReady, +) error { + oldResource := resource.DeepCopyObject().(markableAsReady) + resource.SetAsReady() + + return cli.Status().Patch(ctx, resource, client.MergeFrom(oldResource)) +} diff --git a/internal/management/controller/publication_controller.go b/internal/management/controller/publication_controller.go index 4db811a84e..6106ec8ea1 100644 --- a/internal/management/controller/publication_controller.go +++ b/internal/management/controller/publication_controller.go @@ -120,7 +120,7 @@ func (r *PublicationReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{RequeueAfter: databaseReconciliationInterval}, markAsFailed(ctx, r.Client, &publication, err) } - return r.succeededReconciliation(ctx, &publication) + return ctrl.Result{RequeueAfter: subscriptionReconciliationInterval}, markAsReady(ctx, r.Client, &publication) } func (r *PublicationReconciler) reconcileFinalizer(ctx context.Context, publication apiv1.Publication) error { @@ -168,25 +168,6 @@ func (r *PublicationReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// succeededReconciliation marks the reconciliation as succeeded -func (r *PublicationReconciler) succeededReconciliation( - ctx context.Context, - publication *apiv1.Publication, -) (ctrl.Result, error) { - oldPublication := publication.DeepCopy() - publication.Status.Error = "" - publication.Status.Ready = true - publication.Status.ObservedGeneration = publication.Generation - - if err := r.Client.Status().Patch(ctx, publication, client.MergeFrom(oldPublication)); err != nil { - return ctrl.Result{}, err - } - - return ctrl.Result{ - RequeueAfter: publicationReconciliationInterval, - }, nil -} - // GetCluster gets the managed cluster through the client func (r *PublicationReconciler) GetCluster(ctx context.Context) (*apiv1.Cluster, error) { var cluster apiv1.Cluster diff --git a/internal/management/controller/subscription_controller.go b/internal/management/controller/subscription_controller.go index 4cc3aac6b5..f8d9e1c897 100644 --- a/internal/management/controller/subscription_controller.go +++ b/internal/management/controller/subscription_controller.go @@ -159,8 +159,9 @@ func (r *SubscriptionReconciler) Reconcile(ctx context.Context, req ctrl.Request ) } - return r.succeededReconciliation( + return ctrl.Result{RequeueAfter: subscriptionReconciliationInterval}, markAsReady( ctx, + r.Client, &subscription, ) } @@ -184,25 +185,6 @@ func (r *SubscriptionReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// succeededReconciliation marks the reconciliation as succeeded -func (r *SubscriptionReconciler) succeededReconciliation( - ctx context.Context, - subscription *apiv1.Subscription, -) (ctrl.Result, error) { - oldSubscription := subscription.DeepCopy() - subscription.Status.Error = "" - subscription.Status.Ready = true - subscription.Status.ObservedGeneration = subscription.Generation - - if err := r.Client.Status().Patch(ctx, subscription, client.MergeFrom(oldSubscription)); err != nil { - return ctrl.Result{}, err - } - - return ctrl.Result{ - RequeueAfter: subscriptionReconciliationInterval, - }, nil -} - // GetCluster gets the managed cluster through the client func (r *SubscriptionReconciler) GetCluster(ctx context.Context) (*apiv1.Cluster, error) { var cluster apiv1.Cluster