Skip to content

Commit d2a5440

Browse files
bobh66bobh66
committed
Add UpToDate condition for managed resources
Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
1 parent 8fa945b commit d2a5440

File tree

5 files changed

+123
-39
lines changed

5 files changed

+123
-39
lines changed

‎apis/common/condition.go‎

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ const (
4545
// as a system condition. See the tracking issue for more details
4646
// https://github.com/crossplane/crossplane/issues/5643.
4747
TypeHealthy ConditionType = "Healthy"
48+
49+
// TypeUpToDate resources are believed to accurately represent the remote resource state.
50+
// The condition is controlled by the ResourceUpToDate return value from the Observe method.
51+
TypeUpToDate ConditionType = "UpToDate"
4852
)
4953

5054
// A ConditionReason represents the reason a resource is in a condition.
@@ -65,6 +69,14 @@ const (
6569
ReasonReconcilePaused ConditionReason = "ReconcilePaused"
6670
)
6771

72+
// Reasons a resource is or is not up to date.
73+
const (
74+
ReasonUpdateRestricted ConditionReason = "UpdateRestricted"
75+
ReasonObserveMatch ConditionReason = "ObserveMatch"
76+
ReasonUpdateFailed ConditionReason = "UpdateFailed"
77+
ReasonUpdateRequested ConditionReason = "UpdateRequested"
78+
)
79+
6880
// See https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
6981

7082
// A Condition that may apply to a resource.
@@ -96,8 +108,7 @@ type Condition struct { //nolint:recvcheck // False positive - only has non-poin
96108
}
97109

98110
// Equal returns true if the condition is identical to the supplied condition,
99-
// ignoring the LastTransitionTime. If one or both conditions have not
100-
// provided the ObservedGeneration it is not considered in the comparison.
111+
// ignoring the LastTransitionTime.
101112
func (c Condition) Equal(other Condition) bool {
102113
if c.ObservedGeneration == 0 || other.ObservedGeneration == 0 {
103114
return c.Type == other.Type &&
@@ -130,7 +141,7 @@ func (c Condition) WithObservedGeneration(gen int64) Condition {
130141
// Crossplane system (e.g, Ready, Synced, Healthy).
131142
func IsSystemConditionType(t ConditionType) bool {
132143
switch t {
133-
case TypeReady, TypeSynced, TypeHealthy:
144+
case TypeReady, TypeSynced, TypeHealthy, TypeUpToDate:
134145
return true
135146
}
136147

@@ -312,3 +323,58 @@ func ReconcilePaused() Condition {
312323
Reason: ReasonReconcilePaused,
313324
}
314325
}
326+
327+
// ObserveMatch returns a condition that indicates the Observe
328+
// method returned true.
329+
func ObserveMatch() Condition {
330+
return Condition{
331+
Type: TypeUpToDate,
332+
Status: corev1.ConditionTrue,
333+
LastTransitionTime: metav1.Now(),
334+
Reason: ReasonObserveMatch,
335+
}
336+
}
337+
338+
// UpdateRequested returns a condition that indicates the Observe
339+
// method returned false and the Update method was called successfully.
340+
func UpdateRequested() Condition {
341+
return Condition{
342+
Type: TypeUpToDate,
343+
Status: corev1.ConditionFalse,
344+
LastTransitionTime: metav1.Now(),
345+
Reason: ReasonUpdateRequested,
346+
}
347+
}
348+
349+
// UpdateRestricted returns a condition that indicates the Observe
350+
// method returned false and the Update method is not allowed.
351+
func UpdateRestricted() Condition {
352+
return Condition{
353+
Type: TypeUpToDate,
354+
Status: corev1.ConditionFalse,
355+
LastTransitionTime: metav1.Now(),
356+
Reason: ReasonUpdateRestricted,
357+
}
358+
}
359+
360+
// UpdateFailed returns a condition that indicates the Observe
361+
// method returned false and the Update method was unsuccessful in synchronizing the resource state.
362+
func UpdateFailed() Condition {
363+
return Condition{
364+
Type: TypeUpToDate,
365+
Status: corev1.ConditionFalse,
366+
LastTransitionTime: metav1.Now(),
367+
Reason: ReasonUpdateFailed,
368+
}
369+
}
370+
371+
// PausedUnknown returns a condition that indicates the Observe
372+
// method was not called because reconciliation is paused so the status is unknown.
373+
func PausedUnknown() Condition {
374+
return Condition{
375+
Type: TypeUpToDate,
376+
Status: corev1.ConditionUnknown,
377+
LastTransitionTime: metav1.Now(),
378+
Reason: ReasonReconcilePaused,
379+
}
380+
}

‎apis/common/v1/condition.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,23 @@ func ReconcileError(err error) Condition {
134134
func ReconcilePaused() Condition {
135135
return common.ReconcilePaused()
136136
}
137+
138+
// ObserveMatch returns a condition indicating that the managed resource desired state matches
139+
// the external resource state.
140+
func ObserveMatch() Condition { return common.ObserveMatch() }
141+
142+
// PausedUnknown returns a condition that indicates reconciliation is paused and it is unknown if the desired
143+
// state of the managed resource matches the external resource state.
144+
func PausedUnknown() Condition { return common.PausedUnknown() }
145+
146+
// UpdateRequested returns a condition that indicates a mismatch between the managed resource and the
147+
// external resource was detected and an update of the external resource has been requested.
148+
func UpdateRequested() Condition { return common.UpdateRequested() }
149+
150+
// UpdateFailed returns a condition that indicates a mismatch between the managed resource and the
151+
// external resource was detected and an updated of the external resource was attempted and failed.
152+
func UpdateFailed() Condition { return common.UpdateFailed() }
153+
154+
// UpdateRestricted returns a condition that indicates a mismatch between the managed resource and the
155+
// external resource was detected but an Update was not attempted due to managementPolicy restrictions.
156+
func UpdateRestricted() Condition { return common.UpdateRestricted() }

‎pkg/reconciler/managed/reconciler.go‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -939,7 +939,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
939939
log.Debug("Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation", "annotation", meta.AnnotationKeyReconciliationPaused)
940940
record.Event(managed, event.Normal(reasonReconciliationPaused, "Reconciliation is paused either through the `spec.managementPolicies` or the pause annotation",
941941
"annotation", meta.AnnotationKeyReconciliationPaused))
942-
status.MarkConditions(xpv1.ReconcilePaused())
942+
status.MarkConditions(xpv1.ReconcilePaused(), xpv1.PausedUnknown())
943943
// if the pause annotation is removed or the management policies changed, we will have a chance to reconcile
944944
// again and resume and if status update fails, we will reconcile again to retry to update the status
945945
return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
@@ -1107,11 +1107,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
11071107
record.Event(managed, event.Warning(reasonCannotDisconnect, err))
11081108
}
11091109

1110-
if external != nil {
1111-
if err := external.Disconnect(ctx); err != nil {
1112-
log.Debug("Cannot disconnect from provider", "error", err)
1113-
record.Event(managed, event.Warning(reasonCannotDisconnect, err))
1114-
}
1110+
if err := external.Disconnect(ctx); err != nil {
1111+
log.Debug("Cannot disconnect from provider", "error", err)
1112+
record.Event(managed, event.Warning(reasonCannotDisconnect, err))
11151113
}
11161114
}()
11171115

@@ -1436,7 +1434,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
14361434
// https://github.com/crossplane/crossplane/issues/289
14371435
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
14381436
log.Debug("External resource is up to date", "requeue-after", time.Now().Add(reconcileAfter))
1439-
status.MarkConditions(xpv1.ReconcileSuccess())
1437+
status.MarkConditions(xpv1.ReconcileSuccess(), xpv1.ObserveMatch())
14401438
r.metricRecorder.recordFirstTimeReady(managed)
14411439

14421440
// record that we intentionally did not update the managed resource
@@ -1456,7 +1454,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
14561454
if !policy.ShouldUpdate() {
14571455
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
14581456
log.Debug("Skipping update due to managementPolicies. Reconciliation succeeded", "requeue-after", time.Now().Add(reconcileAfter))
1459-
status.MarkConditions(xpv1.ReconcileSuccess())
1457+
status.MarkConditions(xpv1.ReconcileSuccess(), xpv1.UpdateRestricted().WithMessage(observation.Diff))
14601458

14611459
return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
14621460
}
@@ -1475,7 +1473,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
14751473
}
14761474

14771475
record.Event(managed, event.Warning(reasonCannotUpdate, err))
1478-
status.MarkConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileUpdate)))
1476+
status.MarkConditions(xpv1.ReconcileError(errors.Wrap(err, errReconcileUpdate)), xpv1.UpdateFailed().WithMessage(observation.Diff))
14791477

14801478
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
14811479
}
@@ -1506,7 +1504,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
15061504
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
15071505
log.Debug("Successfully requested update of external resource", "requeue-after", time.Now().Add(reconcileAfter))
15081506
record.Event(managed, event.Normal(reasonUpdated, "Successfully requested update of external resource"))
1509-
status.MarkConditions(xpv1.ReconcileSuccess())
1507+
status.MarkConditions(xpv1.ReconcileSuccess(), xpv1.UpdateRequested().WithMessage(observation.Diff))
15101508

15111509
return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
15121510
}

‎pkg/reconciler/managed/reconciler_legacy_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func TestReconciler(t *testing.T) {
351351
MockGet: legacyManagedMockGetFn(nil, 42),
352352
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
353353
want := newLegacyManaged(42)
354-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
354+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.ObserveMatch().WithObservedGeneration(42))
355355

356356
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
357357
reason := "A successful no-op reconcile should be reported as a conditioned status."
@@ -1088,7 +1088,7 @@ func TestReconciler(t *testing.T) {
10881088
MockGet: legacyManagedMockGetFn(nil, 42),
10891089
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
10901090
want := newLegacyManaged(42)
1091-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
1091+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.ObserveMatch().WithObservedGeneration(42))
10921092

10931093
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
10941094
reason := "A successful no-op reconcile should be reported as a conditioned status."
@@ -1253,7 +1253,7 @@ func TestReconciler(t *testing.T) {
12531253
MockGet: legacyManagedMockGetFn(nil, 42),
12541254
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
12551255
want := newLegacyManaged(42)
1256-
want.SetConditions(xpv1.ReconcileError(errors.Wrap(errBoom, errReconcileUpdate)).WithObservedGeneration(42))
1256+
want.SetConditions(xpv1.ReconcileError(errors.Wrap(errBoom, errReconcileUpdate)).WithObservedGeneration(42), xpv1.UpdateFailed().WithObservedGeneration(42))
12571257

12581258
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
12591259
reason := "Errors while updating an external resource should be reported as a conditioned status."
@@ -1354,7 +1354,7 @@ func TestReconciler(t *testing.T) {
13541354
MockGet: legacyManagedMockGetFn(nil, 42),
13551355
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
13561356
want := newLegacyManaged(42)
1357-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
1357+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.UpdateRequested().WithObservedGeneration(42))
13581358

13591359
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
13601360
reason := "A successful managed resource update should be reported as a conditioned status."
@@ -1398,7 +1398,7 @@ func TestReconciler(t *testing.T) {
13981398
MockGet: legacyManagedMockGetFn(nil, 42),
13991399
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
14001400
want := newLegacyManaged(42)
1401-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
1401+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.UpdateRequested().WithObservedGeneration(42))
14021402

14031403
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
14041404
reason := "A successful managed resource update should be reported as a conditioned status."
@@ -1450,7 +1450,7 @@ func TestReconciler(t *testing.T) {
14501450
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
14511451
want := newLegacyManaged(42)
14521452
want.SetAnnotations(map[string]string{meta.AnnotationKeyReconciliationPaused: "true"})
1453-
want.SetConditions(xpv1.ReconcilePaused().WithObservedGeneration(42))
1453+
want.SetConditions(xpv1.ReconcilePaused().WithObservedGeneration(42), xpv1.PausedUnknown().WithObservedGeneration(42))
14541454

14551455
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
14561456
reason := `If managed resource has the pause annotation with value "true", it should acquire "Synced" status condition with the status "False" and the reason "ReconcilePaused".`
@@ -1480,7 +1480,7 @@ func TestReconciler(t *testing.T) {
14801480
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
14811481
want := newLegacyManaged(42)
14821482
want.SetManagementPolicies(xpv1.ManagementPolicies{})
1483-
want.SetConditions(xpv1.ReconcilePaused().WithObservedGeneration(42))
1483+
want.SetConditions(xpv1.ReconcilePaused().WithObservedGeneration(42), xpv1.PausedUnknown().WithObservedGeneration(42))
14841484

14851485
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
14861486
reason := `If managed resource has the pause annotation with value "true", it should acquire "Synced" status condition with the status "False" and the reason "ReconcilePaused".`
@@ -1516,7 +1516,7 @@ func TestReconciler(t *testing.T) {
15161516
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
15171517
want := newLegacyManaged(42)
15181518
want.SetAnnotations(map[string]string{meta.AnnotationKeyReconciliationPaused: "false"})
1519-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
1519+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.ObserveMatch().WithObservedGeneration(42))
15201520

15211521
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
15221522
reason := `Managed resource should acquire Synced=False/ReconcileSuccess status condition after a resume.`
@@ -1778,7 +1778,7 @@ func TestReconciler(t *testing.T) {
17781778
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
17791779
want := newLegacyManaged(42)
17801780
want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionObserve})
1781-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42).WithObservedGeneration(42))
1781+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42).WithObservedGeneration(42), xpv1.UpdateRestricted().WithObservedGeneration(42))
17821782

17831783
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
17841784
reason := "With ObserveOnly, a successful managed resource observation should be reported as a conditioned status."
@@ -1915,7 +1915,7 @@ func TestReconciler(t *testing.T) {
19151915
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
19161916
want := newLegacyManaged(42)
19171917
want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionObserve, xpv1.ManagementActionLateInitialize, xpv1.ManagementActionCreate, xpv1.ManagementActionDelete})
1918-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
1918+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.UpdateRestricted().WithObservedGeneration(42))
19191919

19201920
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
19211921
reason := `Managed resource should acquire Synced=False/ReconcileSuccess status condition.`
@@ -1966,7 +1966,7 @@ func TestReconciler(t *testing.T) {
19661966
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
19671967
want := newLegacyManaged(42)
19681968
want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionAll})
1969-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42).WithObservedGeneration(42))
1969+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42).WithObservedGeneration(42), xpv1.UpdateRequested().WithObservedGeneration(42))
19701970

19711971
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
19721972
reason := "A successful managed resource update should be reported as a conditioned status."
@@ -2017,7 +2017,7 @@ func TestReconciler(t *testing.T) {
20172017
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
20182018
want := newLegacyManaged(42)
20192019
want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionAll})
2020-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
2020+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.UpdateRequested().WithObservedGeneration(42))
20212021

20222022
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
20232023
reason := "A successful managed resource update should be reported as a conditioned status."
@@ -2069,7 +2069,7 @@ func TestReconciler(t *testing.T) {
20692069
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
20702070
want := newLegacyManaged(42)
20712071
want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionObserve, xpv1.ManagementActionUpdate, xpv1.ManagementActionCreate, xpv1.ManagementActionDelete})
2072-
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
2072+
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.ObserveMatch().WithObservedGeneration(42))
20732073

20742074
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
20752075
reason := "Errors updating a managed resource should be reported as a conditioned status."

0 commit comments

Comments
 (0)