Disable service links to prevent very long startup times #542

floretan · 2020-03-25T12:01:13Z

The Elasticsearch docker image uses environment variables to set Elasticsearch configuration options. Kubernetes automatically adds environment variables for all services in the current namespace.

When trying to debug why Elasticsearch was taking a particularly long time (over 10 minutes) to boot in one of our projects, we noticed that this behavior was dependent on the namespace, even though no quotas or other limitations were in place. The slow start was also accompanied by a high cpu usage for a few minutes, even though Elasticsearch had no indices at that point. We recently had another issue where the massive number of environment variables was causing an issue, and when we disabled the service links with enableServiceLinks: false, the startup behavior went back to normal.

The service links are there to facilitate connecting to other services, but Elasticsearch doesn't usually connect to anything else, so there should be no side effects (when a node connects to another node in the cluster, it uses explicitly declared services, not the automatically generated environment variables).

I'm not sure if this small static configuration changes needs tests, but I'm happy to add them if needed.

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

elasticmachine · 2020-03-25T12:01:15Z

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?

floretan · 2020-05-05T08:47:59Z

This is becoming a blocker for us, it would be very nice to have it merged soon. What would be needed to move forward?

jmlrt · 2020-05-28T16:28:09Z

Hi @floretan, thanks for your PR.
Disable service links make sense for Elasticsearch chart.

enableServiceLinks was introduced in K8S 1.13. Elasticsearch chart README indicates that it requires K8S >= 1.8.

This PR should update the README requirements and we need to discuss it internally.

jmlrt · 2020-06-15T04:39:49Z

Hi @floretan,

After some internal discussion, we would prefer not breaking the current behavior or compatibility with older K8S versions with Elasticsearch chart default values.

Can you create an enableServiceLinks value which would be set to true by default and could be overrided to false?

jmlrt · 2020-06-15T04:41:02Z

elasticsearch/templates/statefulset.yaml

@@ -22,6 +22,7 @@ spec:
  podManagementPolicy: {{ .Values.podManagementPolicy }}
  updateStrategy:
    type: {{ .Values.updateStrategy }}
+  enableServiceLinks: false


Suggested change

enableServiceLinks: false

enableServiceLinks: {{ .Values.enableServiceLinks }}

with enableServiceLinks set to true by default in values.yaml

floretan · 2020-06-15T16:03:32Z

Thank you for taking a closer look at this issue. I hadn't considered the conflict with older kubernetes versions, and I'm glad such attention to details is put into these reviews.

One issue is that enableServiceLinks: true would not be a valid syntax for kubernetes < 1.13, even if this is the default behavior. So far I found three options:

I had a look at using semverCompare to check that we have kubernetes 1.13 or greater, but unfortunately I haven't found a way to access the built-in object .Capabilities.KubeVersion or .Capabilities.KubeVersion.Version as a string that works with both helm 2 and helm 3. Some type checking might work, but the logic wouldn't be very pretty.
Another solution would be to make .Values.enableServiceLinks a configuration option as suggested, but only output the enableServiceLinks field if the value is false. The default value of true in the chart would also match with the default kubernetes behaviour when the field is not specified. This solution would also not be self-explanatory, and would require some comments to explain why things are done this way.
A third option would be to simply ignore the fact that enableServiceLinks: true would not be valid syntax for kubernetes < 1.13. The schema validation was added in helm 3, and the probability that someone is using helm 3 with a kubernetes version as old as 1.12 is pretty small.

Which option would you prefer?

jmlrt · 2020-06-15T18:43:22Z

One issue is that enableServiceLinks: true would not be a valid syntax for kubernetes < 1.13, even if this is the default behavior. So far I found three options:

Yep, sorry I didn't think about that, when I reviewed it this morning.

I had a look at using semverCompare to check that we have kubernetes 1.13 or greater, but unfortunately I haven't found a way to access the built-in object .Capabilities.KubeVersion or .Capabilities.KubeVersion.Version as a string that works with both helm 2 and helm 3. Some type checking might work, but the logic wouldn't be very pretty.

We are using semverCompare with .Capabilities.KubeVersion.GitVersion to check K8S version:

helm-charts/elasticsearch/templates/_helpers.tpl

Lines 71 to 72 in 7624ed3

    
           {{- if semverCompare "<1.9-0" .Capabilities.KubeVersion.GitVersion -}} 
        
           {{- print "apps/v1beta2" -}}

This is working fine with Helm 2 but we didn't really tested it with Helm 3 yet. I just saw that it was no more in Helm 3 doc and is flagged as deprecated in Helm 3 code.

Note that we officially still don't support Helm 3 so staying with .Capabilities.KubeVersion.GitVersion in this PR is fine anyway and if there is an issue with Helm 3 we'll fix it in the same time that we'll fix all the other .Capabilities.KubeVersion.GitVersion references.

Another solution would be to make .Values.enableServiceLinks a configuration option as suggested, but only output the enableServiceLinks field if the value is false. The default value of true in the chart would also match with the default kubernetes behaviour when the field is not specified. This solution would also not be self-explanatory, and would require some comments to explain why things are done this way.

I'm also totally fine with using an enableServiceLinks value set to trueby default and having a conditional block:

{{- if not .Values.enableServiceLinks }}
enableServiceLinks: false
{{- end }}

This wouldn't change the current behavior and we would only need to mention that setting this value false is only with K8S >= 1.13 in the README config description which could avoid adding using .Capabilities.KubeVersion.GitVersion.

Prevents very long startup times when the namespace contains many services. Only valid for kubernetes versions >1.13

floretan · 2020-06-16T15:45:08Z

After taking a closer look at the usage in other charts, I found that .Capabilities.KubeVersion.GitVersion is still the only version that I could find in use despite being deprecated in Helm 3, so I went ahead and updated the PR to include a check using this variable.

This gives us the performance improvement out of the box (as mentioned in the original description, there are no side effects), and prevents adding an unnecessary configuration option.

jmlrt · 2020-06-16T15:49:26Z

elasticsearch/templates/statefulset.yaml

@@ -143,6 +143,9 @@ spec:
      {{- if .Values.imagePullSecrets }}
      imagePullSecrets:
 {{ toYaml .Values.imagePullSecrets | indent 8 }}
+      {{- end }}
+      {{- if semverCompare ">1.13" .Capabilities.KubeVersion.GitVersion }}
+      enableServiceLinks: false


Note that we still want it to be configurable and enabled by default in values.yamlin addition of the K8S version check:

Suggested change

enableServiceLinks: false

enableServiceLinks: {{ .Values.enableServiceLinks }}

Ok, I made the value configurable and updated both values.yaml and README.md.

jmlrt · 2020-06-17T10:47:38Z

jenkins test this please

jmlrt

LGTM⛴

floretan · 2020-06-17T11:48:49Z

The build failure seems to be caused by issues when downloading kubectl, so it's most likely a temporary network access issue rather than an issue with the changes.

jmlrt · 2020-06-17T12:08:32Z

Yep I'm fine with merging 😄
Thanks for this PR 👍

…mes (#542) Prevents very long startup times when the namespace contains many services. Only valid for kubernetes versions >1.13

jmlrt · 2020-06-17T12:12:48Z

backported to 6.8, 7.8and 7.x branches

jmlrt added the enhancement New feature or request label Mar 25, 2020

floretan pushed a commit to wunderio/charts that referenced this pull request May 6, 2020

Apply patch from elastic/helm-charts#542

5b5195a

floretan mentioned this pull request May 6, 2020

Temporarily use our fork of the elasticsearch chart. wunderio/charts#116

Merged

jmlrt suggested changes Jun 15, 2020

View reviewed changes

Disable service links.

63427bd

Prevents very long startup times when the namespace contains many services. Only valid for kubernetes versions >1.13

floretan requested a review from jmlrt June 16, 2020 15:48

jmlrt suggested changes Jun 16, 2020

View reviewed changes

Make the disabling of service links configurable.

6d888b5

floretan requested a review from jmlrt June 17, 2020 10:40

jmlrt approved these changes Jun 17, 2020

View reviewed changes

jmlrt merged commit 00f10e3 into elastic:master Jun 17, 2020

jmlrt added v6.8.11 v7.8.0 labels Jun 17, 2020

jmlrt pushed a commit that referenced this pull request Jun 17, 2020

[elasticsearch] Disable service links to prevent very long startup ti…

53e42f1

…mes (#542) Prevents very long startup times when the namespace contains many services. Only valid for kubernetes versions >1.13

jmlrt pushed a commit that referenced this pull request Jun 17, 2020

[elasticsearch] Disable service links to prevent very long startup ti…

839881a

…mes (#542) Prevents very long startup times when the namespace contains many services. Only valid for kubernetes versions >1.13

jmlrt pushed a commit that referenced this pull request Jun 17, 2020

[elasticsearch] Disable service links to prevent very long startup ti…

ba17055

…mes (#542) Prevents very long startup times when the namespace contains many services. Only valid for kubernetes versions >1.13

jmlrt mentioned this pull request Jun 18, 2020

7.8.0 Release #673

Merged

floretan mentioned this pull request Jul 16, 2020

Include pre-releases in the semver range. #729

Merged

4 tasks

jmlrt mentioned this pull request Oct 28, 2020

fix 854 68 #876

Closed

This was referenced Nov 17, 2020

68 helm 3 #900

Closed

7x helm 3 #901

Closed

68 helm 3 #903

Closed

7.10 init #913

Closed

710 readme enhancements #926

Closed

jmlrt mentioned this pull request Feb 8, 2021

backport 1046 7x #1051

Closed

This was referenced Mar 15, 2021

7.12 #1101

Closed

7.x #1113

Closed

jmlrt mentioned this pull request May 25, 2021

7.x bump #1199

Closed

This was referenced Dec 14, 2021

[meta] initiate 7.16 branch #1475

Merged

[kibana] optionally disable SA token automount (#1301) #1481

Merged

6.8.21 bump #1488

Merged

jmlrt mentioned this pull request Mar 8, 2022

7.17.1 bump #1609

Merged

jmlrt mentioned this pull request Apr 21, 2022

7.17.3 bump #1649

Merged

This was referenced Sep 14, 2022

[meta] fix yaml indent #1698

Merged

[meta] follow-up of #1698 #1700

Merged

Rade333 mentioned this pull request Sep 27, 2022

SLT-792: Set enableServiceLinks to false in mariadb statefulsets wunderio/charts#331

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable service links to prevent very long startup times #542

Disable service links to prevent very long startup times #542

floretan commented Mar 25, 2020

elasticmachine commented Mar 25, 2020

floretan commented May 5, 2020

jmlrt commented May 28, 2020

jmlrt commented Jun 15, 2020

jmlrt Jun 15, 2020

floretan commented Jun 15, 2020

jmlrt commented Jun 15, 2020

floretan commented Jun 16, 2020

jmlrt Jun 16, 2020

floretan Jun 17, 2020

jmlrt commented Jun 17, 2020

jmlrt left a comment

floretan commented Jun 17, 2020

jmlrt commented Jun 17, 2020

jmlrt commented Jun 17, 2020

	enableServiceLinks: false
	enableServiceLinks: {{ .Values.enableServiceLinks }}

Disable service links to prevent very long startup times #542

Disable service links to prevent very long startup times #542

Conversation

floretan commented Mar 25, 2020

elasticmachine commented Mar 25, 2020

floretan commented May 5, 2020

jmlrt commented May 28, 2020

jmlrt commented Jun 15, 2020

jmlrt Jun 15, 2020

Choose a reason for hiding this comment

floretan commented Jun 15, 2020

jmlrt commented Jun 15, 2020

floretan commented Jun 16, 2020

jmlrt Jun 16, 2020

Choose a reason for hiding this comment

floretan Jun 17, 2020

Choose a reason for hiding this comment

jmlrt commented Jun 17, 2020

jmlrt left a comment

Choose a reason for hiding this comment

floretan commented Jun 17, 2020

jmlrt commented Jun 17, 2020

jmlrt commented Jun 17, 2020