Skip to content

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

Merged
Jancis merged 4 commits intomasterfrom
feature/SLT-792
Sep 29, 2022
Merged

SLT-792: Set enableServiceLinks to false in mariadb statefulsets#331
Jancis merged 4 commits intomasterfrom
feature/SLT-792

Conversation

@Rade333
Copy link
Copy Markdown
Contributor

@Rade333 Rade333 commented Sep 27, 2022

Changes proposed:

Background:
In namespaces with large amount of environments, having enableServiceLinks set to the default value of "true" causes the number of environment variables to get so high that bash becomes super slow.

@Rade333 Rade333 requested a review from Jancis September 27, 2022 11:40
@Jancis
Copy link
Copy Markdown
Member

Jancis commented Sep 27, 2022

Any chance it could be exposed as a setting? In case someone still prefers having service links..

@Rade333
Copy link
Copy Markdown
Contributor Author

Rade333 commented Sep 27, 2022

Any chance it could be exposed as a setting? In case someone still prefers having service links..

Sure, it could. But seeing as we don't provide such a setting in any other charts, I thought it would be good to stay consistent :) I suggest we would deploy this like this and create a separate ticket about exposing this parameter as a setting.

@Jancis
Copy link
Copy Markdown
Member

Jancis commented Sep 27, 2022

@Rade333
Copy link
Copy Markdown
Contributor Author

Rade333 commented Sep 27, 2022

Oh :D
Okay, I'll make this a similar setting!

@Jancis
Copy link
Copy Markdown
Member

Jancis commented Sep 27, 2022

https://github.com/elastic/helm-charts/pull/542
It was the OG issue we tracked down a while ago and somehow everyone forgot about mariadb. It's possible there are more related chart services that are affected.

@Rade333
Copy link
Copy Markdown
Contributor Author

Rade333 commented Sep 27, 2022

@Jancis PR updated with the parameter exposed as a setting.

It's possible there are more related chart services that are affected.

I found that these files include PodSpec but do not have the enableServiceLinks parameter set:

  • csi-rclone/templates/1.13-csi-controller-rclone.yaml
  • csi-rclone/templates/1.13-csi-nodeplugin-rclone.yaml
  • csi-rclone/templates/1.19-csi-controller-rclone.yaml
  • csi-rclone/templates/1.19-csi-nodeplugin-rclone.yaml
  • legacy_traefik/templates/deployment.yaml
  • legacy_traefik/templates/storeconfig-job.yaml
  • memcached/templates/deployment.yaml
  • memcached/templates/statefulset.yaml
  • mongodb/templates/arbiter/statefulset.yaml
  • mongodb/templates/hidden/statefulset.yaml
  • mongodb/templates/replicaset/statefulset.yaml
  • rabbitmq/templates/statefulset.yaml
  • redis/templates/redis-master-statefulset.yaml
  • redis/templates/redis-slave-statefulset.yaml
  • silta-cluster/templates/downscaler-cron.yaml
  • silta-cluster/templates/k8s-controller-sidecars.yaml
  • silta-downscaler/templates/downscaler-cron.yaml

I suggest we create a separate ticket for evaluating if those need to be updated as well, and keep this current ticket and PR scoped for mariadb.

@Jancis
Copy link
Copy Markdown
Member

Jancis commented Sep 28, 2022

Thank You for checking those templates. Yes, create a separate (bundled) ticket for all those and this ticket will remain separate.

@Jancis
Copy link
Copy Markdown
Member

Jancis commented Sep 29, 2022

Merging this before drupal and frontend chart releases so they can pack the adjusted (and released) mariadb subchart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants