Disable flower in chart by default (apache#23737)

vincbeck · May 17, 2022 · 3993cb8 · 3993cb8
1 parent 239a9dc
commit 3993cb8
Show file tree

Hide file tree

Showing 14 changed files with 184 additions and 56 deletions.
diff --git a/chart/RELEASE_NOTES.rst b/chart/RELEASE_NOTES.rst
@@ -34,6 +34,19 @@ Default Airflow image is updated to ``2.3.0`` (#23386)
 
 The default Airflow image that is used with the Chart is now ``2.3.0``, previously it was ``2.2.4``.
 
+``ingress.enabled`` is deprecated
+"""""""""""""""""""""""""""""""""
+
+Instead of having a single flag to control ingress resources for both the webserver and flower, there
+are now separate flags to control them individually, ``ingress.web.enabled`` and ``ingress.flower.enabled``.
+``ingress.enabled`` is now deprecated, but will still continue to control them both.
+
+Flower disabled by default
+""""""""""""""""""""""""""
+
+Flower is no longer enabled by default when using CeleryExecutor. If you'd like to deploy it, set
+``flower.enabed`` to true in your values file.
+
 New Features
 ^^^^^^^^^^^^
 

diff --git a/chart/templates/NOTES.txt b/chart/templates/NOTES.txt
@@ -23,7 +23,7 @@ Thank you for installing Apache {{ title .Chart.Name }} {{ .Values.airflowVersio
 
 Your release is named {{ .Release.Name }}.
 
-{{- if .Values.ingress.enabled }}
+{{- if or .Values.ingress.web.enabled .Values.ingress.flower.enabled .Values.ingress.enabled }}
 You can now access your service(s) by following defined Ingress urls:
 
 {{- if .Values.ingress.web.host }}
@@ -59,6 +59,16 @@ DEPRECATION WARNING:
 
 {{- end }}
 
+{{- if .Values.ingress.enabled }}
+
+DEPRECATION WARNING:
+   `ingress.enabled` has been deprecated. There are now separate flags to control the webserver and
+   flower individually, ``ingress.web.enabled`` and ``ingress.flower.enabled``.
+   Please change your values as support for the old name will be dropped in a future release.
+
+{{- end }}
+
+{{- if or .Values.ingress.web.enabled .Values.ingress.enabled }}
 Airflow Webserver:
 {{- range .Values.ingress.web.hosts | default (list .Values.ingress.web.host) }}
       {{- $tlsEnabled := $.Values.ingress.web.tls.enabled -}}
@@ -71,7 +81,8 @@ Airflow Webserver:
       {{- end }}
       http{{ if $tlsEnabled }}s{{ end }}://{{ $hostname }}{{ $.Values.ingress.web.path }}/
 {{- end }}
-{{- if or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor") }}
+{{- end }}
+{{- if and (or .Values.ingress.flower.enabled .Values.ingress.enabled) (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }}
 Flower dashboard:
 {{- range .Values.ingress.flower.hosts | default (list .Values.ingress.flower.host) }}
       {{- $tlsEnabled := $.Values.ingress.flower.tls.enabled -}}

diff --git a/chart/templates/flower/flower-ingress.yaml b/chart/templates/flower/flower-ingress.yaml
@@ -19,7 +19,7 @@
 ## Airflow Flower Ingress
 #################################
 {{- if .Values.flower.enabled }}
-{{- if and .Values.ingress.enabled (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }}
+{{- if and (or .Values.ingress.flower.enabled .Values.ingress.enabled) (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }}
 {{- $apiIsStable := semverCompare ">= 1.19.x" (include "kubeVersion" .) -}}
 {{- if $apiIsStable }}
 apiVersion: networking.k8s.io/v1

diff --git a/chart/templates/webserver/webserver-ingress.yaml b/chart/templates/webserver/webserver-ingress.yaml
@@ -18,7 +18,7 @@
 ################################
 ## Airflow Webserver Ingress
 #################################
-{{- if .Values.ingress.enabled }}
+{{- if or .Values.ingress.web.enabled .Values.ingress.enabled }}
 {{- $apiIsStable := semverCompare ">= 1.19.x" (include "kubeVersion" .) -}}
 {{- if $apiIsStable }}
 apiVersion: networking.k8s.io/v1

diff --git a/chart/values.schema.json b/chart/values.schema.json
@@ -141,15 +141,23 @@
             "x-docsSection": "Ingress",
             "properties": {
                 "enabled": {
-                    "description": "Enable ingress resource.",
-                    "type": "boolean",
-                    "default": false
+                    "description": "Enable all ingress resources (deprecated - use ingress.web.enabled and ingress.flower.enabled).",
+                    "type": [
+                        "boolean",
+                        "null"
+                    ],
+                    "default": null
                 },
                 "web": {
                     "description": "Configuration for the Ingress of the web Service.",
                     "type": "object",
                     "additionalProperties": false,
                     "properties": {
+                        "enabled": {
+                            "description": "Enable web ingress resource.",
+                            "type": "boolean",
+                            "default": false
+                        },
                         "annotations": {
                             "description": "Annotations for the web Ingress.",
                             "type": "object",
@@ -257,6 +265,11 @@
                     "type": "object",
                     "additionalProperties": false,
                     "properties": {
+                        "enabled": {
+                            "description": "Enable flower ingress resource.",
+                            "type": "boolean",
+                            "default": false
+                        },
                         "annotations": {
                             "description": "Annotations for the flower Ingress.",
                             "type": "object",
@@ -2939,7 +2952,7 @@
                 "enabled": {
                     "description": "Enable Flower.",
                     "type": "boolean",
-                    "default": true
+                    "default": false
                 },
                 "command": {
                     "description": "Command to use when running flower (templated).",

diff --git a/chart/values.yaml b/chart/values.yaml
@@ -104,11 +104,14 @@ labels: {}
 
 # Ingress configuration
 ingress:
-  # Enable ingress resource
-  enabled: false
+  # Enable all ingress resources (deprecated - use ingress.web.enabled and ingress.flower.enabled)
+  enabled: ~
 
   # Configs for the Ingress of the web Service
   web:
+    # Enable web ingress resource
+    enabled: false
+
     # Annotations for the web Ingress
     annotations: {}
 
@@ -149,6 +152,9 @@ ingress:
 
   # Configs for the Ingress of the flower Service
   flower:
+    # Enable web ingress resource
+    enabled: false
+
     # Annotations for the flower Ingress
     annotations: {}
 
@@ -1041,7 +1047,7 @@ triggerer:
 flower:
   # Enable flower.
   # If True, and using CeleryExecutor/CeleryKubernetesExecutor, will deploy flower app.
-  enabled: true
+  enabled: false
 
   # Command to use when running flower (templated).
   command: ~

diff --git a/tests/charts/test_airflow_common.py b/tests/charts/test_airflow_common.py
@@ -99,6 +99,7 @@ def test_annotations(self):
             values={
                 "airflowPodAnnotations": {"test-annotation/safe-to-evict": "true"},
                 "cleanup": {"enabled": True},
+                "flower": {"enabled": True},
             },
             show_only=[
                 "templates/scheduler/scheduler-deployment.yaml",
@@ -128,6 +129,7 @@ def test_global_affinity_tolerations_topology_spread_constraints_and_node_select
         k8s_objects = render_chart(
             values={
                 "cleanup": {"enabled": True},
+                "flower": {"enabled": True},
                 "pgbouncer": {"enabled": True},
                 "affinity": {
                     "nodeAffinity": {
@@ -311,8 +313,8 @@ def test_have_all_config_mounts_on_init_containers(self):
     def test_priority_class_name(self):
         docs = render_chart(
             values={
-                "flower": {"priorityClassName": "low-priority-flower"},
-                "pgbouncer": {"priorityClassName": "low-priority-pgbouncer"},
+                "flower": {"enabled": True, "priorityClassName": "low-priority-flower"},
+                "pgbouncer": {"enabled": True, "priorityClassName": "low-priority-pgbouncer"},
                 "scheduler": {"priorityClassName": "low-priority-scheduler"},
                 "statsd": {"priorityClassName": "low-priority-statsd"},
                 "triggerer": {"priorityClassName": "low-priority-triggerer"},
@@ -330,6 +332,7 @@ def test_priority_class_name(self):
             ],
         )
 
+        assert 7 == len(docs)
         for doc in docs:
             component = doc['metadata']['labels']['component']
             priority = doc['spec']['template']['spec']['priorityClassName']

diff --git a/tests/charts/test_annotations.py b/tests/charts/test_annotations.py
@@ -102,6 +102,7 @@ class TestServiceAccountAnnotations:
             (
                 {
                     "flower": {
+                        "enabled": True,
                         "serviceAccount": {
                             "annotations": {
                                 "example": "flower",
@@ -270,6 +271,7 @@ def test_annotations_are_added(self, values, show_only, expected_annotations):
         (
             {
                 "flower": {
+                    "enabled": True,
                     "podAnnotations": {
                         "example": "flower",
                     },

diff --git a/tests/charts/test_basic_helm_chart.py b/tests/charts/test_basic_helm_chart.py
@@ -25,7 +25,7 @@
 
 from tests.charts.helm_template_generator import render_chart
 
-OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 38
+OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 35
 
 
 class TestBaseChartTest(unittest.TestCase):
@@ -45,7 +45,6 @@ def test_basic_deployments(self):
         }
         assert list_of_kind_names_tuples == {
             ('ServiceAccount', 'TEST-BASIC-create-user-job'),
-            ('ServiceAccount', 'TEST-BASIC-flower'),
             ('ServiceAccount', 'TEST-BASIC-migrate-database-job'),
             ('ServiceAccount', 'TEST-BASIC-redis'),
             ('ServiceAccount', 'TEST-BASIC-scheduler'),
@@ -65,14 +64,12 @@ def test_basic_deployments(self):
             ('Role', 'TEST-BASIC-pod-log-reader-role'),
             ('RoleBinding', 'TEST-BASIC-pod-launcher-rolebinding'),
             ('RoleBinding', 'TEST-BASIC-pod-log-reader-rolebinding'),
-            ('Service', 'TEST-BASIC-flower'),
             ('Service', 'TEST-BASIC-postgresql-headless'),
             ('Service', 'TEST-BASIC-postgresql'),
             ('Service', 'TEST-BASIC-redis'),
             ('Service', 'TEST-BASIC-statsd'),
             ('Service', 'TEST-BASIC-webserver'),
             ('Service', 'TEST-BASIC-worker'),
-            ('Deployment', 'TEST-BASIC-flower'),
             ('Deployment', 'TEST-BASIC-scheduler'),
             ('Deployment', 'TEST-BASIC-statsd'),
             ('Deployment', 'TEST-BASIC-triggerer'),
@@ -114,6 +111,7 @@ def test_network_policies_are_valid(self):
             {
                 "networkPolicies": {"enabled": True},
                 "executor": "CeleryExecutor",
+                "flower": {"enabled": True},
                 "pgbouncer": {"enabled": True},
             },
         )
@@ -146,6 +144,7 @@ def test_labels_are_valid(self):
                 "ingress": {"enabled": True},
                 "networkPolicies": {"enabled": True},
                 "cleanup": {"enabled": True},
+                "flower": {"enabled": True},
                 "logs": {"persistence": {"enabled": True}},
                 "dags": {"persistence": {"enabled": True}},
                 "postgresql": {"enabled": False},  # We won't check the objects created by the postgres chart
@@ -240,6 +239,7 @@ def test_labels_are_valid_on_job_templates(self):
                 "redis": {"enabled": True},
                 "networkPolicies": {"enabled": True},
                 "cleanup": {"enabled": True},
+                "flower": {"enabled": True},
                 "postgresql": {"enabled": False},  # We won't check the objects created by the postgres chart
             },
         )
@@ -271,7 +271,10 @@ def test_annotations_on_airflow_pods_in_deployment(self):
         release_name = "TEST-BASIC"
         k8s_objects = render_chart(
             name=release_name,
-            values={"airflowPodAnnotations": {"test-annotation/safe-to-evict": "true"}},
+            values={
+                "airflowPodAnnotations": {"test-annotation/safe-to-evict": "true"},
+                "flower": {"enabled": True},
+            },
             show_only=[
                 "templates/scheduler/scheduler-deployment.yaml",
                 "templates/workers/worker-deployment.yaml",

diff --git a/tests/charts/test_extra_env_env_from.py b/tests/charts/test_extra_env_env_from.py
@@ -70,7 +70,8 @@ class ExtraEnvEnvFromTest(unittest.TestCase):
     def setUpClass(cls) -> None:
         values_str = textwrap.dedent(
             """
-            executor: "CeleryExecutor"
+            flower:
+              enabled: true
             extraEnvFrom: |
               - secretRef:
                   name: '{{ .Release.Name }}-airflow-connections'