Improve ES pods comparison using a hash of the spec

[sebgl]

Compare the entire pod while looking for a matching pod, instead of just looking at a few fields.

Comparison is performed this way:

ObjectMeta

• compare labels and annotations: we make sure that the actual pod contains all labels and annotations (keys and values) specified in the spec. It is OK for the actual pod to contain additional labels and annotations: they may have been set by the user after initial creation and should not force a replacement of the pod.

PodSpec

• compute a hash of the pretty-printed pod spec before it is created, then annotate the pod with that has right before creation. This is inspired from StatefulSets revisions.
• Just compare spec hashes to determine if the pod matches the spec.

I had to refactor some bits of the pod creation code to make things cleaner:

• Move ES data and logs volumes from the initcontainer package to the volume package
• Make sure we create all volumes and volume mounts in the initial pod spec, for comparison purpose. We used to only add them right before pod creation, because their name is based on the pod name that we don't have yet. Here we include them at pod spec creation time with a placeholder name, then make sure we replace them right before pod creation. The pod spec hash is based on the placeholder name, hence applies correctly to any pod name.
• Minor changes in the way parameters are moved around.

Fixes #1046.
Should help with #851 since we don't take injected pod environment variables into consideration for pod comparison, but I did not perform any test on AKS.

[pebrc]

Looks good to me! I like it I think it is a step in the right direction even though we still have to do the mental leap of adding a placeholder first and replacing it later. But you commented that well enough so 👍

[sebgl]

jenkins test this please

[sebgl]

jenkins test this please

[sebgl]

jenkins test this please

[barkbay]

I have a concern here because usually with deployments or statefulset user is allowed to change the value of an annotation (or remove the annotation) without triggering a new deployment.

It may sound weird but it might be used by a mutation webhook or an external system to store some informations on the pods that are running in a K8S cluster. (tbh this is what happen for me, I'm playing with MutatingWebhook which updates a annotation)

I don't know if we want to be compatible with that but it is just to point out that we may still have some issues with MutatingWebhook or any external system that would addupdate/delete some annotations while it is fine with deployments or statefulset.

[sebgl]

Good point!
Should we maybe include the initial labels & annotations the pod was created with into the hash computation? A user may update/remove/add any label/annotation on a running pod: since we would compare with the initially created labels & annotations and not the current ones, we would not trigger a pod replacement, unless we want to change that initial set of labels?

[sebgl]

jenkins test this please

[sebgl]

I added some commits to make sure we take labels (and maybe future annotations) we build into the pod spec into consideration when computing the hash.
We now manipulate a corev1.PodTemplateSpec around (including objectMeta) instead of just corev1.PodSpec. This allows users to patch labels and annotations on existing pods without causing the pod to be recreated: we compare with the initial set of labels in the template built by the operator.
Side benefit is we can get rid of the custom map comparison function since we now work with the template hash only.

[barkbay]

LGTM! 👍

I did some manual tests with a custom MutatingWebhook which adds a pod as a sidecar, a new env. variable and changes the annotations. Everything worked fine. I'm wondering if we should not add a e2e test with such a MutatingWebhook deployed (in an other PR of course)