reconciler/managed: avoid requeuing if an update event is pending

[sttts]

Description of your changes

We use this:

return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)

This pattern means that the object is immediately (modulo queue delay) reconciled again. We use cached clients (controller-runtime's default). Chance is high that the immediate client.Get call will return a stale object if that Update did a change. We will run all the complex reconcile logic (all the work twice), create noise and all that, events for example, to eventually find out that the world has moved on (through our own update) and our update of the second reconcile iteration will fail with a conflict error (another event).

The right pattern would be to only requeue if the update was a noop (= resource version didn't change), or even just set Requeue: false. If the update did something, we will see a watch event anyway (for watched resources at least).

I have:

• [x] Read and followed Crossplane's contribution process.
• [x] Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

[negz]

So the edge case we'd be susceptible to would be hitting the same error twice in a row. In that case nothing would trigger a reconcile to be queued until the poll interval expired (or a watch event happened).

Actually, we have to hit a case where we return reconcile.Result{RequeueAfter: r.pollInterval}. So I think in theory if we returned reconcile.Result{} in error scenarios there'd be a risk that we'd sit waiting for a watch event (or sync interval) before being requeued.

[turkenh]

How different is this from https://github.com/crossplane/crossplane-runtime/pull/372 ?

[lsviben]

Going through old PRs, is this PR still relevant?