diff --git a/reconcilers/aggregate.go b/reconcilers/aggregate.go index 2e0645c..cb9d5ec 100644 --- a/reconcilers/aggregate.go +++ b/reconcilers/aggregate.go @@ -257,14 +257,23 @@ func (r *AggregateReconciler[T]) Reconcile(ctx context.Context, req Request) (Re ctx = StashOriginalResourceType(ctx, r.Type) ctx = StashResourceType(ctx, r.Type) - beforeCtx, result, err := r.BeforeReconcile(ctx, req) + beforeCtx, beforeResult, err := r.BeforeReconcile(ctx, req) if err != nil { - return result, err + return beforeResult, err } if beforeCtx != nil { ctx = beforeCtx } + reconcileResult, err := r.reconcile(ctx, req) + + return r.AfterReconcile(ctx, req, AggregateResults(beforeResult, reconcileResult), err) +} + +func (r *AggregateReconciler[T]) reconcile(ctx context.Context, req Request) (Result, error) { + log := logr.FromContextOrDiscard(ctx) + c := RetrieveConfigOrDie(ctx) + resource := r.Type.DeepCopyObject().(T) if err := c.Get(ctx, req.NamespacedName, resource); err != nil { if apierrs.IsNotFound(err) { @@ -275,19 +284,18 @@ func (r *AggregateReconciler[T]) Reconcile(ctx context.Context, req Request) (Re if !errors.Is(err, ErrQuiet) { log.Error(err, "unable to fetch resource") } - return r.AfterReconcile(ctx, req, result, err) + return Result{}, err } } if resource.GetDeletionTimestamp() != nil { // resource is being deleted, nothing to do - return r.AfterReconcile(ctx, req, result, nil) + return Result{}, nil } - reconcileResult, err := r.Reconciler.Reconcile(ctx, resource) - result = AggregateResults(result, reconcileResult) + result, err := r.Reconciler.Reconcile(ctx, resource) if err != nil && !errors.Is(err, ErrHaltSubReconcilers) { - return r.AfterReconcile(ctx, req, result, err) + return result, err } // hack, ignore track requests from the child reconciler, we have it covered @@ -299,13 +307,10 @@ func (r *AggregateReconciler[T]) Reconcile(ctx context.Context, req Request) (Re }) desired, err := r.desiredResource(ctx, resource) if err != nil { - return r.AfterReconcile(ctx, req, result, err) + return result, err } _, err = r.stamp.Manage(ctx, resource, resource, desired) - if err != nil { - return r.AfterReconcile(ctx, req, result, err) - } - return r.AfterReconcile(ctx, req, result, nil) + return result, err } func (r *AggregateReconciler[T]) desiredResource(ctx context.Context, resource T) (T, error) { diff --git a/reconcilers/resource.go b/reconcilers/resource.go index 3573a45..6325bca 100644 --- a/reconcilers/resource.go +++ b/reconcilers/resource.go @@ -234,28 +234,38 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res ctx = StashOriginalConfig(ctx, c) ctx = StashOriginalResourceType(ctx, r.Type) ctx = StashResourceType(ctx, r.Type) - originalResource := r.Type.DeepCopyObject().(T) - beforeCtx, result, err := r.BeforeReconcile(ctx, req) + beforeCtx, beforeResult, err := r.BeforeReconcile(ctx, req) if err != nil { - return result, err + return beforeResult, err } if beforeCtx != nil { ctx = beforeCtx } + reconcileResult, err := r.reconcileOuter(ctx, req) + + return r.AfterReconcile(ctx, req, AggregateResults(beforeResult, reconcileResult), err) +} + +func (r *ResourceReconciler[T]) reconcileOuter(ctx context.Context, req Request) (Result, error) { + log := logr.FromContextOrDiscard(ctx) + c := RetrieveOriginalConfigOrDie(ctx) + + originalResource := r.Type.DeepCopyObject().(T) + if err := c.Get(ctx, req.NamespacedName, originalResource); err != nil { if apierrs.IsNotFound(err) { // we'll ignore not-found errors, since they can't be fixed by an immediate // requeue (we'll need to wait for a new notification), and we can get them // on deleted requests. - return r.AfterReconcile(ctx, req, result, nil) + return Result{}, nil } if !errors.Is(err, ErrQuiet) { log.Error(err, "unable to fetch resource") } - return r.AfterReconcile(ctx, req, result, err) + return Result{}, err } resource := originalResource.DeepCopyObject().(T) @@ -265,11 +275,10 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res } r.initializeConditions(ctx, resource) - reconcileResult, err := r.reconcile(ctx, resource) - result = AggregateResults(result, reconcileResult) + result, err := r.reconcileInner(ctx, resource) if r.SkipStatusUpdate { - return r.AfterReconcile(ctx, req, result, err) + return result, err } // attempt to restore last transition time for unchanged conditions @@ -288,7 +297,7 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res "Failed to patch status: %v", patchErr) } - return r.AfterReconcile(ctx, req, result, patchErr) + return result, patchErr } c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusPatched", "Patched status") @@ -301,7 +310,7 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusUpdateFailed", "Failed to update status: %v", updateErr) } - return r.AfterReconcile(ctx, req, result, updateErr) + return result, updateErr } c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusUpdated", "Updated status") @@ -312,7 +321,7 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res return result, err } -func (r *ResourceReconciler[T]) reconcile(ctx context.Context, resource T) (Result, error) { +func (r *ResourceReconciler[T]) reconcileInner(ctx context.Context, resource T) (Result, error) { if resource.GetDeletionTimestamp() != nil && len(resource.GetFinalizers()) == 0 { // resource is being deleted and has no pending finalizers, nothing to do return Result{}, nil