From 4960438333befb0f20f1659960dab143a93cb5b4 Mon Sep 17 00:00:00 2001 From: Nir Rosenthal Date: Thu, 21 Mar 2024 11:15:00 +0200 Subject: [PATCH 1/7] add persistentVolumeClaimRetentionPolicy --- .../scheduler/scheduler-deployment.yaml | 3 +++ .../triggerer/triggerer-deployment.yaml | 3 +++ .../templates/workers/worker-deployment.yaml | 3 +++ chart/values.schema.json | 19 +++++++++++++++++++ chart/values.yaml | 7 +++++++ 5 files changed, 35 insertions(+) diff --git a/chart/templates/scheduler/scheduler-deployment.yaml b/chart/templates/scheduler/scheduler-deployment.yaml index a19c6cbd93023..a19697092ab33 100644 --- a/chart/templates/scheduler/scheduler-deployment.yaml +++ b/chart/templates/scheduler/scheduler-deployment.yaml @@ -71,6 +71,9 @@ spec: {{- if and $stateful .Values.scheduler.updateStrategy }} updateStrategy: {{- toYaml .Values.scheduler.updateStrategy | nindent 4 }} {{- end }} + {{- if and $stateful .Values.workers.persistence.persistentVolumeClaimRetentionPolicy -}} + persistentVolumeClaimRetentionPolicy: {{ .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} + {{- end }} {{- if and (not $stateful) .Values.scheduler.strategy }} strategy: {{- toYaml .Values.scheduler.strategy | nindent 4 }} {{- end }} diff --git a/chart/templates/triggerer/triggerer-deployment.yaml b/chart/templates/triggerer/triggerer-deployment.yaml index 42e4ddf7a17fc..3d8e6d21d95cf 100644 --- a/chart/templates/triggerer/triggerer-deployment.yaml +++ b/chart/templates/triggerer/triggerer-deployment.yaml @@ -73,6 +73,9 @@ spec: {{- if and (not $persistence) (.Values.triggerer.strategy) }} strategy: {{- toYaml .Values.triggerer.strategy | nindent 4 }} {{- end }} + {{- if and $persistence .Values.triggerer.persistence.persistentVolumeClaimRetentionPolicy -}} + persistentVolumeClaimRetentionPolicy: {{ .Values.triggerer.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} + {{- end }} template: metadata: labels: diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index 0cf3ecb9f6e2f..2fe6fe0a94773 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -66,6 +66,9 @@ spec: {{- if $revisionHistoryLimit }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} + {{- if and $persistence .Values.workers.persistence.persistentVolumeClaimRetentionPolicy -}} + persistentVolumeClaimRetentionPolicy: {{ .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} + {{- end }} selector: matchLabels: tier: airflow diff --git a/chart/values.schema.json b/chart/values.schema.json index fed2168ea76a0..507ac3c488c3d 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -1601,6 +1601,25 @@ "type": "boolean", "default": true }, + "persistentVolumeClaimRetentionPolicy": { + "type": [ + "object", + "null" + ], + "additionalProperties": false, + "properties": { + "whenDeleted": { + "description": "Whether to retain the PVC when the StatefulSet is deleted.", + "type": "string", + "default": "Retain" + }, + "whenScaled": { + "description": "Whether to retain the PVC when the StatefulSet is scaled.", + "type": "string", + "default": "Retain" + } + } + }, "size": { "description": "Volume size for worker StatefulSet.", "type": "string", diff --git a/chart/values.yaml b/chart/values.yaml index c623f3e9dd939..26910bf043819 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -619,6 +619,11 @@ workers: persistence: # Enable persistent volumes enabled: true + # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed + persistentVolumeClaimRetentionPolicy: ~ + # persistentVolumeClaimRetentionPolicy: + # whenDeleted: Delete + # whenScaled: Delete # Volume size for worker StatefulSet size: 100Gi # If using a custom storageClass, pass name ref to all statefulSets here @@ -1532,6 +1537,8 @@ triggerer: persistence: # Enable persistent volumes enabled: true + # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed + persistentVolumeClaimRetentionPolicy: ~ # Volume size for triggerer StatefulSet size: 100Gi # If using a custom storageClass, pass name ref to all statefulSets here From b2b2200e76b136ddfe9ea363fd1c76f040751b04 Mon Sep 17 00:00:00 2001 From: Nir Rosenthal Date: Thu, 21 Mar 2024 11:25:56 +0200 Subject: [PATCH 2/7] add new triggerer persistentVolumeClaimRetentionPolicy to values schema --- chart/values.schema.json | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/chart/values.schema.json b/chart/values.schema.json index 507ac3c488c3d..1662737b649d4 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2965,6 +2965,25 @@ "type": "string", "default": "100Gi" }, + "persistentVolumeClaimRetentionPolicy": { + "type": [ + "object", + "null" + ], + "additionalProperties": false, + "properties": { + "whenDeleted": { + "description": "Whether to retain the PVC when the StatefulSet is deleted.", + "type": "string", + "default": "Retain" + }, + "whenScaled": { + "description": "Whether to retain the PVC when the StatefulSet is scaled.", + "type": "string", + "default": "Retain" + } + } + }, "storageClassName": { "description": "If using a custom StorageClass, pass name ref to all StatefulSets here.", "type": [ From 7d9c133d6d049398d95cc52bea588905f45f355a Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Sun, 16 Jun 2024 22:36:47 +0300 Subject: [PATCH 3/7] add persistentVolumeClaimRetentionPolicy --- .../pod-template-file.kubernetes-helm-yaml | 3 +++ .../scheduler/scheduler-deployment.yaml | 4 ++-- .../triggerer/triggerer-deployment.yaml | 4 ++-- chart/templates/workers/worker-deployment.yaml | 4 ++-- chart/values.yaml | 6 +++--- helm_tests/airflow_core/test_scheduler.py | 18 ++++++++++++++++++ helm_tests/airflow_core/test_triggerer.py | 18 ++++++++++++++++++ helm_tests/airflow_core/test_worker.py | 18 ++++++++++++++++++ 8 files changed, 66 insertions(+), 9 deletions(-) diff --git a/chart/files/pod-template-file.kubernetes-helm-yaml b/chart/files/pod-template-file.kubernetes-helm-yaml index 421922561608c..dba92f62de25b 100644 --- a/chart/files/pod-template-file.kubernetes-helm-yaml +++ b/chart/files/pod-template-file.kubernetes-helm-yaml @@ -45,6 +45,9 @@ metadata: checksum/kerberos-keytab: {{ include (print $.Template.BasePath "/secrets/kerberos-keytab-secret.yaml") . | sha256sum }} {{- end }} spec: + {{- if and $persistence .Values.workers.persistence.persistentVolumeClaimRetentionPolicy }} + persistentVolumeClaimRetentionPolicy: {{- toYaml .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} + {{- end }} initContainers: {{- if and .Values.dags.gitSync.enabled (not .Values.dags.persistence.enabled) }} {{- include "git_sync_container" (dict "Values" .Values "is_init" "true" "Template" .Template) | nindent 4 }} diff --git a/chart/templates/scheduler/scheduler-deployment.yaml b/chart/templates/scheduler/scheduler-deployment.yaml index a19697092ab33..ac1e2a8240872 100644 --- a/chart/templates/scheduler/scheduler-deployment.yaml +++ b/chart/templates/scheduler/scheduler-deployment.yaml @@ -71,8 +71,8 @@ spec: {{- if and $stateful .Values.scheduler.updateStrategy }} updateStrategy: {{- toYaml .Values.scheduler.updateStrategy | nindent 4 }} {{- end }} - {{- if and $stateful .Values.workers.persistence.persistentVolumeClaimRetentionPolicy -}} - persistentVolumeClaimRetentionPolicy: {{ .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} + {{- if and $stateful .Values.workers.persistence.persistentVolumeClaimRetentionPolicy }} + persistentVolumeClaimRetentionPolicy: {{- toYaml .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} {{- end }} {{- if and (not $stateful) .Values.scheduler.strategy }} strategy: {{- toYaml .Values.scheduler.strategy | nindent 4 }} diff --git a/chart/templates/triggerer/triggerer-deployment.yaml b/chart/templates/triggerer/triggerer-deployment.yaml index 3d8e6d21d95cf..da109c0a26977 100644 --- a/chart/templates/triggerer/triggerer-deployment.yaml +++ b/chart/templates/triggerer/triggerer-deployment.yaml @@ -73,8 +73,8 @@ spec: {{- if and (not $persistence) (.Values.triggerer.strategy) }} strategy: {{- toYaml .Values.triggerer.strategy | nindent 4 }} {{- end }} - {{- if and $persistence .Values.triggerer.persistence.persistentVolumeClaimRetentionPolicy -}} - persistentVolumeClaimRetentionPolicy: {{ .Values.triggerer.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} + {{- if and $persistence .Values.triggerer.persistence.persistentVolumeClaimRetentionPolicy }} + persistentVolumeClaimRetentionPolicy: {{- toYaml .Values.triggerer.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} {{- end }} template: metadata: diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index 2fe6fe0a94773..a32c86c289a9b 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -66,8 +66,8 @@ spec: {{- if $revisionHistoryLimit }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} - {{- if and $persistence .Values.workers.persistence.persistentVolumeClaimRetentionPolicy -}} - persistentVolumeClaimRetentionPolicy: {{ .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} + {{- if and $persistence .Values.workers.persistence.persistentVolumeClaimRetentionPolicy }} + persistentVolumeClaimRetentionPolicy: {{- toYaml .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} {{- end }} selector: matchLabels: diff --git a/chart/values.yaml b/chart/values.yaml index 26910bf043819..7df883e8a71e1 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -619,9 +619,9 @@ workers: persistence: # Enable persistent volumes enabled: true - # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed + # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled/removed persistentVolumeClaimRetentionPolicy: ~ - # persistentVolumeClaimRetentionPolicy: + # persistentVolumeClaimRetentionPolicy: # whenDeleted: Delete # whenScaled: Delete # Volume size for worker StatefulSet @@ -1537,7 +1537,7 @@ triggerer: persistence: # Enable persistent volumes enabled: true - # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled\removed + # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled/removed persistentVolumeClaimRetentionPolicy: ~ # Volume size for triggerer StatefulSet size: 100Gi diff --git a/helm_tests/airflow_core/test_scheduler.py b/helm_tests/airflow_core/test_scheduler.py index 3fb7f6754d8d5..a329b833a45e9 100644 --- a/helm_tests/airflow_core/test_scheduler.py +++ b/helm_tests/airflow_core/test_scheduler.py @@ -839,6 +839,24 @@ def test_scheduler_template_storage_class_name(self): "spec.volumeClaimTemplates[0].spec.storageClassName", docs[0] ) + def test_persistent_volume_claim_retention_policy(self): + docs = render_chart( + values={ + "executor": "LocalExecutor", + "workers": { + "persistence": { + "enabled": True, + "persistentVolumeClaimRetentionPolicy": {"whenDeleted": "Delete"}, + } + }, + }, + show_only=["templates/scheduler/scheduler-deployment.yaml"], + ) + + assert { + "whenDeleted": "Delete", + } == jmespath.search("spec.persistentVolumeClaimRetentionPolicy", docs[0]) + class TestSchedulerNetworkPolicy: """Tests scheduler network policy.""" diff --git a/helm_tests/airflow_core/test_triggerer.py b/helm_tests/airflow_core/test_triggerer.py index 0d8c4dd8206bc..9ef19b38df239 100644 --- a/helm_tests/airflow_core/test_triggerer.py +++ b/helm_tests/airflow_core/test_triggerer.py @@ -647,6 +647,24 @@ def test_triggerer_template_storage_class_name(self): "spec.volumeClaimTemplates[0].spec.storageClassName", docs[0] ) + def test_persistent_volume_claim_retention_policy(self): + docs = render_chart( + values={ + "executor": "CeleryExecutor", + "triggerer": { + "persistence": { + "enabled": True, + "persistentVolumeClaimRetentionPolicy": {"whenDeleted": "Delete"}, + } + }, + }, + show_only=["templates/triggerer/triggerer-deployment.yaml"], + ) + + assert { + "whenDeleted": "Delete", + } == jmespath.search("spec.persistentVolumeClaimRetentionPolicy", docs[0]) + class TestTriggererServiceAccount: """Tests triggerer service account.""" diff --git a/helm_tests/airflow_core/test_worker.py b/helm_tests/airflow_core/test_worker.py index 1fcc88caa6809..85384f176c682 100644 --- a/helm_tests/airflow_core/test_worker.py +++ b/helm_tests/airflow_core/test_worker.py @@ -82,6 +82,24 @@ def test_should_add_extra_containers(self): "image": "test-registry/test-repo:test-tag", } == jmespath.search("spec.template.spec.containers[-1]", docs[0]) + def test_persistent_volume_claim_retention_policy(self): + docs = render_chart( + values={ + "executor": "CeleryExecutor", + "workers": { + "persistence": { + "enabled": True, + "persistentVolumeClaimRetentionPolicy": {"whenDeleted": "Delete"}, + } + }, + }, + show_only=["templates/workers/worker-deployment.yaml"], + ) + + assert { + "whenDeleted": "Delete", + } == jmespath.search("spec.persistentVolumeClaimRetentionPolicy", docs[0]) + def test_should_template_extra_containers(self): docs = render_chart( values={ From 78bf2104d1db487f93f4266307fd92b21f0508da Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Mon, 17 Jun 2024 08:22:17 +0300 Subject: [PATCH 4/7] revert pod-template-file --- chart/files/pod-template-file.kubernetes-helm-yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/chart/files/pod-template-file.kubernetes-helm-yaml b/chart/files/pod-template-file.kubernetes-helm-yaml index dba92f62de25b..421922561608c 100644 --- a/chart/files/pod-template-file.kubernetes-helm-yaml +++ b/chart/files/pod-template-file.kubernetes-helm-yaml @@ -45,9 +45,6 @@ metadata: checksum/kerberos-keytab: {{ include (print $.Template.BasePath "/secrets/kerberos-keytab-secret.yaml") . | sha256sum }} {{- end }} spec: - {{- if and $persistence .Values.workers.persistence.persistentVolumeClaimRetentionPolicy }} - persistentVolumeClaimRetentionPolicy: {{- toYaml .Values.workers.persistence.persistentVolumeClaimRetentionPolicy | nindent 4 }} - {{- end }} initContainers: {{- if and .Values.dags.gitSync.enabled (not .Values.dags.persistence.enabled) }} {{- include "git_sync_container" (dict "Values" .Values "is_init" "true" "Template" .Template) | nindent 4 }} From fe00899906fc890a27ef1017b39ae3c150c1a595 Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Wed, 19 Jun 2024 08:21:14 +0300 Subject: [PATCH 5/7] add description to values schema --- chart/values.schema.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/chart/values.schema.json b/chart/values.schema.json index 1662737b649d4..b85cb6fac95c9 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -1602,10 +1602,12 @@ "default": true }, "persistentVolumeClaimRetentionPolicy": { + "description": "How PVCs are deleted during the lifecycle of a StatefulSet", "type": [ "object", "null" ], + "default": null, "additionalProperties": false, "properties": { "whenDeleted": { From 9cd81778ad7740385ce1133e25f7fe0ad4358fa3 Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Wed, 19 Jun 2024 20:45:20 +0300 Subject: [PATCH 6/7] add description to values.json --- chart/values.schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/chart/values.schema.json b/chart/values.schema.json index b85cb6fac95c9..000cb87c7d1aa 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2968,6 +2968,7 @@ "default": "100Gi" }, "persistentVolumeClaimRetentionPolicy": { + "description": "How PVCs are deleted during the lifecycle of a StatefulSet", "type": [ "object", "null" From a2afc9eb352c24754883a90d5acf9296d200834d Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Thu, 20 Jun 2024 23:00:16 +0300 Subject: [PATCH 7/7] fix values-schema, imporove comments --- chart/values.schema.json | 62 ++++++++++++++++------------------------ chart/values.yaml | 4 +-- 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/chart/values.schema.json b/chart/values.schema.json index 000cb87c7d1aa..ac5cf02e6ec6e 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -1602,25 +1602,8 @@ "default": true }, "persistentVolumeClaimRetentionPolicy": { - "description": "How PVCs are deleted during the lifecycle of a StatefulSet", - "type": [ - "object", - "null" - ], - "default": null, - "additionalProperties": false, - "properties": { - "whenDeleted": { - "description": "Whether to retain the PVC when the StatefulSet is deleted.", - "type": "string", - "default": "Retain" - }, - "whenScaled": { - "description": "Whether to retain the PVC when the StatefulSet is scaled.", - "type": "string", - "default": "Retain" - } - } + "$ref": "#/definitions/persistentVolumeClaimRetentionPolicy", + "description": "PersistentVolumeClaim retention policy to be used in the lifecycle of a StatefulSet" }, "size": { "description": "Volume size for worker StatefulSet.", @@ -2968,24 +2951,8 @@ "default": "100Gi" }, "persistentVolumeClaimRetentionPolicy": { - "description": "How PVCs are deleted during the lifecycle of a StatefulSet", - "type": [ - "object", - "null" - ], - "additionalProperties": false, - "properties": { - "whenDeleted": { - "description": "Whether to retain the PVC when the StatefulSet is deleted.", - "type": "string", - "default": "Retain" - }, - "whenScaled": { - "description": "Whether to retain the PVC when the StatefulSet is scaled.", - "type": "string", - "default": "Retain" - } - } + "$ref": "#/definitions/persistentVolumeClaimRetentionPolicy", + "description": "PersistentVolumeClaim retention policy to be used in the lifecycle of a StatefulSet" }, "storageClassName": { "description": "If using a custom StorageClass, pass name ref to all StatefulSets here.", @@ -11876,6 +11843,27 @@ } } } + }, + "persistentVolumeClaimRetentionPolicy": { + "description": "PersistentVolumeClaim retention policy to be used in the lifecycle of a StatefulSet", + "type": [ + "object", + "null" + ], + "default": null, + "additionalProperties": false, + "properties": { + "whenDeleted": { + "description": "Whether to retain the PVC when the StatefulSet is deleted.", + "type": "string", + "default": "Retain" + }, + "whenScaled": { + "description": "Whether to retain the PVC when the StatefulSet is scaled.", + "type": "string", + "default": "Retain" + } + } } } } diff --git a/chart/values.yaml b/chart/values.yaml index 7df883e8a71e1..bcdf71ae05020 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -619,7 +619,7 @@ workers: persistence: # Enable persistent volumes enabled: true - # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled/removed + # This policy determines whether PVCs should be deleted when StatefulSet is scaled down or removed. persistentVolumeClaimRetentionPolicy: ~ # persistentVolumeClaimRetentionPolicy: # whenDeleted: Delete @@ -1537,7 +1537,7 @@ triggerer: persistence: # Enable persistent volumes enabled: true - # persistentVolumeClaimRetentionPolicy if statefulset is used, to determine delete when scaled/removed + # This policy determines whether PVCs should be deleted when StatefulSet is scaled down or removed. persistentVolumeClaimRetentionPolicy: ~ # Volume size for triggerer StatefulSet size: 100Gi