Remove .Values.service check before setting serviceName

[tweedge]

It appears that {{-if .Values.service }} is evaluating falsely here, preventing a required value in Helm 3 from being set (serviceName), and making this change resolves #497 without any other modifications to the template. Other StatefulSet definitions that are known to work (at a basic level) with Helm 3, such as elasticsearch/templates/statefulset.yaml#L16, do not implement this check.

It seems that making this change allows for additional Helm 3 compatibility without breaking anything in Helm 2, though correct me if I'm wrong there - I don't have anything running Helm 2.

• Chart version not bumped (the versions are all bumped and released at the same time)
• README.md updated with any new values or changes
• Updated template tests in ${CHART}/tests/*.py
• Updated integration tests in ${CHART}/examples/*/test/goss.yaml

^ Checked off all of the above as this change impacts none of them.

[elasticmachine]

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-checker-service]

💚 CLA has been signed

[tweedge]

Just signed the Contributor Agreement.

[jmlrt]

cla/check

[jmlrt]

jenkins test this please

[jmlrt]

Hi @tweedge, thanks for this PR.

template-testing is failing.

Can you update Logstash tests?

[tweedge]

Looking - thanks!

[tweedge]

Updated test & checked that no other serviceName checks are performed. I'm not entirely sure why this check was present, so I'm interested to see if Helm 2 functional testing works 100% as expected.

[jmlrt]

jenkins test this please

[tweedge]

Looks like no dice on Logstash + ES 6, per Jenkins. Though this is for Logstash + ES 7, so I don't know if that's acceptable or not.

[jmlrt]

@tweedge Logstash 6 test is failing for some strange reason (hostname is helm-logstash-six-logstash-0.svc.cluster.local with you PR while it is helm-logstash-six-logstash-0 without your PR).

Note that with this change, Service is no more optional as we shouldn't have a serviceName in StatefulSet without a Service.

Can you update Service and the related doc, tests and values to make it mandatory?

We'll see if that fix the strange Logstash 6 behavior.

[jmlrt]

jenkins test this please

[aidan-melen]

Thanks for working on this. Any updates on the last test?

[botelastic]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. To track this PR (even if closed), please open a corresponding issue if one does not already exist.

[jmlrt]

Hi @tweedge, @aidan-melen,
After deeper investigation, having an headless service is mandatory for statefulsets.
I created #695 to handle it and should merge it soon. This will replace this PR and fix #497.

I'm closing this one. Thanks for your help 👍