refactor: optimize retrieval of secondary resource

[metacosm]

Signed-off-by: Chris Laprun [email protected]

[csviri]

just putting here for the record related stuff:

https://github.com/operator-framework/java-operator-sdk/pull/2001 https://github.com/operator-framework/java-operator-sdk/issues/1999 https://github.com/operator-framework/java-operator-sdk/discussions/1995

So basically this was discussed in context of KeyCloak operator, and we agreed on this approach.

[csviri]

I was thinking more about this, and here I'm probably not right:

Practical: the change from this PR would simply not work in this scenario: the config map. (as in the test) is created prior the dependent resource is reconciled. From the PR the logic would recognize that the config map is there so won't create / update it. Now we delete the primary resource, since we did not create/update the resource the owner reference it not added, so the config map is not deleted. This is inconsistent with the current and desireg behavior, so simply wrong.

So basically update would not match and would add the owner reference.

So having this directly from cache could work as in this PR, but it is hard to be backwards compatible with this feature - since we using getSecondaryResource not only "read the resource" but read "related resources".

Maybe this we could discuss further also on the community meeting.

( TBH the create / update disctinguising does not make that much sense in the context of SSA, so there is bit logical issue also there. )

[metacosm]

just putting here for the record related stuff:

#2001 #1999 #1995

So basically this was discussed in context of KeyCloak operator, and we agreed on this approach.

I guess I didn't follow that discussion that closely then. I don't understand how the owner reference not being present would prevent the resource from being updated. The failing test in this PR shows that this isn't an issue: the ConfigMap that is manually created is properly updated by the dependent resource once JOSDK notices that it doesn't match the desired state.

Maybe we should check that there is not an owner reference with controller set to true and then fail the workflow in that case if an update is required because the reconciler cannot make an appropriate decision in that case: a resource exists that doesn't match the desired state but that is controlled by a different operator. This requires a human intervention to decide what needs to be done in that case, imo.

[csviri]

Was thinking more about this, and actually, what we could do is do this way as you propose when that feature flag createResourceOnlyIfNotExistingWithSSA is set to false. And do as now (so with getSecondaryResources if this is true.)

[metacosm]

Somewhat related: another example of the getSecondaryResources behavior being confusing: https://github.com/quarkiverse/quarkus-operator-sdk/issues/1161. There have been several such instances since the v5 update. In my opinion, the createResourceOnlyIfNotExistingWithSSA should be false by default.

[csviri]

Somewhat related: another example of the getSecondaryResources behavior being confusing: quarkiverse/quarkus-operator-sdk#1161. There have been several such instances since the v5 update. In my opinion, the createResourceOnlyIfNotExistingWithSSA should be false by default.

That is probably related to this https://github.com/operator-framework/java-operator-sdk/pull/2881 not v5