Skip to content

Commit

Permalink
fix helm chart worker deployment without kerberos (#11681)
Browse files Browse the repository at this point in the history
Follow up to #11130 : we shouldn't mount the `kerberos-keytab` volume
in the worker deployment if we are not using
kerberos in the first place.
(the previous behavior is breaking the chart)

(cherry picked from commit 4c54718)
  • Loading branch information
Florent Chehab authored and kaxil committed Nov 18, 2020
1 parent e7e612e commit c43329f
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 8 deletions.
2 changes: 2 additions & 0 deletions chart/templates/workers/worker-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,11 @@ spec:
{{- if .Values.workers.extraVolumes }}
{{ toYaml .Values.workers.extraVolumes | indent 8 }}
{{- end }}
{{- if .Values.kerberos.enabled }}
- name: kerberos-keytab
secret:
secretName: {{ include "kerberos_keytab_secret" . | quote }}
{{- end }}
- name: config
configMap:
name: {{ template "airflow_config" . }}
Expand Down
8 changes: 4 additions & 4 deletions chart/tests/test_celery_kubernetes_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ def test_should_create_a_worker_deployment_with_the_celery_executo(self):
show_only=["templates/workers/worker-deployment.yaml"],
)

self.assertEqual("config", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[2].name", docs[0]))
self.assertEqual("config", jmespath.search("spec.template.spec.volumes[0].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))

def test_should_create_a_worker_deployment_with_the_celery_kubernetes_executor(self):
docs = render_chart(
Expand All @@ -43,5 +43,5 @@ def test_should_create_a_worker_deployment_with_the_celery_kubernetes_executor(s
show_only=["templates/workers/worker-deployment.yaml"],
)

self.assertEqual("config", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[2].name", docs[0]))
self.assertEqual("config", jmespath.search("spec.template.spec.volumes[0].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))
8 changes: 4 additions & 4 deletions chart/tests/test_git_sync_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ def test_should_add_dags_volume_to_the_worker_if_git_sync_and_peristence_is_enab
show_only=["templates/workers/worker-deployment.yaml"],
)

self.assertEqual("config", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[2].name", docs[0]))
self.assertEqual("config", jmespath.search("spec.template.spec.volumes[0].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))

def test_should_add_dags_volume_to_the_worker_if_git_sync_is_enabled_and_peristence_is_disabled(self):
docs = render_chart(
Expand All @@ -43,8 +43,8 @@ def test_should_add_dags_volume_to_the_worker_if_git_sync_is_enabled_and_periste
show_only=["templates/workers/worker-deployment.yaml"],
)

self.assertEqual("config", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[2].name", docs[0]))
self.assertEqual("config", jmespath.search("spec.template.spec.volumes[0].name", docs[0]))
self.assertEqual("dags", jmespath.search("spec.template.spec.volumes[1].name", docs[0]))

def test_should_add_git_sync_container_to_worker_if_persistence_is_not_enabled_but_git_sync_is(self):
docs = render_chart(
Expand Down
32 changes: 32 additions & 0 deletions chart/tests/test_kerberos.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

import json
import unittest

from tests.helm_template_generator import render_chart


class KerberosTest(unittest.TestCase):
def test_kerberos_not_mentioned_in_render_if_disabled(self):
k8s_objects = render_chart(name="NO-KERBEROS", values={"kerberos": {'enabled': False}})
# ignore airflow config map
k8s_objects_to_consider = [
obj for obj in k8s_objects if obj["metadata"]["name"] != "NO-KERBEROS-airflow-config"
]
k8s_objects_to_consider_str = json.dumps(k8s_objects_to_consider)
self.assertNotIn("kerberos", k8s_objects_to_consider_str)

0 comments on commit c43329f

Please sign in to comment.