Improve template behavior in Kafka Node Pools

[scholzj]

This proposal tries to address the discussion outcome from strimzi/strimzi-kafka-operator#11791 and proposes a change to how we treat situations when the template section is configured in both Kafka and KafkaNodePool resources. This is a breaking change for users who are in this situation. But based on the previous discussion, it seems to be the preferred way going forward, and should be more understandable to users.

[siegenthalerroger]

Is there any reason why the merge would only happen on the top-level properties and not all the way down as per the strategic merge patch semantics from kubernetes?

As a user that would be what I would expect to happen, or the previous behaviour where it was all overriden. The middle ground seems like the worst in terms of communicating the behaviour. I do believe that in general this is a step in the right direction though and I do prefer it to the status quo.

[scholzj]

Is there any reason why the merge would only happen on the top-level properties and not all the way down as per the strategic merge patch semantics from kubernetes?

As a user that would be what I would expect to happen, or the previous behaviour where it was all overriden. The middle ground seems like the worst in terms of communicating the behaviour. I do believe that in general this is a step in the right direction though and I do prefer it to the status quo.

I guess that would make it essentially impossible to unset anything that is set in the Kafka CR template, which would be a blocker. I think it would make it also pretty hard to implement. Dealing with conflicts would be pretty hard as well.

And TBH, for me personally, it would be a very unexpected behavior. (But I might not be the best sample on which to define what would and would not be expected of course)

[siegenthalerroger]

I do concur that unsetting items with SMP (strategic merge patch) is quite annoying but there is a way to do it in kustomize with the $patch: delete or $patch: replace. This seems to be even less of a standard than merging in general already isn't itself so I may be wrong with my assertion of a "normal" user expecting this behaviour. It is however the behaviour that kubectl/kustomize provide.

Ref: https://github.com/kubernetes-sigs/kustomize/blob/ab48be374781b4b38b8a864a5609d168712808c5/examples/inlinePatch.md?plain=1#L105

[ppatierno]

LGTM. It describes what was discussed so I am fine with it.

[PaulRMellor]

Looks good
Proposal shows the benefits of allowing merge in template behavior
Clear docs will be an important aspect of the change

[PaulRMellor]

Thanks for addressing comments.
The addition to rejected alternatives looks good. 👍