Skip to content

Draft: Add UpToDate condition for managed resources#942

Open
bobh66 wants to merge 1 commit intocrossplane:mainfrom
nokia:uptodate
Open

Draft: Add UpToDate condition for managed resources#942
bobh66 wants to merge 1 commit intocrossplane:mainfrom
nokia:uptodate

Conversation

@bobh66
Copy link
Contributor

@bobh66 bobh66 commented Mar 16, 2026

Description of your changes

Adds a new UpToDate condition (name is subject to better suggestions) that exposes the ResourceUpToDate status from the Observe() call. This allows users to know whether the ManagedResource desired state matches the external resource, and whether an update has been requested, failed, or restricted by management policies.

There are five basic (non-error condition) scenarios:

  • Reconciliation is paused so the condition status is Unknown
  • The resource is up to date with the remote resource so the condition is True
  • The resource has differences from the remote resource and an Update() call has failed, the condition is False
  • The resource has differences from the remote resource and an Update() call cannot be made due to restrictions from managementPolicies, the condition is False
  • The resource has differences from the remote resource and Update() returned success - the condition is False because we can't know if the update is actually asynchronous and may yet fail, but if it succeeds the next reconciliation will show the changes from the update and the condition should become True.

The three False condition scenarios will include the contents of the observation.Diff if it is available.

This is important when importing existing resources to be able to be sure that Crossplane will not make any enexpected changes to the external resource.

Fixes #939

I have:

Need help with this checklist? See the cheat sheet.

@bobh66 bobh66 requested a review from a team as a code owner March 16, 2026 18:12
@bobh66 bobh66 requested a review from jbw976 March 16, 2026 18:12
// Equal returns true if the condition is identical to the supplied condition,
// ignoring the LastTransitionTime. If one or both conditions have not
// provided the ObservedGeneration it is not considered in the comparison.
// ignoring the LastTransitionTime.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops - left over from previous changes. Must not have cleaned up properly. I'll remove.

record.Event(managed, event.Warning(reasonCannotDisconnect, err))
}

if external != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change? is this safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
want := newLegacyManaged(42)
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42))
want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42), xpv1.ObserveMatch().WithObservedGeneration(42))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like we ever assert on the diff message in any of these tests -- could/should we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should for the three cases where it can happen. I'll have to look into how we can inject specific diffs, since it isn't finding any now.

Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose the ResourceUpToDate status from Observe in the MR status/conditions

2 participants