Add Pod topology spread constraints chart value

fkrestan commented 1 year ago

Resolves #260

spencergilbert commented 1 year ago

Thanks for the contribution! I merged some changes that will probably need to be merged in/resolved against your changes.

I also agree with your comment with regards to "selectorLabels", I'll see what I can think of.

fkrestan commented 1 year ago

I merged some changes that will probably need to be merged in/resolved against your changes.

I've re-based against the latest develop and updated the docstring to match the new style.

spencergilbert commented 1 year ago

Great, thank you! I'm aiming to play around locally and get this reviewed/merged by the end of the week.

spencergilbert commented 1 year ago

@fkrestan do we need to check that this hasn't been set if the user is deploying a Daemonset? Would that error, or just be ignored by the API?

fkrestan commented 1 year ago

@fkrestan do we need to check that this hasn't been set if the user is deploying a Daemonset? Would that error, or just be ignored by the API?

I've tested this and it seems to be ignored at least in effect:

The spread constraint gets applied and doesn't get stripped form the DaemonSet spec. topologySpreadConstraints is part of the Pod API spec which is embedded in the DaemonSet so it makes sense that this passes API validation.
Looking at the kubectl describe output, it seems to be applied and respected. I don't see any complaints in Kubernetes API nor controller manager logs.
Looking at the documentation, it doesn't seem possible to create a constraint that would not get satisfiedg by DaemonSet.

IMO there is no need to put any checks in place.

vectordotdev / helm-charts