shawkins
commented
9 months ago if the matchers are turned off in some very rare cases this might cause a loop in the reconciliation (in combination with the resources controller, like Deployment)
Would you just consider adding java docs to the ResourceUpdaterMatcher.matches and KubernetesDependentResource.match methods warning about this scenario?
The only simple alternative I can think of would be make the previous value based upon a hash rather than the resourceVersion. That will make it stable (as long as the serialized form of the resource is stable), but will be more intensive to compute / compare.
When a KubernetesDependentResource makes an update, it adds an annotation with the actual version of the resource:
https://github.com/operator-framework/java-operator-sdk/blob/6cab68b53829befa4755742fe0155878f5964bb9/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java#L187-L192
This is then used to if an event comes it could be skipped (won't trigger the reconciliation) since it is a result of the controller. This halves the reconciliation execution with the same result. The actual setup that uses
matchersworks well. No functional issue.The
matchers(SSA based matcher by default) are however an optimization, that other controller might or might do, also there are some special cases that are hard to handle. So in case the matcher simply returns false, and the reconciliation is triggered, while the controller of the dependent resource (not our controller) made changes, the update done by the dependent resource (with the changed resources version in the annotation) will trigger an update (therefore an update event in the informer). While this will be filtered out, it would not create an event (thus there would not be a change (also noresourceVersionchange) if the annotation with resource version is not changed.This causes a real problem just in the case that the controller of the resource is reacting on every change, a typical (and not found more yet) is the
Deployment, where there is somehow an exceptional functionality if the annotations are changed thegenerationis changed in the metadata. What triggers a reconciliation of theDeployment, that results in a change ofobservedGeneration. Which again triggers the reconciliation on our side. (If the matchers are in place this not causes an infinite loop).So there are things here:
matchersare turned off in some very rare cases this might cause a loop in the reconciliation (in combination with the resources controller, likeDeployment)Note that there is a feature flag there to not use this annotation.
Should we do anything regarding this? Well would be nicer to have a simple algorithm without the annotation. The previous algorithm was more complex but did not use the annotation. So we might revisit this and see if we can do better.