From fbf34659db5667e816f8d7e4333e8053731de403 Mon Sep 17 00:00:00 2001 From: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> Date: Tue, 4 May 2021 11:16:14 -0600 Subject: [PATCH] Fix chart DAG gitsync and persistence with KubernetesExecutor This prevents a gitsync contianer from being created in the KubernetesExecutor worker if DAG persistence is enabled as the DAG will already be on the volume. This also only mounts the DAGs volume once in the worker. --- .../pod-template-file.kubernetes-helm-yaml | 10 +-- .../templates/workers/worker-deployment.yaml | 2 +- chart/tests/test_pod_template_file.py | 61 ++++++++++++++++++- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/chart/files/pod-template-file.kubernetes-helm-yaml b/chart/files/pod-template-file.kubernetes-helm-yaml index 02f8b0fff8167..655b8a62e3f46 100644 --- a/chart/files/pod-template-file.kubernetes-helm-yaml +++ b/chart/files/pod-template-file.kubernetes-helm-yaml @@ -20,7 +20,7 @@ kind: Pod metadata: name: dummy-name spec: -{{- if .Values.dags.gitSync.enabled }} +{{- if and .Values.dags.gitSync.enabled (not .Values.dags.persistence.enabled) }} initContainers: {{- include "git_sync_container" (dict "Values" .Values "is_init" "true") | indent 4 }} {{- end }} @@ -61,17 +61,11 @@ spec: name: git-sync-ssh-key subPath: ssh {{- end }} -{{- if .Values.dags.persistence.enabled }} +{{- if or .Values.dags.gitSync.enabled .Values.dags.persistence.enabled }} - mountPath: {{ include "airflow_dags_mount_path" . }} name: dags readOnly: true {{- end }} -{{- if .Values.dags.gitSync.enabled }} - - mountPath: {{ include "airflow_dags" . }} - name: dags - readOnly: true - subPath: {{.Values.dags.gitSync.dest }}/{{ .Values.dags.gitSync.subPath }} -{{- end }} {{- if .Values.workers.extraVolumeMounts }} {{ toYaml .Values.workers.extraVolumeMounts | indent 8 }} {{- end }} diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index b51cedaac6f1e..6e3b6fe10dc26 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -158,7 +158,7 @@ spec: subPath: airflow_local_settings.py readOnly: true {{- end }} -{{- if or .Values.dags.persistence.enabled .Values.dags.gitSync.enabled }} +{{- if or .Values.dags.persistence.enabled .Values.dags.gitSync.enabled }} - name: dags mountPath: {{ template "airflow_dags_mount_path" . }} {{- end }} diff --git a/chart/tests/test_pod_template_file.py b/chart/tests/test_pod_template_file.py index 3ec6fb51e59ba..4e9f6935fc2ae 100644 --- a/chart/tests/test_pod_template_file.py +++ b/chart/tests/test_pod_template_file.py @@ -22,6 +22,7 @@ from shutil import copyfile import jmespath +from parameterized import parameterized from tests.helm_template_generator import render_chart @@ -101,6 +102,56 @@ def test_should_add_an_init_container_if_git_sync_is_true(self): "volumeMounts": [{"mountPath": "/git-root", "name": "dags"}], } == jmespath.search("spec.initContainers[0]", docs[0]) + def test_should_not_add_init_container_if_dag_persistence_is_true(self): + docs = render_chart( + values={ + "dags": { + "persistence": {"enabled": True}, + "gitSync": {"enabled": True}, + } + }, + show_only=["templates/pod-template-file.yaml"], + ) + + assert jmespath.search("spec.initContainers", docs[0]) is None + + @parameterized.expand( + [ + ({"gitSync": {"enabled": True}},), + ({"persistence": {"enabled": True}},), + ( + { + "gitSync": {"enabled": True}, + "persistence": {"enabled": True}, + }, + ), + ] + ) + def test_dags_mount(self, dag_values): + docs = render_chart( + values={"dags": dag_values}, + show_only=["templates/pod-template-file.yaml"], + ) + + assert {"mountPath": "/opt/airflow/dags", "name": "dags", "readOnly": True} in jmespath.search( + "spec.containers[0].volumeMounts", docs[0] + ) + + def test_dags_mount_with_gitsync_and_persistence(self): + docs = render_chart( + values={ + "dags": { + "gitSync": {"enabled": True}, + "persistence": {"enabled": True}, + } + }, + show_only=["templates/pod-template-file.yaml"], + ) + + assert {"mountPath": "/opt/airflow/dags", "name": "dags", "readOnly": True} in jmespath.search( + "spec.containers[0].volumeMounts", docs[0] + ) + def test_validate_if_ssh_params_are_added(self): docs = render_chart( values={ @@ -182,7 +233,7 @@ def test_should_set_username_and_pass_env_variables(self): "valueFrom": {"secretKeyRef": {"name": "user-pass-secret", "key": "GIT_SYNC_PASSWORD"}}, } in jmespath.search("spec.initContainers[0].env", docs[0]) - def test_should_set_the_volume_claim_correctly_when_using_an_existing_claim(self): + def test_should_set_the_dags_volume_claim_correctly_when_using_an_existing_claim(self): docs = render_chart( values={"dags": {"persistence": {"enabled": True, "existingClaim": "test-claim"}}}, show_only=["templates/pod-template-file.yaml"], @@ -192,6 +243,14 @@ def test_should_set_the_volume_claim_correctly_when_using_an_existing_claim(self "spec.volumes", docs[0] ) + def test_should_use_empty_dir_for_gitsync_without_persistence(self): + docs = render_chart( + values={"dags": {"gitSync": {"enabled": True}}}, + show_only=["templates/pod-template-file.yaml"], + ) + + assert {"name": "dags", "emptyDir": {}} in jmespath.search("spec.volumes", docs[0]) + def test_should_set_a_custom_image_in_pod_template(self): docs = render_chart( values={"images": {"pod_template": {"repository": "dummy_image", "tag": "latest"}}},