Implement strategic merge patch (#62)

[hieu2102]

Implements #62

• add field Containers to ValkeyClusterSpec
• add function deployment.mergePatchContainers
• modify the function signatures of deployment.generateContainersDef and deployment.createClusterDeployment to return error in addition to the output object
• add tests

[jdheyburn]

This looks very straight forward to me. I've not tested it locally.

Should we update ExporterSpec such that it only supports enabled? Since all customisation would now be handled via the new Containers merge?

[bjosv]

Should we update ExporterSpec such that it only supports enabled? Since all customisation would now be handled via the new Containers merge?

Unless they are convenient and commonly changed configs. I guess there is nothing blocking a user to modify "valkey-server" in the same way, to set cluster.Spec.Resources using the merge patch function instead.

[jdheyburn]

@bjosv True, nothing stops the user doing that. Perhaps we can support these top-level fields for the time being.

[sandeepkunusoth]

Just curious, some other operator uses podTemplateSpec, which is useful when users need volumes, initContainers(maybe example usecase where we are reading acl secrets from vault etc)

[jdheyburn]

@sandeepkunusoth I would image we would expose those fields (Volumes, etc.) at the root level similar to what Prometheus Operator does: https://github.com/prometheus-operator/prometheus-operator/blob/f3c143e5499e020744d57ba8a94045af71bf1a83/pkg/client/applyconfiguration/monitoring/v1/commonprometheusfields.go#L28

What example operators are you referencing?

[sandeepkunusoth]

@sandeepkunusoth I would image we would expose those fields (Volumes, etc.) at the root level similar to what Prometheus Operator does: https://github.com/prometheus-operator/prometheus-operator/blob/f3c143e5499e020744d57ba8a94045af71bf1a83/pkg/client/applyconfiguration/monitoring/v1/commonprometheusfields.go#L28

What example operators are you referencing?

https://github.com/k8ssandra/cass-operator#:~:text=podTemplateSpec%3A%0A%20%20%20%20securityContext%3A%20%7B%7D%0A%20%20%20%20containers%3A%0A%20%20%20%20%20%20%2D%20name%3A%20cassandra%0A%20%20%20%20%20%20%20%20securityContext%3A%20%7B%7D

we can use containers maybe for now

[jdheyburn]

I think podTemplateSpec and containerSpec are indifferent to each other, and we have already started building on this model (we have the PR ready here). I like the idea of our operator providing some abstraction away from the pod template that we're writing on their behalf. Any fields not included in the CRD we can look to expose ourselves.

I appreciate the call out though @sandeepkunusoth

[jdheyburn]

Thanks for raising this @hieu2102!

[jdheyburn]

@hieu2102 can you try pushing a dummy commit so to retrigger the CI builds? Seems there is some hangover from a Github issue last week

git commit --allow-empty -m "chore: trigger CI" --signoff
git push

[hieu2102]

@jdheyburn done