-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[elasticsearch] Added health pod name override for compatibility #1058
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM⛴
jenkins test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM⛴
jenkins test this please |
unrelated errors, merging |
…stic#1058) Co-authored-by: Julien Mailleret <[email protected]>
…stic#1058) Co-authored-by: Julien Mailleret <[email protected]>
…stic#1058) Co-authored-by: Julien Mailleret <[email protected]>
…) (#1274) Co-authored-by: Julien Mailleret <[email protected]> Co-authored-by: Tom Hobson <[email protected]>
…) (#1273) Co-authored-by: Julien Mailleret <[email protected]> Co-authored-by: Tom Hobson <[email protected]>
…) (#1272) Co-authored-by: Julien Mailleret <[email protected]> Co-authored-by: Tom Hobson <[email protected]>
…stic#1058) Co-authored-by: Julien Mailleret <[email protected]>
${CHART}/tests/*.py
${CHART}/examples/*/test/goss.yaml
Could 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.