[elasticsearch] Added health pod name override for compatibility#1058
[elasticsearch] Added health pod name override for compatibility#1058jmlrt merged 2 commits intoelastic:masterfrom
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
💚 CLA has been signed |
|
jenkins test this please |
jmlrt
left a comment
There was a problem hiding this comment.
While the code LGTM, could you add a bit more details about why this is needed?
"helm.sh/hook-delete-policy": hook-succeeded clean the test pod when test is successful.
Also "{{ .Release.Name }}-{{ randAlpha 5 | lower }}-test" should ensure that 2 different releases deployend in the same namespace should generate different pod names.
Could not see the name overrides contained within the README.md, I can add them if needed
nameOverride is described in https://github.com/elastic/helm-charts/blob/master/elasticsearch/README.md#L141, healthNameOverride should also have its description if this change is still needed.
|
Yeah so, Helmfile generates all of the charts and values into one directory. This is then commited to a github repository for helm to run upgrades on. With randomly generated names it means that every time a commit is done to the cluster repo, a new elasticsearch test pod is made. This is especially annoying when you have multiple commits at once, as it creates merge conflicts for the cluster PRs. Having to manually fix your CI/CD every hour is very annoying. We've removed elasticsearch from our cluster for this reason Allowing a health pod name override just means that if this pod is already created, it wont regenerate it and cause conflicts. If we add some new values to elasticsearch or upgrade the version, it will recreate. (Apologies for the Readme, fix incoming) |
|
@jmlrt sorry to be annoying, would like this to be merged before I have to do conflict fixing :) |
|
jenkins test this please |
|
unrelated errors, merging |
…stic#1058) Co-authored-by: Julien Mailleret <8582351+jmlrt@users.noreply.github.com>
…stic#1058) Co-authored-by: Julien Mailleret <8582351+jmlrt@users.noreply.github.com>
…stic#1058) Co-authored-by: Julien Mailleret <8582351+jmlrt@users.noreply.github.com>
…) (#1274) Co-authored-by: Julien Mailleret <8582351+jmlrt@users.noreply.github.com> Co-authored-by: Tom Hobson <34216245+tomhobson@users.noreply.github.com>
…) (#1273) Co-authored-by: Julien Mailleret <8582351+jmlrt@users.noreply.github.com> Co-authored-by: Tom Hobson <34216245+tomhobson@users.noreply.github.com>
…) (#1272) Co-authored-by: Julien Mailleret <8582351+jmlrt@users.noreply.github.com> Co-authored-by: Tom Hobson <34216245+tomhobson@users.noreply.github.com>
…stic#1058) Co-authored-by: Julien Mailleret <8582351+jmlrt@users.noreply.github.com>
${CHART}/tests/*.py${CHART}/examples/*/test/goss.yamlCould not see the name overrides contained within the README.md, I can add them if needed.
This is to add support for tools like
https://github.com/roboll/helmfile and https://jenkins-x.io/v3/
As they generate a new pod each time, this creates a lot of noise and can cause merge conflicts, being able to override the health pod name prevents the pod being recreated when nothing has changed and enables smooth sailing.