From 916e726e335c19e7984b45026295cc8c424eacc4 Mon Sep 17 00:00:00 2001 From: "sebastian.vargas" Date: Wed, 2 Mar 2022 20:23:37 +0000 Subject: [PATCH] Erase the delete conditions and refactor some code to fix the reconcile flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alby Hernández --- controllers/replika_controller.go | 29 +++++++++++++---------------- controllers/replika_status.go | 8 -------- controllers/replika_sync.go | 11 ----------- 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/controllers/replika_controller.go b/controllers/replika_controller.go index a5dfa20..ecea984 100644 --- a/controllers/replika_controller.go +++ b/controllers/replika_controller.go @@ -18,7 +18,6 @@ package controllers import ( "context" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "time" @@ -62,9 +61,8 @@ func (r *ReplikaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re result = ctrl.Result{} // 2.1 It does NOT exist: manage removal - if errors.IsNotFound(err) { + if err = client.IgnoreNotFound(err); err == nil { LogInfof(ctx, "Replika resource not found. Ignoring since object must be deleted.") - err = nil return result, err } @@ -73,17 +71,8 @@ func (r *ReplikaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re return result, err } - // 3. Update the status before the requeue - defer func() { - err = r.Status().Update(ctx, replikaManifest) - if err != nil { - LogInfof(ctx, "Failed to update the conditionnnnnnn on replika: %s", req.Name) - } - }() - // 4. Check if the Replika instance is marked to be deleted: indicated by the deletion timestamp being set - isReplikaMarkedToBeDeleted := replikaManifest.GetDeletionTimestamp() != nil - if isReplikaMarkedToBeDeleted { + if !replikaManifest.DeletionTimestamp.IsZero() { if controllerutil.ContainsFinalizer(replikaManifest, replikaFinalizer) { // Delete all created targets err = r.DeleteTargets(ctx, replikaManifest) @@ -96,12 +85,20 @@ func (r *ReplikaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re controllerutil.RemoveFinalizer(replikaManifest, replikaFinalizer) err = r.Update(ctx, replikaManifest) if err != nil { - return result, err + LogInfof(ctx, "Failed to update finalizer of replika: %s", req.Name) } } return result, err } + // 3. Update the status before the requeue + defer func() { + err = r.Status().Update(ctx, replikaManifest) + if err != nil { + LogInfof(ctx, "Failed to update the condition on replika: %s", req.Name) + } + }() + // 5. Add finalizer to the Replika CR if !controllerutil.ContainsFinalizer(replikaManifest, replikaFinalizer) { controllerutil.AddFinalizer(replikaManifest, replikaFinalizer) @@ -114,14 +111,14 @@ func (r *ReplikaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // 6. The Replika CR already exist: manage the update err = r.UpdateTargets(ctx, replikaManifest) if err != nil { - LogInfof(ctx, "Can not update the targets for the Replika: "+replikaManifest.Name) + LogInfof(ctx, "Can not update the targets for the Replika: %s", replikaManifest.Name) return result, err } // 7. Schedule periodical request RequeueTime, err := r.GetSynchronizationTime(replikaManifest) if err != nil { - LogInfof(ctx, "Can not requeue the Replika: "+replikaManifest.Name) + LogInfof(ctx, "Can not requeue the Replika: %s", replikaManifest.Name) } result.RequeueAfter = RequeueTime diff --git a/controllers/replika_status.go b/controllers/replika_status.go index e012fbc..8e96e90 100644 --- a/controllers/replika_status.go +++ b/controllers/replika_status.go @@ -22,14 +22,6 @@ const ( ConditionReasonSourceReplicationFailed = "SourceReplicationFailed" ConditionReasonSourceReplicationFailedMessage = "Error replicating the source on targets" - // Get existent target list - ConditionReasonTargetGetListFailed = "TargetGetListFailed" - ConditionReasonTargetGetListFailedMessage = "Error getting the targets from the cluster" - - // Target delete failed - ConditionReasonTargetDeleteFailed = "TargetDeleteFailed" - ConditionReasonTargetDeleteFailedMessage = "Target resource failed on deletion" - // Success ConditionReasonSourceSynced = "SourceSynced" ConditionReasonSourceSyncedMessage = "Source was successfully synchronized" diff --git a/controllers/replika_sync.go b/controllers/replika_sync.go index 8edf061..3a81737 100644 --- a/controllers/replika_sync.go +++ b/controllers/replika_sync.go @@ -257,11 +257,6 @@ func (r *ReplikaReconciler) DeleteTargets(ctx context.Context, replika *replikav // Look for the targets inside the cluster err = r.List(ctx, targets, client.MatchingLabels{resourceReplikaLabelPartOfKey: replika.Name}) if err != nil { - r.UpdateReplikaCondition(replika, r.NewReplikaCondition(ConditionTypeSourceSynced, - metav1.ConditionFalse, - ConditionReasonTargetGetListFailed, - ConditionReasonTargetGetListFailedMessage, - )) return err } @@ -269,12 +264,6 @@ func (r *ReplikaReconciler) DeleteTargets(ctx context.Context, replika *replikav for i := range targets.Items { err = r.Delete(ctx, &targets.Items[i]) if err != nil { - r.UpdateReplikaCondition(replika, r.NewReplikaCondition(ConditionTypeSourceSynced, - metav1.ConditionFalse, - ConditionReasonTargetDeleteFailed, - ConditionReasonTargetDeleteFailedMessage, - )) - return err } }