Skip to content

Commit bab993d

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

File tree

5 files changed

+119
-32
lines changed

5 files changed

+119
-32
lines changed

apis/common/condition.go

Lines changed: 68 additions & 1 deletion
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.
@@ -130,7 +142,7 @@ func (c Condition) WithObservedGeneration(gen int64) Condition {
130142
// Crossplane system (e.g, Ready, Synced, Healthy).
131143
func IsSystemConditionType(t ConditionType) bool {
132144
switch t {
133-
case TypeReady, TypeSynced, TypeHealthy:
145+
case TypeReady, TypeSynced, TypeHealthy, TypeUpToDate:
134146
return true
135147
}
136148

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

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: 5 additions & 5 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)
@@ -1436,7 +1436,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
14361436
// https://github.com/crossplane/crossplane/issues/289
14371437
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
14381438
log.Debug("External resource is up to date", "requeue-after", time.Now().Add(reconcileAfter))
1439-
status.MarkConditions(xpv1.ReconcileSuccess())
1439+
status.MarkConditions(xpv1.ReconcileSuccess(), xpv1.ObserveMatch())
14401440
r.metricRecorder.recordFirstTimeReady(managed)
14411441

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

14611461
return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
14621462
}
@@ -1475,7 +1475,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
14751475
}
14761476

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

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

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

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)