Use predicate to filter reconciles

[karlkfi]

• Don't enqueue a reconcile if the ResourceVersion hasn't changed
• Remove lock and resourceVersion cache that was attempting to dedupe the hard way
• Reduce watch scope from cluster to namespace for Deployments and RootSync Secrets
• Add RootSync watch for managed ClusterRoleBindings (shared), to ensure that changes to the crb are reverted without needing to update another watched object.
• Add RootSync ownership for managed ServiceAccounts, to ensure that changes to the sa are reverted without needing to update another watched object.

[google-oss-prow[bot]]

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

[google-oss-prow[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [karlkfi]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[karlkfi]

/test all

[sdowell]

LGTM but someone else should take a look

[karlkfi]

/hold

It occurs to me that this might not do what I expected.

The reason I initially added the dedupping was because the Reconcile was causing RSync status updates to happen using the same initial generation. They weren't always duplicate updates, because some other robject might have changed, but the GET at the beginning of the Reconcile was returning an RSync with a generation we'd already seen. My assumption was that this was due to the reconciler-manager using the controller-manager's caching client. Apparently the caching client doesn't invalidate the cache on writes, only on watch updates. So if another watched object (like the Deployment) triggered a reconcile, the client cache might still give you the same RSync it had cached, not the new one from the previous reconcile.

But the predicate in this PR doesn't check the RSync in the cache. It only checks that the object that triggered the watch event has changed (old RV != new RV). This may not actually do what the existing deduping code was doing, because the existing code was always deduping the RSync, not the watched object that triggered the reconcile.

So we probably do want this predicate added, but we probably don't want to remove the other dedupping code.

So I'm gonna put this on hold for now. I'll probably need to add tests to confirm it actually does what I expected.

[karlkfi]

Obsolete