diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index 3723b5ceb..76ee237d4 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -710,6 +710,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot get managed resource", "error", err) return reconcile.Result{}, errors.Wrap(resource.IgnoreNotFound(err), errGetManaged) } + orig := managed.DeepCopyObject().(resource.Managed) record := r.record.WithAnnotations("external-name", meta.GetExternalName(managed)) log = log.WithValues( @@ -738,7 +739,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu managed.SetConditions(xpv1.ReconcilePaused()) // if the pause annotation is removed or the management policies changed, we will have a chance to reconcile // again and resume and if status update fails, we will reconcile again to retry to update the status - return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // Check if the ManagementPolicies is set to a non-default value while the @@ -753,7 +754,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug(err.Error()) record.Event(managed, event.Warning(reasonManagementPolicyInvalid, err)) managed.SetConditions(xpv1.ReconcileError(err)) - return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // If managed resource has a deletion timestamp and a deletion policy of @@ -775,7 +776,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot unpublish connection details", "error", err) record.Event(managed, event.Warning(reasonCannotUnpublish, err)) managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if err := r.managed.RemoveFinalizer(ctx, managed); err != nil { // If this is the first time we encounter this issue we'll be @@ -784,7 +785,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // backoff. log.Debug("Cannot remove managed resource finalizer", "error", err) managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // We've successfully unpublished our managed resource's connection @@ -802,7 +803,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot initialize managed resource", "error", err) record.Event(managed, event.Warning(reasonCannotInitialize, err)) managed.SetConditions(xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // If we started but never completed creation of an external resource we @@ -813,7 +814,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug(errCreateIncomplete) record.Event(managed, event.Warning(reasonCannotInitialize, errors.New(errCreateIncomplete))) managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.New(errCreateIncomplete))) - return reconcile.Result{Requeue: false}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: false}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // We resolve any references before observing our external resource because @@ -835,7 +836,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot resolve managed resource references", "error", err) record.Event(managed, event.Warning(reasonCannotResolveRefs, err)) managed.SetConditions(xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } } @@ -849,7 +850,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot connect to provider", "error", err) record.Event(managed, event.Warning(reasonCannotConnect, err)) managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileConnect))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } defer func() { if err := r.external.Disconnect(ctx); err != nil { @@ -869,7 +870,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot observe external resource", "error", err) record.Event(managed, event.Warning(reasonCannotObserve, err)) managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileObserve))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // In the observe-only mode, !observation.ResourceExists will be an error @@ -877,7 +878,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu if !observation.ResourceExists && policy.ShouldOnlyObserve() { record.Event(managed, event.Warning(reasonCannotObserve, errors.New(errExternalResourceNotExist))) managed.SetConditions(xpv1.ReconcileError(errors.Wrap(errors.New(errExternalResourceNotExist), errReconcileObserve))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // If this resource has a non-zero creation grace period we want to wait @@ -888,7 +889,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu if !observation.ResourceExists && meta.ExternalCreateSucceededDuring(managed, r.creationGracePeriod) { log.Debug("Waiting for external resource existence to be confirmed") record.Event(managed, event.Normal(reasonPending, "Waiting for external resource existence to be confirmed")) - return reconcile.Result{Requeue: true}, nil + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, nil) } if meta.WasDeleted(managed) { @@ -905,7 +906,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot delete external resource", "error", err) record.Event(managed, event.Warning(reasonCannotDelete, err)) managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(errors.Wrap(err, errReconcileDelete))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // We've successfully requested deletion of our external resource. @@ -918,7 +919,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Successfully requested deletion of external resource") record.Event(managed, event.Normal(reasonDeleted, "Successfully requested deletion of external resource")) managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileSuccess()) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if err := r.managed.UnpublishConnection(ctx, managed, observation.ConnectionDetails); err != nil { // If this is the first time we encounter this issue we'll be @@ -928,7 +929,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot unpublish connection details", "error", err) record.Event(managed, event.Warning(reasonCannotUnpublish, err)) managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if err := r.managed.RemoveFinalizer(ctx, managed); err != nil { // If this is the first time we encounter this issue we'll be @@ -937,7 +938,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // backoff. log.Debug("Cannot remove managed resource finalizer", "error", err) managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // We've successfully deleted our external resource (if necessary) and @@ -945,7 +946,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // added a finalizer to this resource then it should no longer exist and // thus there is no point trying to update its status. log.Debug("Successfully deleted managed resource") - return reconcile.Result{Requeue: false}, nil + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: false}, nil) } if _, err := r.managed.PublishConnection(ctx, managed, observation.ConnectionDetails); err != nil { @@ -955,7 +956,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot publish connection details", "error", err) record.Event(managed, event.Warning(reasonCannotPublish, err)) managed.SetConditions(xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if err := r.managed.AddFinalizer(ctx, managed); err != nil { @@ -964,7 +965,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // not, we requeue explicitly, which will trigger backoff. log.Debug("Cannot add finalizer", "error", err) managed.SetConditions(xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if !observation.ResourceExists && policy.ShouldCreate() { @@ -980,7 +981,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug(errUpdateManaged, "error", err) record.Event(managed, event.Warning(reasonCannotUpdateManaged, errors.Wrap(err, errUpdateManaged))) managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.Wrap(err, errUpdateManaged))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } creation, err := external.Create(externalCtx, managed) @@ -1012,7 +1013,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu } managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.Wrap(err, errReconcileCreate))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // In some cases our external-name may be set by Create above. @@ -1034,7 +1035,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug(errUpdateManagedAnnotations, "error", err) record.Event(managed, event.Warning(reasonCannotUpdateManaged, errors.Wrap(err, errUpdateManagedAnnotations))) managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.Wrap(err, errUpdateManagedAnnotations))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if _, err := r.managed.PublishConnection(ctx, managed, creation.ConnectionDetails); err != nil { @@ -1044,7 +1045,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot publish connection details", "error", err) record.Event(managed, event.Warning(reasonCannotPublish, err)) managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // We've successfully created our external resource. In many cases the @@ -1054,7 +1055,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Successfully requested creation of external resource") record.Event(managed, event.Normal(reasonCreated, "Successfully requested creation of external resource")) managed.SetConditions(xpv1.Creating(), xpv1.ReconcileSuccess()) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if observation.ResourceLateInitialized && policy.ShouldLateInitialize() { @@ -1069,7 +1070,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug(errUpdateManaged, "error", err) record.Event(managed, event.Warning(reasonCannotUpdateManaged, err)) managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errUpdateManaged))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } } @@ -1082,7 +1083,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // https://github.com/crossplane/crossplane/issues/289 log.Debug("External resource is up to date", "requeue-after", time.Now().Add(r.pollInterval)) managed.SetConditions(xpv1.ReconcileSuccess()) - return reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if observation.Diff != "" { @@ -1093,7 +1094,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu if !policy.ShouldUpdate() { log.Debug("Skipping update due to managementPolicies. Reconciliation succeeded", "requeue-after", time.Now().Add(r.pollInterval)) managed.SetConditions(xpv1.ReconcileSuccess()) - return reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } update, err := external.Update(externalCtx, managed) @@ -1106,7 +1107,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot update external resource") record.Event(managed, event.Warning(reasonCannotUpdate, err)) managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileUpdate))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } if _, err := r.managed.PublishConnection(ctx, managed, update.ConnectionDetails); err != nil { @@ -1116,7 +1117,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot publish connection details", "error", err) record.Event(managed, event.Warning(reasonCannotPublish, err)) managed.SetConditions(xpv1.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) } // We've successfully updated our external resource. Per the below issue @@ -1127,5 +1128,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Successfully requested update of external resource", "requeue-after", time.Now().Add(r.pollInterval)) record.Event(managed, event.Normal(reasonUpdated, "Successfully requested update of external resource")) managed.SetConditions(xpv1.ReconcileSuccess()) - return reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return skipRequeueOnUpdate(orig, managed, reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)) +} + +func skipRequeueOnUpdate(orig, o resource.Managed, result reconcile.Result, err error) (reconcile.Result, error) { + if err == nil && orig.GetResourceVersion() != o.GetResourceVersion() { + // the object has changed. We get a watch event and do not need to + // requeue. This helps to avoid a reconcile on a stale read when the + // informer has not caught up. + result.Requeue = false + } + return result, err }