fix: skip reconciliation when label selector does not match

[porridge]

• This change was originally developed in ROX-19221: Fix reconciling with label selector for multiple reconcilers stackrox/helm-operator#42

Issue

• Assume there is CR with label foo=bar.
• a reconciler with label selector foo=bar processes the CR as expected
• another reconciler, with label selector foo=another, also reconciles foo=bar CR. This is not expected.

Root cause

There is controller.Watch set for Secret kind. This watch has event handler. The Event handler only filters events by GroupVersionKind of the CR so label selector is ignored

Fix

Check for the matching label selector before performing reconcile. Skip reconcile if label is not matching

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.55%. Comparing base (08ab7fb) to head (81c36cb).
Report is 76 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (08ab7fb) and HEAD (81c36cb). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (08ab7fb)	HEAD (81c36cb)
	2	1

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #431      +/-   ##
==========================================
- Coverage   85.06%   78.55%   -6.52%     
==========================================
  Files          19       31      +12     
  Lines        1346     2490    +1144     
==========================================
+ Hits         1145     1956     +811     
- Misses        125      445     +320     
- Partials       76       89      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joelanford]

I'm not sure I'm fully understanding the scenario described in the description. Are you talking about this watch?

It looks like that watch+handler adds items to the workqueue the owner of the secret (which is the CR), but does not check that the owner matches the label selector.

If I'm understanding correctly, another way to implement this would be to have a custom handler that:

• knows about the label selector
• Gets the owner CR from the informer cache
• only adds the request to the workqueue if the owner CR matches the label selector

This would ensure the request never even makes it into the workqueue and would avoid unnecessary reconcile calls (which can mess with controller-runtime metrics and logging in a way that might be undesirable).

We had a similar thing in Rukpak (except based on a "provisionerClassName" match rather than a label selector): https://github.com/operator-framework/rukpak/blob/5ffcfff615566f3a1a482a5f6649cd3e07f2427f/pkg/util/util.go#L52-L108

[porridge]

@joelanford your idea sounds much more elegant. In an attempt to try and distribute the workload I asked @kurlov and he kindly agreed to take a look at implementing this in a week or so.

[openshift-merge-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[kurlov]

@joelanford I have opened a separate PR #443.
But I decided to change the secret watch because there might be a potential bug. ATM running two reconcilers with the same GVK but different label selectors make all of them to reconcile each other CRs ignoring provided label selectors. If this is not a valid scenario then I will change the implementation to what you have suggested initially

[porridge]

Superseded by #443