Elasticsearch: set PVC labels through setting all StatefulSet labels to its volumeClaimTemplate #665

desaintmartin · 2020-06-15T14:56:03Z

Elasticsearch: set PVC labels through setting all StatefulSet labels to its volumeClaimTemplate.

Allows to track created PVCs (example: delete all PVCs from this recently deleted Release).

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-06-15T14:56:05Z

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?

jmlrt · 2020-06-25T12:32:10Z

jenkins test this please

jmlrt

Hi @desaintmartin,
Thank you for this PR 👍

jmlrt

The upgrade test is failing because volumeClaimTemplates update is forbidden for statefulsets:

14:52:00 Upgrading Elasticsearch chart
14:52:01 Error: UPGRADE FAILED: StatefulSet.apps "upgrade-master" is invalid: spec: Forbidden: updates to statefulset spec for fields other than 'replicas', 'template', and 'updateStrategy' are forbidden
14:52:01 UPGRADE FAILED

https://devops-ci.elastic.co/job/elastic+helm-charts+pull-request+integration-elasticsearch/484/ES_SUITE=upgrade,KUBERNETES_VERSION=1.14,label=docker&&virtual/console

I would suggest adding a persistence.labels.enabled value. This label would default to false to no break upgrade from previous versions and would add the labels only when overrided to true.

WDYT?

desaintmartin · 2020-06-25T15:41:36Z

Sure, here you are. Would you need tests for this?

jmlrt · 2020-06-25T16:36:02Z

jenkins test this please

jmlrt · 2020-06-25T16:47:24Z

elasticsearch/templates/statefulset.yaml

@@ -26,6 +26,16 @@ spec:
  volumeClaimTemplates:
  - metadata:
      name: {{ template "elasticsearch.uname" . }}
+    {{- if .Values.labels.enabled }}


Suggested change

{{- if .Values.labels.enabled }}

{{- if .Values.persistence.labels.enabled }}

jmlrt · 2020-06-25T16:57:24Z

Sure, here you are. Would you need tests for this?

Adding test would be great. Ideally we should have an assert in test_default to ensure that there is no pvc labels with default values, and another test like test_adding_storageclass_annotation_to_volumeclaimtemplate to ensure that labels are added when they are defined.

desaintmartin · 2020-06-26T13:11:15Z

Updated!

jmlrt · 2020-06-26T13:40:28Z

jenkins test this please

elasticsearch/tests/elasticsearch_test.py

cla-checker-service · 2020-06-29T13:45:03Z

💚 CLA has been signed

…to its volumeClaimTemplate. Allows to track created PVCs (example: delete all PVCs from this recently deleted Release). Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

desaintmartin · 2020-06-29T13:52:21Z

Done, signed, rebased.
When running tests locally, it seemed to pass, but I didn't pay attention too much, maybe I did something wrong.

jmlrt · 2020-06-29T14:17:29Z

jenkins test this please

jmlrt

LGTM⛴
Thanks for this PR 👍

…to its volumeClaimTemplate (#665)

jmlrt · 2020-06-29T15:18:34Z

backported to 6.8, 7.8and 7.x branches

desaintmartin force-pushed the labels branch from b785c46 to be3c093 Compare June 15, 2020 15:04

jmlrt approved these changes Jun 25, 2020

View reviewed changes

jmlrt suggested changes Jun 25, 2020

View reviewed changes

desaintmartin force-pushed the labels branch from ed54111 to 21a7a06 Compare June 26, 2020 13:00

jmlrt suggested changes Jun 26, 2020

View reviewed changes

elasticsearch/tests/elasticsearch_test.py Outdated Show resolved Hide resolved

desaintmartin force-pushed the labels branch from 9306eec to 34ae359 Compare June 29, 2020 13:45

Elasticsearch: set PVC labels through setting all StatefulSet labels …

9daeac8

…to its volumeClaimTemplate. Allows to track created PVCs (example: delete all PVCs from this recently deleted Release). Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

desaintmartin force-pushed the labels branch from 34ae359 to 9daeac8 Compare June 29, 2020 13:51

jmlrt approved these changes Jun 29, 2020

View reviewed changes

jmlrt merged commit 96c5155 into elastic:master Jun 29, 2020

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

Elasticsearch: set PVC labels through setting all StatefulSet labels …

2f66a12

…to its volumeClaimTemplate (#665)

jmlrt added elasticsearch enhancement New feature or request v6.8.11 v7.8.1 labels Jun 29, 2020

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

Elasticsearch: set PVC labels through setting all StatefulSet labels …

c2f0c6e

…to its volumeClaimTemplate (#665)

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

Elasticsearch: set PVC labels through setting all StatefulSet labels …

c0b75d5

…to its volumeClaimTemplate (#665)

jmlrt mentioned this pull request Jul 16, 2020

7.8.1 Release #730

Closed

jmlrt mentioned this pull request Jul 16, 2020

6.8.11 Release #731

Closed

This was referenced Jul 27, 2020

6.8.11 Release #753

Merged

7.8.1 Release #754

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Elasticsearch: set PVC labels through setting all StatefulSet labels to its volumeClaimTemplate #665

Elasticsearch: set PVC labels through setting all StatefulSet labels to its volumeClaimTemplate #665

desaintmartin commented Jun 15, 2020

elasticmachine commented Jun 15, 2020

jmlrt commented Jun 25, 2020

jmlrt left a comment

jmlrt left a comment

desaintmartin commented Jun 25, 2020

jmlrt commented Jun 25, 2020

jmlrt Jun 25, 2020

jmlrt commented Jun 25, 2020

desaintmartin commented Jun 26, 2020

jmlrt commented Jun 26, 2020

cla-checker-service bot commented Jun 29, 2020 •

edited

Loading

desaintmartin commented Jun 29, 2020

jmlrt commented Jun 29, 2020

jmlrt left a comment

jmlrt commented Jun 29, 2020

	{{- if .Values.labels.enabled }}
	{{- if .Values.persistence.labels.enabled }}

Elasticsearch: set PVC labels through setting all StatefulSet labels to its volumeClaimTemplate #665

Elasticsearch: set PVC labels through setting all StatefulSet labels to its volumeClaimTemplate #665

Conversation

desaintmartin commented Jun 15, 2020

elasticmachine commented Jun 15, 2020

jmlrt commented Jun 25, 2020

jmlrt left a comment

Choose a reason for hiding this comment

jmlrt left a comment

Choose a reason for hiding this comment

desaintmartin commented Jun 25, 2020

jmlrt commented Jun 25, 2020

jmlrt Jun 25, 2020

Choose a reason for hiding this comment

jmlrt commented Jun 25, 2020

desaintmartin commented Jun 26, 2020

jmlrt commented Jun 26, 2020

cla-checker-service bot commented Jun 29, 2020 • edited Loading

desaintmartin commented Jun 29, 2020

jmlrt commented Jun 29, 2020

jmlrt left a comment

Choose a reason for hiding this comment

jmlrt commented Jun 29, 2020

cla-checker-service bot commented Jun 29, 2020 •

edited

Loading