From 1ddd1640e5dc7ef777ba6bfe85a727235ea3fa6d Mon Sep 17 00:00:00 2001 From: Florent Chehab Date: Sat, 31 Oct 2020 22:55:35 +0100 Subject: [PATCH 1/2] Fix chart network policies definion and compliance Followup to #12003: * Some network policies & ingress are not valid against the jsonschema (empty values mostly) * Some network policies conditionnal definitions were incorrect --- chart/templates/flower/flower-ingress.yaml | 6 ++--- .../flower/flower-networkpolicy.yaml | 14 +++++------ .../webserver/webserver-networkpolicy.yaml | 14 +++++------ .../workers/worker-networkpolicy.yaml | 2 +- chart/tests/test_basic_helm_chart.py | 25 +++++++++++++++++++ 5 files changed, 43 insertions(+), 18 deletions(-) diff --git a/chart/templates/flower/flower-ingress.yaml b/chart/templates/flower/flower-ingress.yaml index 228631b84588f..361a25892bcae 100644 --- a/chart/templates/flower/flower-ingress.yaml +++ b/chart/templates/flower/flower-ingress.yaml @@ -29,10 +29,10 @@ metadata: release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- with .Values.ingress.flower.annotations }} annotations: - {{- range $key, $value := .Values.ingress.flower.annotations }} - {{ $key }}: {{ $value | quote }} - {{- end }} + {{ toYaml . | indent 4 }} + {{- end }} spec: {{- if .Values.ingress.flower.tls.enabled }} tls: diff --git a/chart/templates/flower/flower-networkpolicy.yaml b/chart/templates/flower/flower-networkpolicy.yaml index e3dd67c03f4fc..c5633962c113d 100644 --- a/chart/templates/flower/flower-networkpolicy.yaml +++ b/chart/templates/flower/flower-networkpolicy.yaml @@ -40,13 +40,13 @@ spec: component: flower release: {{ .Release.Name }} policyTypes: - - Ingress - ingress: - - from: + - Ingress {{- if .Values.flower.extraNetworkPolicies }} -{{ toYaml .Values.flower.extraNetworkPolicies | indent 4 }} + ingress: + - from: +{{ toYaml .Values.flower.extraNetworkPolicies | indent 6 }} + ports: + - protocol: TCP + port: {{ .Values.ports.flowerUI }} {{- end }} - ports: - - protocol: TCP - port: {{ .Values.ports.flowerUI }} {{- end }} diff --git a/chart/templates/webserver/webserver-networkpolicy.yaml b/chart/templates/webserver/webserver-networkpolicy.yaml index 3b2eaa75e4e80..591a38c202663 100644 --- a/chart/templates/webserver/webserver-networkpolicy.yaml +++ b/chart/templates/webserver/webserver-networkpolicy.yaml @@ -39,13 +39,13 @@ spec: component: webserver release: {{ .Release.Name }} policyTypes: - - Ingress - ingress: - - from: + - Ingress {{- if .Values.webserver.extraNetworkPolicies }} -{{ toYaml .Values.webserver.extraNetworkPolicies | indent 4 }} + ingress: + - from: +{{ toYaml .Values.webserver.extraNetworkPolicies | indent 6 }} + ports: + - protocol: TCP + port: {{ .Values.ports.airflowUI }} {{- end }} - ports: - - protocol: TCP - port: {{ .Values.ports.airflowUI }} {{- end }} diff --git a/chart/templates/workers/worker-networkpolicy.yaml b/chart/templates/workers/worker-networkpolicy.yaml index 2c5e12319df8a..193b55ee288ce 100644 --- a/chart/templates/workers/worker-networkpolicy.yaml +++ b/chart/templates/workers/worker-networkpolicy.yaml @@ -18,7 +18,7 @@ ################################ ## Airflow Worker NetworkPolicy ################################# -{{- if (and .Values.networkPolicies.enabled ( or (eq .Values.executor "CeleryExecutor")) (eq .Values.executor "CeleryKubernetesExecutor")) }} +{{- if and .Values.networkPolicies.enabled (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }} apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: diff --git a/chart/tests/test_basic_helm_chart.py b/chart/tests/test_basic_helm_chart.py index c55b4d923cea4..2fb9071ecf0c0 100644 --- a/chart/tests/test_basic_helm_chart.py +++ b/chart/tests/test_basic_helm_chart.py @@ -88,6 +88,31 @@ def test_basic_deployment_without_default_users(self): self.assertNotIn(('Job', 'TEST-BASIC-create-user'), list_of_kind_names_tuples) self.assertEqual(OBJECT_COUNT_IN_BASIC_DEPLOYMENT - 1, len(k8s_objects)) + def test_network_policies_are_valid(self): + k8s_objects = render_chart( + "TEST-BASIC", + { + "networkPolicies": {"enabled": True}, + "executor": "CeleryExecutor", + "pgbouncer": {"enabled": True}, + }, + ) + kind_names_tuples = { + (k8s_object['kind'], k8s_object['metadata']['name']) for k8s_object in k8s_objects + } + + expected_kind_names = [ + ('NetworkPolicy', 'TEST-BASIC-redis-policy'), + ('NetworkPolicy', 'TEST-BASIC-flower-policy'), + ('NetworkPolicy', 'TEST-BASIC-pgbouncer-policy'), + ('NetworkPolicy', 'TEST-BASIC-scheduler-policy'), + ('NetworkPolicy', 'TEST-BASIC-statsd-policy'), + ('NetworkPolicy', 'TEST-BASIC-webserver-policy'), + ('NetworkPolicy', 'TEST-BASIC-worker-policy'), + ] + for kind_name in expected_kind_names: + self.assertIn(kind_name, kind_names_tuples) + def test_chart_is_consistent_with_official_airflow_image(self): def get_k8s_objs_with_image(obj: Union[List[Any], Dict[str, Any]]) -> List[Dict[str, Any]]: """ From 87ada941f392e707085b74644dd36aaac70792e0 Mon Sep 17 00:00:00 2001 From: Florent Chehab Date: Sat, 31 Oct 2020 21:23:53 +0100 Subject: [PATCH 2/2] Support external redis instance in helm chart The main objective here is to support the use of an external redis instance in the helm chart. The values 'data.brokerUrl' and 'data.brokerUrlSecretName' are added and templates are updated. This support is added with no breaking changes (hopefully); only the redis.brokerURLSecretName value is removed, but it wasn't actually used in the chart. Extensive tests for the redis related part of this chart are also added (including runtime checks on the values). Docs also updated. Closes #11705 --- chart/README.md | 41 +++- chart/templates/_helpers.yaml | 2 +- chart/templates/check-values.yaml | 51 ++++ .../templates/redis/redis-networkpolicy.yaml | 2 +- chart/templates/redis/redis-service.yaml | 2 +- chart/templates/redis/redis-statefulset.yaml | 2 +- chart/templates/secrets/redis-secrets.yaml | 18 +- chart/tests/test_basic_helm_chart.py | 4 +- chart/tests/test_redis.py | 230 ++++++++++++++++++ chart/values.schema.json | 31 ++- chart/values.yaml | 11 +- 11 files changed, 361 insertions(+), 33 deletions(-) create mode 100644 chart/templates/check-values.yaml create mode 100644 chart/tests/test_redis.py diff --git a/chart/README.md b/chart/README.md index 83fc64d34eb7e..8bece660bab09 100644 --- a/chart/README.md +++ b/chart/README.md @@ -207,8 +207,10 @@ The following tables lists the configurable parameters of the Airflow chart and | `extraConfigMaps` | Extra ConfigMaps that will be managed by the chart | `{}` | | `data.metadataSecretName` | Secret name to mount Airflow connection string from | `~` | | `data.resultBackendSecretName` | Secret name to mount Celery result backend connection string from | `~` | +| `data.brokerUrlSecretName` | Secret name to mount redis connection url string from | `~` | | `data.metadataConection` | Field separated connection data (alternative to secret name) | `{}` | | `data.resultBackendConnection` | Field separated connection data (alternative to secret name) | `{}` | +| `data.brokerUrl` | String containing the redis broker url (if you are using an "external" redis) | `{}` | | `fernetKey` | String representing an Airflow Fernet key | `~` | | `fernetKeySecretName` | Secret name for Airflow Fernet key | `~` | | `kerberos.enabled` | Enable kerberos support for workers | `false` | @@ -218,7 +220,7 @@ The following tables lists the configurable parameters of the Airflow chart and | `kerberos.keytabPath` | Path for the Kerberos keytab file | `/etc/airflow.keytab` | | `kerberos.principal` | Name of the Kerberos principal | `airflow` | | `kerberos.reinitFrequency` | Frequency of reinitialization of the Kerberos token | `3600` | -| `kerberos.config` | Content of the configuration file for kerberos (might be templated using Helm templates) | `` | +| `kerberos.config` | Content of the configuration file for kerberos (might be templated using Helm templates) | `` | | `workers.replicas` | Replica count for Celery workers (if applicable) | `1` | | `workers.keda.enabled` | Enable KEDA autoscaling features | `false` | | `workers.keda.pollingInverval` | How often KEDA should poll the backend database for metrics in seconds | `5` | @@ -234,9 +236,9 @@ The following tables lists the configurable parameters of the Airflow chart and | `workers.resources.requests.memory` | Memory Request of workers | `~` | | `workers.terminationGracePeriodSeconds` | How long Kubernetes should wait for Celery workers to gracefully drain before force killing | `600` | | `workers.safeToEvict` | Allow Kubernetes to evict worker pods if needed (node downscaling) | `true` | -| `workers.serviceAccountAnnotations` | Annotations to add to worker kubernetes service account | `{}` | -| `workers.extraVolumes` | Mount additional volumes into worker | `[]` | -| `workers.extraVolumeMounts` | Mount additional volumes into worker | `[]` | +| `workers.serviceAccountAnnotations` | Annotations to add to worker kubernetes service account | `{}` | +| `workers.extraVolumes` | Mount additional volumes into worker | `[]` | +| `workers.extraVolumeMounts` | Mount additional volumes into worker | `[]` | | `workers.nodeSelector` | Node labels for pod assignment | `{}` | | `workers.affinity` | Affinity labels for pod assignment | `{}` | | `workers.tolerations` | Toleration labels for pod assignment | `[]` | @@ -249,9 +251,9 @@ The following tables lists the configurable parameters of the Airflow chart and | `scheduler.resources.requests.memory` | Memory Request of scheduler | `~` | | `scheduler.airflowLocalSettings` | Custom Airflow local settings python file | `~` | | `scheduler.safeToEvict` | Allow Kubernetes to evict scheduler pods if needed (node downscaling) | `true` | -| `scheduler.serviceAccountAnnotations` | Annotations to add to scheduler kubernetes service account | `{}` | -| `scheduler.extraVolumes` | Mount additional volumes into scheduler | `[]` | -| `scheduler.extraVolumeMounts` | Mount additional volumes into scheduler | `[]` | +| `scheduler.serviceAccountAnnotations` | Annotations to add to scheduler kubernetes service account | `{}` | +| `scheduler.extraVolumes` | Mount additional volumes into scheduler | `[]` | +| `scheduler.extraVolumeMounts` | Mount additional volumes into scheduler | `[]` | | `scheduler.nodeSelector` | Node labels for pod assignment | `{}` | | `scheduler.affinity` | Affinity labels for pod assignment | `{}` | | `scheduler.tolerations` | Toleration labels for pod assignment | `[]` | @@ -282,6 +284,18 @@ The following tables lists the configurable parameters of the Airflow chart and | `pgbouncer.nodeSelector` | Node labels for pod assignment | `{}` | | `pgbouncer.affinity` | Affinity labels for pod assignment | `{}` | | `pgbouncer.tolerations` | Toleration labels for pod assignment | `[]` | +| `redis.enabled` | Enable the redis provisioned by the chart | `true` | +| `redis.terminationGracePeriodSeconds` | Grace period for tasks to finish after SIGTERM is sent from Kubernetes. | `600` | +| `redis.persistence.enabled` | Enable persistent volumes. | `true` | +| `redis.persistence.size` | Volume size for redis StatefulSet. | `1Gi` | +| `redis.persistence.storageClassName` | If using a custom storageClass, pass name ref to all StatefulSets here. | `default` | +| `redis.resources.limits.cpu` | CPU Limit of redis | `~` | +| `redis.resources.limits.memory` | Memory Limit of redis | `~` | +| `redis.resources.requests.cpu` | CPU Request of redis | `~` | +| `redis.resources.requests.memory` | Memory Request of redis | `~` | +| `redis.passwordSecretName` | Redis password secret. | `~` | +| `redis.password` | If password is set, create secret with it, else generate a new one on install. | `~` | +| `redis.safeToEvict` | This setting tells Kubernetes that its ok to evict when it wants to scale a node down. | `true` | | `redis.nodeSelector` | Node labels for pod assignment | `{}` | | `redis.affinity` | Affinity labels for pod assignment | `{}` | | `redis.tolerations` | Toleration labels for pod assignment | `[]` | @@ -334,6 +348,19 @@ helm install airflow . \ --set workers.persistence.enabled=false ``` +## Using an external redis instance + +When using the `CeleryExecutor` or the `CeleryKubernetesExecutor` the chart will by default create a redis Deployment/StatefulSet alongside airflow. +You can also use "your own" redis instance by providing the `data.brokerUrl` (or `data.borkerUrlSecretName`) value directly: + +```bash +helm install airflow . \ + --namespace airflow \ + --set executor=CeleryExecutor \ + --set redis.enabled=false \ + --set data.brokerUrl=redis://redis-user:password@redis-host:6379/0 +``` + ## Walkthrough using kind **Install kind, and create a cluster:** diff --git a/chart/templates/_helpers.yaml b/chart/templates/_helpers.yaml index 530b1d083bd5d..90e1318d5e11b 100644 --- a/chart/templates/_helpers.yaml +++ b/chart/templates/_helpers.yaml @@ -47,7 +47,7 @@ - name: AIRFLOW__CELERY__BROKER_URL valueFrom: secretKeyRef: - name: {{ .Release.Name }}-broker-url + name: {{ default (printf "%s-broker-url" .Release.Name) .Values.data.brokerUrlSecretName }} key: connection {{- end }} {{- if .Values.elasticsearch.enabled }} diff --git a/chart/templates/check-values.yaml b/chart/templates/check-values.yaml new file mode 100644 index 0000000000000..01a764a5c4cac --- /dev/null +++ b/chart/templates/check-values.yaml @@ -0,0 +1,51 @@ +# 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. + +{{- /* +The sole purpose of this yaml file is it to check the values file is consistent for some complexe combinations. +*/ -}} + +{{- /* +############################## + Redis related checks +############################# +*/ -}} + + {{- if or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor") }} + {{- if .Values.redis.enabled }} + + {{- if and .Values.redis.passwordSecretName (not .Values.data.brokerUrlSecretName) }} + {{ required "When using the internal redis of the chart and setting the value redis.passwordSecretName, you must also set the value data.brokerUrlSecretName." nil }} + {{- end }} + + {{- if and .Values.redis.passwordSecretName .Values.redis.password }} + {{ required "You must not set both values redis.passwordSecretName and redis.password" nil }} + {{- end }} + + {{- else }} + + {{- if not (or .Values.data.brokerUrlSecretName .Values.data.brokerUrl) }} + {{ required "You must set one of the values data.brokerUrlSecretName or data.brokerUrl when using a Celery based executor with redis.enabled set to false (we need the url to the redis instance)." nil }} + {{- end }} + + {{- end }} + + {{- if and .Values.data.brokerUrlSecretName .Values.data.brokerUrl }} + {{ required "You must not set both values data.brokerUrlSecretName and data.brokerUrl" nil }} + {{- end }} + + {{- end }} diff --git a/chart/templates/redis/redis-networkpolicy.yaml b/chart/templates/redis/redis-networkpolicy.yaml index a7a795053f082..b0f62b2426578 100644 --- a/chart/templates/redis/redis-networkpolicy.yaml +++ b/chart/templates/redis/redis-networkpolicy.yaml @@ -18,7 +18,7 @@ ################################ ## Airflow Redis NetworkPolicy ################################# -{{- if (and .Values.networkPolicies.enabled (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor"))) }} +{{- if and .Values.redis.enabled .Values.networkPolicies.enabled (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }} apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: diff --git a/chart/templates/redis/redis-service.yaml b/chart/templates/redis/redis-service.yaml index b6e79db0885fa..bcd1b21744b9b 100644 --- a/chart/templates/redis/redis-service.yaml +++ b/chart/templates/redis/redis-service.yaml @@ -18,7 +18,7 @@ ################################ ## Airflow Redis Service ################################# -{{- if or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor") }} +{{- if and .Values.redis.enabled (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }} kind: Service apiVersion: v1 metadata: diff --git a/chart/templates/redis/redis-statefulset.yaml b/chart/templates/redis/redis-statefulset.yaml index 6da1b9c367626..9375384bcf36d 100644 --- a/chart/templates/redis/redis-statefulset.yaml +++ b/chart/templates/redis/redis-statefulset.yaml @@ -18,7 +18,7 @@ ################################ ## Airflow Redis StatefulSet ################################# -{{- if or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor") }} +{{- if and .Values.redis.enabled (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }} {{- $nodeSelector := or .Values.redis.nodeSelector .Values.nodeSelector }} {{- $affinity := or .Values.redis.affinity .Values.affinity }} {{- $tolerations := or .Values.redis.tolerations .Values.tolerations }} diff --git a/chart/templates/secrets/redis-secrets.yaml b/chart/templates/secrets/redis-secrets.yaml index 958474d232403..f47750fa845af 100644 --- a/chart/templates/secrets/redis-secrets.yaml +++ b/chart/templates/secrets/redis-secrets.yaml @@ -18,9 +18,10 @@ ################################ ## Airflow Redis Password Secret ################################# -# If both of these secret names are not set, we will either use the set password, or generate one -{{- if not (and .Values.redis.passwordSecretName .Values.redis.brokerURLSecretName) }} -{{ $random_redis_password := randAlphaNum 10 }} +{{- $random_redis_password := randAlphaNum 10 }} +{{- if or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor") }} +{{- if and .Values.redis.enabled (not .Values.redis.passwordSecretName) }} +# If passwordSecretName is not set, we will either use the set password, or use the generated one kind: Secret apiVersion: v1 metadata: @@ -40,9 +41,11 @@ type: Opaque data: password: {{ (default $random_redis_password .Values.redis.password) | b64enc | quote }} --- -################################ +{{- end }} +{{- if not .Values.data.brokerUrlSecretName }} +################################## ## Airflow Redis Connection Secret -################################# +################################## kind: Secret apiVersion: v1 metadata: @@ -60,5 +63,10 @@ metadata: "helm.sh/hook-weight": "0" type: Opaque data: +{{- if .Values.redis.enabled }} connection: {{ (printf "redis://:%s@%s-redis:6379/0" (default $random_redis_password .Values.redis.password) .Release.Name) | b64enc | quote }} +{{- else }} + connection: {{ (printf "%s" .Values.data.brokerUrl) | b64enc | quote }} +{{- end }} +{{- end }} {{- end }} diff --git a/chart/tests/test_basic_helm_chart.py b/chart/tests/test_basic_helm_chart.py index 2fb9071ecf0c0..7d835a6fa9cb5 100644 --- a/chart/tests/test_basic_helm_chart.py +++ b/chart/tests/test_basic_helm_chart.py @@ -22,7 +22,7 @@ from tests.helm_template_generator import render_chart -OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 24 +OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 22 class TestBaseChartTest(unittest.TestCase): @@ -62,8 +62,6 @@ def test_basic_deployments(self): ('Deployment', 'TEST-BASIC-webserver'), ('StatefulSet', 'TEST-BASIC-postgresql'), ('Secret', 'TEST-BASIC-fernet-key'), - ('Secret', 'TEST-BASIC-redis-password'), - ('Secret', 'TEST-BASIC-broker-url'), ('Job', 'TEST-BASIC-create-user'), ('Job', 'TEST-BASIC-run-airflow-migrations'), ], diff --git a/chart/tests/test_redis.py b/chart/tests/test_redis.py new file mode 100644 index 0000000000000..d4c5db791df43 --- /dev/null +++ b/chart/tests/test_redis.py @@ -0,0 +1,230 @@ +# 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 unittest +from base64 import b64decode +from subprocess import CalledProcessError +from typing import Optional + +from parameterized import parameterized + +from tests.helm_template_generator import prepare_k8s_lookup_dict, render_chart + +RELEASE_NAME_REDIS = "TEST-REDIS" + +REDIS_OBJECTS = { + "NETWORK_POLICY": ("NetworkPolicy", f"{RELEASE_NAME_REDIS}-redis-policy"), + "SERVICE": ("Service", f"{RELEASE_NAME_REDIS}-redis"), + "STATEFUL_SET": ("StatefulSet", f"{RELEASE_NAME_REDIS}-redis"), + "SECRET_PASSWORD": ("Secret", f"{RELEASE_NAME_REDIS}-redis-password"), + "SECRET_BROKER_URL": ("Secret", f"{RELEASE_NAME_REDIS}-broker-url"), +} +SET_POSSIBLE_REDIS_OBJECT_KEYS = set(REDIS_OBJECTS.values()) + +CELERY_EXECUTORS_PARAMS = [("CeleryExecutor",), ("CeleryKubernetesExecutor",)] + + +class RedisTest(unittest.TestCase): + @staticmethod + def get_broker_url_in_broker_url_secret(k8s_obj_by_key): + broker_url_in_obj = b64decode( + k8s_obj_by_key[REDIS_OBJECTS["SECRET_BROKER_URL"]]["data"]["connection"] + ).decode("utf-8") + return broker_url_in_obj + + @staticmethod + def get_redis_password_in_password_secret(k8s_obj_by_key): + password_in_obj = b64decode( + k8s_obj_by_key[REDIS_OBJECTS["SECRET_PASSWORD"]]["data"]["password"] + ).decode("utf-8") + return password_in_obj + + @staticmethod + def get_broker_url_secret_in_deployment(k8s_obj_by_key, kind: str, name: str) -> str: + deployment_obj = k8s_obj_by_key[(kind, f"{RELEASE_NAME_REDIS}-{name}")] + containers = deployment_obj["spec"]["template"]["spec"]["containers"] + container = next(obj for obj in containers if obj["name"] == name) + + envs = container["env"] + env = next(obj for obj in envs if obj["name"] == "AIRFLOW__CELERY__BROKER_URL") + return env["valueFrom"]["secretKeyRef"]["name"] + + def assert_password_and_broker_url_secrets( + self, k8s_obj_by_key, expected_password_match: Optional[str], expected_broker_url_match: Optional[str] + ): + if expected_password_match is not None: + redis_password_in_password_secret = self.get_redis_password_in_password_secret(k8s_obj_by_key) + self.assertRegex(redis_password_in_password_secret, expected_password_match) + else: + self.assertNotIn(REDIS_OBJECTS["SECRET_PASSWORD"], k8s_obj_by_key.keys()) + + if expected_broker_url_match is not None: + # assert redis broker url in secret + broker_url_in_broker_url_secret = self.get_broker_url_in_broker_url_secret(k8s_obj_by_key) + self.assertRegex(broker_url_in_broker_url_secret, expected_broker_url_match) + else: + self.assertNotIn(REDIS_OBJECTS["SECRET_BROKER_URL"], k8s_obj_by_key.keys()) + + def assert_broker_url_env( + self, k8s_obj_by_key, expected_broker_url_secret_name=REDIS_OBJECTS["SECRET_BROKER_URL"][1] + ): + broker_url_secret_in_scheduler = self.get_broker_url_secret_in_deployment( + k8s_obj_by_key, "StatefulSet", "worker" + ) + self.assertEqual(broker_url_secret_in_scheduler, expected_broker_url_secret_name) + broker_url_secret_in_worker = self.get_broker_url_secret_in_deployment( + k8s_obj_by_key, "Deployment", "scheduler" + ) + self.assertEqual(broker_url_secret_in_worker, expected_broker_url_secret_name) + + @parameterized.expand(CELERY_EXECUTORS_PARAMS) + def test_redis_by_chart_default(self, executor): + k8s_objects = render_chart( + RELEASE_NAME_REDIS, + { + "executor": executor, + "networkPolicies": {"enabled": True}, + "redis": {"enabled": True}, + }, + ) + k8s_obj_by_key = prepare_k8s_lookup_dict(k8s_objects) + + created_redis_objects = SET_POSSIBLE_REDIS_OBJECT_KEYS & set(k8s_obj_by_key.keys()) + self.assertEqual(created_redis_objects, SET_POSSIBLE_REDIS_OBJECT_KEYS) + + self.assert_password_and_broker_url_secrets( + k8s_obj_by_key, + expected_password_match=r"\w+", + expected_broker_url_match=fr"redis://:\w+@{RELEASE_NAME_REDIS}-redis:6379/0", + ) + + self.assert_broker_url_env(k8s_obj_by_key) + + @parameterized.expand(CELERY_EXECUTORS_PARAMS) + def test_redis_by_chart_password(self, executor): + k8s_objects = render_chart( + RELEASE_NAME_REDIS, + { + "executor": executor, + "networkPolicies": {"enabled": True}, + "redis": {"enabled": True, "password": "test-redis-password"}, + }, + ) + k8s_obj_by_key = prepare_k8s_lookup_dict(k8s_objects) + + created_redis_objects = SET_POSSIBLE_REDIS_OBJECT_KEYS & set(k8s_obj_by_key.keys()) + self.assertEqual(created_redis_objects, SET_POSSIBLE_REDIS_OBJECT_KEYS) + + self.assert_password_and_broker_url_secrets( + k8s_obj_by_key, + expected_password_match="test-redis-password", + expected_broker_url_match=f"redis://:test-redis-password@{RELEASE_NAME_REDIS}-redis:6379/0", + ) + + self.assert_broker_url_env(k8s_obj_by_key) + + @parameterized.expand(CELERY_EXECUTORS_PARAMS) + def test_redis_by_chart_password_secret_name_missing_broker_url_secret_name(self, executor): + with self.assertRaises(CalledProcessError): + render_chart( + RELEASE_NAME_REDIS, + { + "executor": executor, + "redis": { + "enabled": True, + "passwordSecretName": "test-redis-password-secret-name", + }, + }, + ) + + @parameterized.expand(CELERY_EXECUTORS_PARAMS) + def test_redis_by_chart_password_secret_name(self, executor): + expected_broker_url_secret_name = "test-redis-broker-url-secret-name" + k8s_objects = render_chart( + RELEASE_NAME_REDIS, + { + "executor": executor, + "networkPolicies": {"enabled": True}, + "data": {"brokerUrlSecretName": expected_broker_url_secret_name}, + "redis": { + "enabled": True, + "passwordSecretName": "test-redis-password-secret-name", + }, + }, + ) + k8s_obj_by_key = prepare_k8s_lookup_dict(k8s_objects) + + created_redis_objects = SET_POSSIBLE_REDIS_OBJECT_KEYS & set(k8s_obj_by_key.keys()) + self.assertEqual( + created_redis_objects, + SET_POSSIBLE_REDIS_OBJECT_KEYS + - {REDIS_OBJECTS["SECRET_PASSWORD"], REDIS_OBJECTS["SECRET_BROKER_URL"]}, + ) + + self.assert_password_and_broker_url_secrets( + k8s_obj_by_key, expected_password_match=None, expected_broker_url_match=None + ) + + self.assert_broker_url_env(k8s_obj_by_key, expected_broker_url_secret_name) + + @parameterized.expand(CELERY_EXECUTORS_PARAMS) + def test_external_redis_broker_url(self, executor): + k8s_objects = render_chart( + RELEASE_NAME_REDIS, + { + "executor": executor, + "networkPolicies": {"enabled": True}, + "data": { + "brokerUrl": "redis://redis-user:password@redis-host:6379/0", + }, + "redis": {"enabled": False}, + }, + ) + k8s_obj_by_key = prepare_k8s_lookup_dict(k8s_objects) + + created_redis_objects = SET_POSSIBLE_REDIS_OBJECT_KEYS & set(k8s_obj_by_key.keys()) + self.assertEqual(created_redis_objects, {REDIS_OBJECTS["SECRET_BROKER_URL"]}) + + self.assert_password_and_broker_url_secrets( + k8s_obj_by_key, + expected_password_match=None, + expected_broker_url_match="redis://redis-user:password@redis-host:6379/0", + ) + + self.assert_broker_url_env(k8s_obj_by_key) + + @parameterized.expand(CELERY_EXECUTORS_PARAMS) + def test_external_redis_broker_url_secret_name(self, executor): + expected_broker_url_secret_name = "redis-broker-url-secret-name" + k8s_objects = render_chart( + RELEASE_NAME_REDIS, + { + "executor": executor, + "networkPolicies": {"enabled": True}, + "data": {"brokerUrlSecretName": expected_broker_url_secret_name}, + "redis": {"enabled": False}, + }, + ) + k8s_obj_by_key = prepare_k8s_lookup_dict(k8s_objects) + + created_redis_objects = SET_POSSIBLE_REDIS_OBJECT_KEYS & set(k8s_obj_by_key.keys()) + self.assertEqual(created_redis_objects, set()) + + self.assert_password_and_broker_url_secrets( + k8s_obj_by_key, expected_password_match=None, expected_broker_url_match=None + ) + + self.assert_broker_url_env(k8s_obj_by_key, expected_broker_url_secret_name) diff --git a/chart/values.schema.json b/chart/values.schema.json index 6a4edc79bf483..ab6bae1fef85f 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -388,7 +388,7 @@ } }, "data": { - "description": "Airflow database configuration.", + "description": "Airflow database & redis configuration.", "type": "object", "additionalProperties": false, "properties": { @@ -406,6 +406,13 @@ "null" ] }, + "brokerUrlSecretName": { + "description": "Redis broker URL secret.", + "type": [ + "string", + "null" + ] + }, "metadataConnection": { "description": "Metadata connection configuration.", "type": "object", @@ -473,6 +480,13 @@ "type": "string" } } + }, + "brokerUrl": { + "description": "Direct url to the redis broker (when using an external redis instance).", + "type": [ + "string", + "null" + ] } } }, @@ -1088,10 +1102,14 @@ } }, "redis": { - "description": "", + "description": "Configuration for the redis provisioned by the chart.", "type": "object", "additionalProperties": false, "properties": { + "enabled": { + "description": "Enable the redis provisioned by the chart (you can also use an external redis instance with data.brokerUrl or data.brokerUrlSecretName).", + "type": "boolean" + }, "terminationGracePeriodSeconds": { "description": "Grace period for tasks to finish after SIGTERM is sent from Kubernetes.", "type": "integer" @@ -1106,7 +1124,7 @@ "type": "boolean" }, "size": { - "description": "Volume size for worker StatefulSet.", + "description": "Volume size for redis StatefulSet.", "type": "string" }, "storageClassName": { @@ -1128,13 +1146,6 @@ "null" ] }, - "brokerURLSecretName": { - "description": "Redis broker URL secret.", - "type": [ - "string", - "null" - ] - }, "password": { "description": "If password is set, create secret with it, else generate a new one on install.", "type": [ diff --git a/chart/values.yaml b/chart/values.yaml index 65b16603dc3ee..1abc4b97032fe 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -214,11 +214,12 @@ extraEnvFrom: ~ # - configMapRef: # name: '{{ .Release.Name }}-airflow-variables' -# Airflow database config +# Airflow database & redis config data: # If secret names are provided, use those secrets metadataSecretName: ~ resultBackendSecretName: ~ + brokerUrlSecretName: ~ # Otherwise pass connection values in metadataConnection: @@ -235,6 +236,7 @@ data: port: 5432 db: postgres sslmode: disable + brokerUrl: ~ # Fernet key settings fernetKey: ~ @@ -571,7 +573,9 @@ pgbouncer: affinity: {} tolerations: [] +# Configuration for the redis provisioned by the chart redis: + enabled: true terminationGracePeriodSeconds: 600 persistence: @@ -590,12 +594,11 @@ redis: # cpu: 100m # memory: 128Mi - # If set use as redis secret + # If set use as redis secret. Make sure to also set data.brokerUrlSecretName value. passwordSecretName: ~ - brokerURLSecretName: ~ # Else, if password is set, create secret with it, - # else generate a new one on install + # Otherwise a new password will be generated on install password: ~ # This setting tells kubernetes that its ok to evict