-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[elasticsearch] add value to disable service #1115
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 |
358a6ff
to
30e0341
Compare
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 |
will need to be rebased after #1141 merge to fix tests |
4a0a0c0
to
210671e
Compare
@jmlrt Thanks for the review! I added the new value to the README and will rebase the branch if required |
139ab8c
to
a0350f1
Compare
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.
Hi @nflaig, I rebased your branch on master and fixed the conflicts.
The change LGTM to me.
Adding a test in https://github.com/elastic/helm-charts/blob/master/elasticsearch/tests/elasticsearch_test.py to ensure that the service isn't created when setting service.enabled=false
would be great.
Are you interested in adding this test? Otherwise I think we can merge if tests are successful.
a0350f1
to
d80aeaf
Compare
@jmlrt Thanks for rebasing the branch. I added a test to check if disabling the service works as expected if |
d80aeaf
to
3f8e99e
Compare
Signed-off-by: nflaig <nflaig@protonmail.com>
3f8e99e
to
c63ecfd
Compare
👍🏻
Yes, sorry I messed it a bit while rebasing... |
jenkins test this please |
Signed-off-by: nflaig <nflaig@protonmail.com>
Signed-off-by: nflaig <nflaig@protonmail.com>
Signed-off-by: nflaig <nflaig@protonmail.com>
Signed-off-by: nflaig <nflaig@protonmail.com>
Hey guys,
right now it is not possible to completely disable the service but it is not required if the helm chart is deployed to a cluster with a service mesh (consul, istio, etc.).
Since the
service.enabled
does not apply to the headless service a different name could be considered but I think the issue rather is that both services use the sameservice
value rather thanservice
andserviceHeadless
.Fix #998
${CHART}/tests/*.py
${CHART}/examples/*/test/goss.yaml