Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: added observedGeneration to the Ready and Sync condition #789

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+63 −27
Open

chore: added observedGeneration to the Ready and Sync condition #789

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 63 additions & 27 deletions pkg/reconciler/managed/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
log.Debug("Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation", "annotation", meta.AnnotationKeyReconciliationPaused)
record.Event(managed, event.Normal(reasonReconciliationPaused, "Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation",
"annotation", meta.AnnotationKeyReconciliationPaused))
managed.SetConditions(xpv1.ReconcilePaused())
managed.SetConditions(xpv1.ReconcilePaused().WithObservedGeneration(managed.GetGeneration()))
// 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)
Expand All @@ -900,7 +900,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonManagementPolicyInvalid, err))
managed.SetConditions(xpv1.ReconcileError(err))
managed.SetConditions(xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -925,7 +925,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotUnpublish, err))
managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err))
managed.SetConditions(
xpv1.Deleting().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
if err := r.managed.RemoveFinalizer(ctx, managed); err != nil {
Expand All @@ -937,7 +940,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
if kerrors.IsConflict(err) {
return reconcile.Result{Requeue: true}, nil
}
managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err))
managed.SetConditions(
xpv1.Deleting().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -959,7 +965,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotInitialize, err))
managed.SetConditions(xpv1.ReconcileError(err))
managed.SetConditions(xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -970,7 +976,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
if meta.ExternalCreateIncomplete(managed) {
log.Debug(errCreateIncomplete)
record.Event(managed, event.Warning(reasonCannotInitialize, errors.New(errCreateIncomplete)))
managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.New(errCreateIncomplete)))
managed.SetConditions(
xpv1.Creating().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(errors.New(errCreateIncomplete)).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: false}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -995,7 +1004,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotResolveRefs, err))
managed.SetConditions(xpv1.ReconcileError(err))
managed.SetConditions(xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
}
Expand All @@ -1012,7 +1021,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotConnect, err))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileConnect)))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileConnect)).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
defer func() {
Expand Down Expand Up @@ -1040,15 +1049,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotObserve, err))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileObserve)))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileObserve)).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

// In the observe-only mode, !observation.ResourceExists will be an error
// case, and we will explicitly return this information to the user.
if !observation.ResourceExists && policy.ShouldOnlyObserve() {
record.Event(managed, event.Warning(reasonCannotObserve, errors.New(errExternalResourceNotExist)))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(errors.New(errExternalResourceNotExist), errReconcileObserve)))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(errors.New(errExternalResourceNotExist), errReconcileObserve)).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand Down Expand Up @@ -1086,7 +1095,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
log.Info(errRecordChangeLog, "error", err)
}
record.Event(managed, event.Warning(reasonCannotDelete, err))
managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(errors.Wrap(err, errReconcileDelete)))
managed.SetConditions(
xpv1.Deleting().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(errors.Wrap(err, errReconcileDelete)).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1102,7 +1114,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
log.Info(errRecordChangeLog, "error", err)
}
record.Event(managed, event.Normal(reasonDeleted, "Successfully requested deletion of external resource"))
managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileSuccess())
managed.SetConditions(
xpv1.Deleting().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileSuccess().WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
if err := r.managed.UnpublishConnection(ctx, managed, observation.ConnectionDetails); err != nil {
Expand All @@ -1115,7 +1130,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotUnpublish, err))
managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err))
managed.SetConditions(
xpv1.Deleting().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
if err := r.managed.RemoveFinalizer(ctx, managed); err != nil {
Expand All @@ -1127,7 +1145,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
if kerrors.IsConflict(err) {
return reconcile.Result{Requeue: true}, nil
}
managed.SetConditions(xpv1.Deleting(), xpv1.ReconcileError(err))
managed.SetConditions(
xpv1.Deleting().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1149,7 +1170,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotPublish, err))
managed.SetConditions(xpv1.ReconcileError(err))
managed.SetConditions(xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1161,7 +1182,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
if kerrors.IsConflict(err) {
return reconcile.Result{Requeue: true}, nil
}
managed.SetConditions(xpv1.ReconcileError(err))
managed.SetConditions(xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1180,7 +1201,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotUpdateManaged, errors.Wrap(err, errUpdateManaged)))
managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.Wrap(err, errUpdateManaged)))
managed.SetConditions(
xpv1.Creating().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(errors.Wrap(err, errUpdateManaged)).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand Down Expand Up @@ -1217,7 +1241,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
if err := r.change.Log(ctx, managedPreOp, v1alpha1.OperationType_OPERATION_TYPE_CREATE, err, creation.AdditionalDetails); err != nil {
log.Info(errRecordChangeLog, "error", err)
}
managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.Wrap(err, errReconcileCreate)))
managed.SetConditions(
xpv1.Creating().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(errors.Wrap(err, errReconcileCreate)).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand Down Expand Up @@ -1246,7 +1273,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotUpdateManaged, errors.Wrap(err, errUpdateManagedAnnotations)))
managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(errors.Wrap(err, errUpdateManagedAnnotations)))
managed.SetConditions(
xpv1.Creating().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(errors.Wrap(err, errUpdateManagedAnnotations)).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1259,7 +1289,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
return reconcile.Result{Requeue: true}, nil
}
record.Event(managed, event.Warning(reasonCannotPublish, err))
managed.SetConditions(xpv1.Creating(), xpv1.ReconcileError(err))
managed.SetConditions(
xpv1.Creating().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1269,7 +1302,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
// ready for use.
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())
managed.SetConditions(
xpv1.Creating().WithObservedGeneration(managed.GetGeneration()),
xpv1.ReconcileSuccess().WithObservedGeneration(managed.GetGeneration()),
)
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1284,7 +1320,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
if err := r.client.Update(ctx, managed); err != nil {
log.Debug(errUpdateManaged, "error", err)
record.Event(managed, event.Warning(reasonCannotUpdateManaged, err))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errUpdateManaged)))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errUpdateManaged)).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
}
Expand All @@ -1298,7 +1334,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
// https://github.com/crossplane/crossplane/issues/289
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
log.Debug("External resource is up to date", "requeue-after", time.Now().Add(reconcileAfter))
managed.SetConditions(xpv1.ReconcileSuccess())
managed.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(managed.GetGeneration()))
r.metricRecorder.recordFirstTimeReady(managed)

// record that we intentionally did not update the managed resource
Expand All @@ -1318,7 +1354,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
if !policy.ShouldUpdate() {
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
log.Debug("Skipping update due to managementPolicies. Reconciliation succeeded", "requeue-after", time.Now().Add(reconcileAfter))
managed.SetConditions(xpv1.ReconcileSuccess())
managed.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1334,7 +1370,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
log.Info(errRecordChangeLog, "error", err)
}
record.Event(managed, event.Warning(reasonCannotUpdate, err))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileUpdate)))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileUpdate)).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1350,7 +1386,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
// not, we requeue explicitly, which will trigger backoff.
log.Debug("Cannot publish connection details", "error", err)
record.Event(managed, event.Warning(reasonCannotPublish, err))
managed.SetConditions(xpv1.ReconcileError(err))
managed.SetConditions(xpv1.ReconcileError(err).WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

Expand All @@ -1362,6 +1398,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
log.Debug("Successfully requested update of external resource", "requeue-after", time.Now().Add(reconcileAfter))
record.Event(managed, event.Normal(reasonUpdated, "Successfully requested update of external resource"))
managed.SetConditions(xpv1.ReconcileSuccess())
managed.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(managed.GetGeneration()))
return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}