You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by po...@apache.org on 2022/09/27 18:42:20 UTC

[airflow] branch main updated: Add statsd overrideMappings in helm chart values (#26598)

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new 8da6a9c5d7 Add statsd overrideMappings in helm chart values (#26598)
8da6a9c5d7 is described below

commit 8da6a9c5d7009c210a3b488eeb1cb703ad47a351
Author: Bob Du <i...@bobdu.cc>
AuthorDate: Wed Sep 28 02:42:12 2022 +0800

    Add statsd overrideMappings in helm chart values (#26598)
    
    * add statsd overrideMappings in helm chart values
    
    * add statsd overrideMappings unit test in helm chart
    
    * fix statsd configmap unit test in helm chart
---
 chart/dockerfiles/statsd-exporter/Dockerfile       | 31 ---------
 .../dockerfiles/statsd-exporter/build_and_push.sh  | 61 ------------------
 .../mappings.yml => files/statsd-mappings.yml}     |  0
 chart/templates/configmaps/statsd-configmap.yaml   | 11 +++-
 chart/templates/statsd/statsd-deployment.yaml      |  4 --
 chart/values.schema.json                           | 11 +++-
 chart/values.yaml                                  | 11 +++-
 scripts/ci/pre_commit/pre_commit_chart_schema.py   |  1 +
 tests/charts/test_basic_helm_chart.py              | 21 +++++-
 tests/charts/test_rbac.py                          |  1 +
 tests/charts/test_statsd.py                        | 75 ++++++++++++++++++++++
 11 files changed, 123 insertions(+), 104 deletions(-)

diff --git a/chart/dockerfiles/statsd-exporter/Dockerfile b/chart/dockerfiles/statsd-exporter/Dockerfile
deleted file mode 100644
index d255a3e4fe..0000000000
--- a/chart/dockerfiles/statsd-exporter/Dockerfile
+++ /dev/null
@@ -1,31 +0,0 @@
-# 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.
-ARG STATSD_VERSION="missing_version"
-
-FROM prom/statsd-exporter:${STATSD_VERSION}
-
-ARG STATSD_VERSION
-ARG AIRFLOW_STATSD_EXPORTER_VERSION
-ARG COMMIT_SHA
-
-LABEL org.apache.airflow.component="statsd-exporter" \
-    org.apache.airflow.stasd.version="${STATSD_VERSION}" \
-    org.apache.airflow.airflow-stasd-exporter.version="${AIRFLOW_STATSD_EXPORTER_VERSION}" \
-    org.apache.airflow.commit-sha="${COMMIT_SHA}" \
-    maintainer="Apache Airflow Community <de...@airflow.apache.org>"
-
-COPY mappings.yml /etc/statsd-exporter/mappings.yml
diff --git a/chart/dockerfiles/statsd-exporter/build_and_push.sh b/chart/dockerfiles/statsd-exporter/build_and_push.sh
deleted file mode 100755
index 585ee9e408..0000000000
--- a/chart/dockerfiles/statsd-exporter/build_and_push.sh
+++ /dev/null
@@ -1,61 +0,0 @@
-#!/usr/bin/env bash
-# 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.
-set -euo pipefail
-DOCKERHUB_USER=${DOCKERHUB_USER:="apache"}
-readonly DOCKERHUB_USER
-DOCKERHUB_REPO=${DOCKERHUB_REPO:="airflow"}
-readonly DOCKERHUB_REPO
-
-STATSD_VERSION="v0.17.0"
-readonly STATSD_VERSION
-
-AIRFLOW_STATSD_EXPORTER_VERSION="2021.04.28"
-readonly AIRFLOW_STATSD_EXPORTER_VERSION
-
-COMMIT_SHA=$(git rev-parse HEAD)
-readonly COMMIT_SHA
-
-TAG="${DOCKERHUB_USER}/${DOCKERHUB_REPO}:airflow-statsd-exporter-${AIRFLOW_STATSD_EXPORTER_VERSION}-${STATSD_VERSION}"
-readonly TAG
-
-function center_text() {
-    columns=$(tput cols || echo 80)
-    printf "%*s\n" $(( (${#1} + columns) / 2)) "$1"
-}
-
-cd "$( dirname "${BASH_SOURCE[0]}" )" || exit 1
-
-center_text "Building image"
-
-docker build . \
-    --pull \
-    --build-arg "STATSD_VERSION=${STATSD_VERSION}" \
-    --build-arg "AIRFLOW_STATSD_EXPORTER_VERSION=${AIRFLOW_STATSD_EXPORTER_VERSION}" \
-    --build-arg "COMMIT_SHA=${COMMIT_SHA}" \
-    --tag "${TAG}"
-
-center_text "Checking image"
-
-docker run --rm "${TAG}" --version
-
-echo Image labels:
-docker inspect "${TAG}" --format '{{ json .ContainerConfig.Labels }}' | python3 -m json.tool
-
-center_text "Pushing image"
-
-docker push "${TAG}"
diff --git a/chart/dockerfiles/statsd-exporter/mappings.yml b/chart/files/statsd-mappings.yml
similarity index 100%
rename from chart/dockerfiles/statsd-exporter/mappings.yml
rename to chart/files/statsd-mappings.yml
diff --git a/chart/templates/configmaps/statsd-configmap.yaml b/chart/templates/configmaps/statsd-configmap.yaml
index 69d80990a8..2139d4ac06 100644
--- a/chart/templates/configmaps/statsd-configmap.yaml
+++ b/chart/templates/configmaps/statsd-configmap.yaml
@@ -18,7 +18,7 @@
 ################################
 ## Airflow StatsD ConfigMap
 #################################
-{{- if and .Values.statsd.enabled .Values.statsd.extraMappings }}
+{{- if and .Values.statsd.enabled }}
 apiVersion: v1
 kind: ConfigMap
 metadata:
@@ -34,6 +34,13 @@ metadata:
 {{- end }}
 data:
   mappings.yml: |-
-{{ .Files.Get "dockerfiles/statsd-exporter/mappings.yml" | indent 4 }}
+{{- if .Values.statsd.overrideMappings }}
+    mappings:
+{{ toYaml .Values.statsd.overrideMappings | indent 6 }}
+{{- else }}
+{{ .Files.Get "files/statsd-mappings.yml" | indent 4 }}
+{{- if .Values.statsd.extraMappings }}
 {{ toYaml .Values.statsd.extraMappings | indent 6 }}
 {{- end }}
+{{- end }}
+{{- end }}
diff --git a/chart/templates/statsd/statsd-deployment.yaml b/chart/templates/statsd/statsd-deployment.yaml
index 47ddf5beb0..58ac7cc547 100644
--- a/chart/templates/statsd/statsd-deployment.yaml
+++ b/chart/templates/statsd/statsd-deployment.yaml
@@ -59,9 +59,7 @@ spec:
 {{- end }}
 {{- if or .Values.statsd.extraMappings .Values.statsd.podAnnotations }}
       annotations:
-        {{- if .Values.statsd.extraMappings }}
         checksum/statsd-config: {{ include (print $.Template.BasePath "/configmaps/statsd-configmap.yaml") . | sha256sum }}
-        {{- end }}
         {{- if .Values.statsd.podAnnotations }}
         {{- toYaml .Values.statsd.podAnnotations | nindent 8 }}
         {{- end }}
@@ -113,7 +111,6 @@ spec:
             initialDelaySeconds: 10
             periodSeconds: 10
             timeoutSeconds: 5
-{{- if .Values.statsd.extraMappings }}
           volumeMounts:
             - name: config
               mountPath: /etc/statsd-exporter/mappings.yml
@@ -123,4 +120,3 @@ spec:
           configMap:
             name: {{ .Release.Name }}-statsd
 {{- end }}
-{{- end }}
diff --git a/chart/values.schema.json b/chart/values.schema.json
index eef451ac7a..77852fc435 100644
--- a/chart/values.schema.json
+++ b/chart/values.schema.json
@@ -571,12 +571,12 @@
                         "repository": {
                             "description": "The StatsD image repository.",
                             "type": "string",
-                            "default": "apache/airflow"
+                            "default": "quay.io/prometheus/statsd-exporter"
                         },
                         "tag": {
                             "description": "The StatsD image tag.",
                             "type": "string",
-                            "default": "airflow-statsd-exporter-2021.04.28-v0.17.0"
+                            "default": "v0.22.8"
                         },
                         "pullPolicy": {
                             "description": "The StatsD image pull policy.",
@@ -4117,7 +4117,12 @@
                     }
                 },
                 "extraMappings": {
-                    "description": "Additional mappings for StatsD exporter.",
+                    "description": "Additional mappings for StatsD exporter.If set, will merge default mapping and extra mappings, default mapping has higher priority. So, if you want to change some default mapping, please use `overrideMappings`",
+                    "type": "array",
+                    "default": []
+                },
+                "overrideMappings": {
+                    "description": "Override mappings for StatsD exporter.If set, will ignore setting item in default and `extraMappings`. So, If you use it, ensure all mapping item contains in it.",
                     "type": "array",
                     "default": []
                 },
diff --git a/chart/values.yaml b/chart/values.yaml
index 91f3f63e3f..d6517f4cd5 100644
--- a/chart/values.yaml
+++ b/chart/values.yaml
@@ -76,8 +76,8 @@ images:
     tag: ~
     pullPolicy: IfNotPresent
   statsd:
-    repository: apache/airflow
-    tag: airflow-statsd-exporter-2021.04.28-v0.17.0
+    repository: quay.io/prometheus/statsd-exporter
+    tag: v0.22.8
     pullPolicy: IfNotPresent
   redis:
     repository: redis
@@ -1366,8 +1366,15 @@ statsd:
   priorityClassName: ~
 
   # Additional mappings for StatsD exporter.
+  # If set, will merge default mapping and extra mappings, default mapping has higher priority.
+  # So, if you want to change some default mapping, please use `overrideMappings`
   extraMappings: []
 
+  # Override mappings for StatsD exporter.
+  # If set, will ignore setting item in default and `extraMappings`.
+  # So, If you use it, ensure all mapping item contains in it.
+  overrideMappings: []
+
   podAnnotations: {}
 
 # PgBouncer settings
diff --git a/scripts/ci/pre_commit/pre_commit_chart_schema.py b/scripts/ci/pre_commit/pre_commit_chart_schema.py
index b1bc96dfad..901ffb8334 100755
--- a/scripts/ci/pre_commit/pre_commit_chart_schema.py
+++ b/scripts/ci/pre_commit/pre_commit_chart_schema.py
@@ -36,6 +36,7 @@ KNOWN_INVALID_TYPES = {
     "$['properties']['ingress']['properties']['web']['properties']['succeedingPaths']",
     # The value of this parameter is passed to statsd_exporter, which does not have a strict type definition.
     "$['properties']['statsd']['properties']['extraMappings']",
+    "$['properties']['statsd']['properties']['overrideMappings']",
 }
 VENDORED_PATHS = {
     # We don't want to check the upstream k8s definitions
diff --git a/tests/charts/test_basic_helm_chart.py b/tests/charts/test_basic_helm_chart.py
index 5669cabb8b..e1acb9a284 100644
--- a/tests/charts/test_basic_helm_chart.py
+++ b/tests/charts/test_basic_helm_chart.py
@@ -27,7 +27,7 @@ from parameterized import parameterized
 
 from tests.charts.helm_template_generator import render_chart
 
-OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 34
+OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 35
 
 
 class TestBaseChartTest(unittest.TestCase):
@@ -81,6 +81,7 @@ class TestBaseChartTest(unittest.TestCase):
             ('Secret', 'test-basic-postgresql'),
             ('Secret', 'test-basic-redis-password'),
             ('ConfigMap', 'test-basic-airflow-config'),
+            ('ConfigMap', 'test-basic-statsd'),
             ('Role', 'test-basic-pod-launcher-role'),
             ('Role', 'test-basic-pod-log-reader-role'),
             ('RoleBinding', 'test-basic-pod-launcher-rolebinding'),
@@ -158,6 +159,7 @@ class TestBaseChartTest(unittest.TestCase):
             ('Secret', 'test-basic-postgresql'),
             ('Secret', 'test-basic-redis-password'),
             ('ConfigMap', 'test-basic-airflow-config'),
+            ('ConfigMap', 'test-basic-statsd'),
             ('Role', 'test-basic-pod-launcher-role'),
             ('Role', 'test-basic-pod-log-reader-role'),
             ('RoleBinding', 'test-basic-pod-launcher-rolebinding'),
@@ -208,6 +210,23 @@ class TestBaseChartTest(unittest.TestCase):
         assert ('Job', 'test-basic-create-user') not in list_of_kind_names_tuples
         assert expected_object_count_in_basic_deployment - 2 == len(k8s_objects)
 
+    @parameterized.expand(["2.3.2", "2.4.0", "default"])
+    def test_basic_deployment_without_statsd(self, version):
+        expected_object_count_in_basic_deployment = self._get_object_count(version)
+        k8s_objects = render_chart(
+            "test-basic",
+            values=self._get_values_with_version(values={"statsd": {'enabled': False}}, version=version),
+        )
+        list_of_kind_names_tuples = [
+            (k8s_object['kind'], k8s_object['metadata']['name']) for k8s_object in k8s_objects
+        ]
+        assert ('ServiceAccount', 'test-basic-statsd') not in list_of_kind_names_tuples
+        assert ('ConfigMap', 'test-basic-statsd') not in list_of_kind_names_tuples
+        assert ('Service', 'test-basic-statsd') not in list_of_kind_names_tuples
+        assert ('Deployment', 'test-basic-statsd') not in list_of_kind_names_tuples
+
+        assert expected_object_count_in_basic_deployment - 4 == len(k8s_objects)
+
     def test_network_policies_are_valid(self):
         k8s_objects = render_chart(
             "test-basic",
diff --git a/tests/charts/test_rbac.py b/tests/charts/test_rbac.py
index 11bf2723aa..488bc2b6c8 100644
--- a/tests/charts/test_rbac.py
+++ b/tests/charts/test_rbac.py
@@ -29,6 +29,7 @@ DEPLOYMENT_NO_RBAC_NO_SA_KIND_NAME_TUPLES = [
     ('Secret', 'test-rbac-pgbouncer-config'),
     ('Secret', 'test-rbac-pgbouncer-stats'),
     ('ConfigMap', 'test-rbac-airflow-config'),
+    ('ConfigMap', 'test-rbac-statsd'),
     ('Service', 'test-rbac-postgresql-headless'),
     ('Service', 'test-rbac-postgresql'),
     ('Service', 'test-rbac-statsd'),
diff --git a/tests/charts/test_statsd.py b/tests/charts/test_statsd.py
index a704826bb7..91b51a4a94 100644
--- a/tests/charts/test_statsd.py
+++ b/tests/charts/test_statsd.py
@@ -19,6 +19,7 @@ from __future__ import annotations
 import unittest
 
 import jmespath
+import yaml
 from parameterized import parameterized
 
 from tests.charts.helm_template_generator import render_chart
@@ -32,6 +33,16 @@ class StatsdTest(unittest.TestCase):
 
         assert "statsd" == jmespath.search("spec.template.spec.containers[0].name", docs[0])
 
+        assert {"name": "config", "configMap": {"name": "release-name-statsd"}} in jmespath.search(
+            "spec.template.spec.volumes", docs[0]
+        )
+
+        assert {
+            "name": "config",
+            "mountPath": "/etc/statsd-exporter/mappings.yml",
+            "subPath": "mappings.yml",
+        } in jmespath.search("spec.template.spec.containers[0].volumeMounts", docs[0])
+
     def test_should_add_volume_and_volume_mount_when_exist_extra_mappings(self):
         extra_mapping = {
             "match": "airflow.pool.queued_slots.*",
@@ -53,6 +64,27 @@ class StatsdTest(unittest.TestCase):
             "subPath": "mappings.yml",
         } in jmespath.search("spec.template.spec.containers[0].volumeMounts", docs[0])
 
+    def test_should_add_volume_and_volume_mount_when_exist_override_mappings(self):
+        override_mapping = {
+            "match": "airflow.pool.queued_slots.*",
+            "name": "airflow_pool_queued_slots",
+            "labels": {"pool": "$1"},
+        }
+        docs = render_chart(
+            values={"statsd": {"enabled": True, "overrideMappings": [override_mapping]}},
+            show_only=["templates/statsd/statsd-deployment.yaml"],
+        )
+
+        assert {"name": "config", "configMap": {"name": "release-name-statsd"}} in jmespath.search(
+            "spec.template.spec.volumes", docs[0]
+        )
+
+        assert {
+            "name": "config",
+            "mountPath": "/etc/statsd-exporter/mappings.yml",
+            "subPath": "mappings.yml",
+        } in jmespath.search("spec.template.spec.containers[0].volumeMounts", docs[0])
+
     @parameterized.expand([(8, 10), (10, 8), (8, None), (None, 10), (None, None)])
     def test_revision_history_limit(self, revision_history_limit, global_revision_history_limit):
         values = {"statsd": {"enabled": True}}
@@ -134,3 +166,46 @@ class StatsdTest(unittest.TestCase):
             show_only=["templates/statsd/statsd-deployment.yaml"],
         )
         assert jmespath.search("spec.template.spec.containers[0].resources", docs[0]) == {}
+
+    def test_statsd_configmap_by_default(self):
+        docs = render_chart(show_only=["templates/configmaps/statsd-configmap.yaml"])
+
+        mappings_yml = jmespath.search('data."mappings.yml"', docs[0])
+        mappings_yml_obj = yaml.safe_load(mappings_yml)
+
+        assert "airflow_dagrun_dependency_check" == mappings_yml_obj["mappings"][0]["name"]
+        assert "airflow_pool_starving_tasks" == mappings_yml_obj["mappings"][-1]["name"]
+
+    def test_statsd_configmap_when_exist_extra_mappings(self):
+        extra_mapping = {
+            "match": "airflow.pool.queued_slots.*",
+            "name": "airflow_pool_queued_slots",
+            "labels": {"pool": "$1"},
+        }
+        docs = render_chart(
+            values={"statsd": {"enabled": True, "extraMappings": [extra_mapping]}},
+            show_only=["templates/configmaps/statsd-configmap.yaml"],
+        )
+
+        mappings_yml = jmespath.search('data."mappings.yml"', docs[0])
+        mappings_yml_obj = yaml.safe_load(mappings_yml)
+
+        assert "airflow_dagrun_dependency_check" == mappings_yml_obj["mappings"][0]["name"]
+        assert "airflow_pool_queued_slots" == mappings_yml_obj["mappings"][-1]["name"]
+
+    def test_statsd_configmap_when_exist_override_mappings(self):
+        override_mapping = {
+            "match": "airflow.pool.queued_slots.*",
+            "name": "airflow_pool_queued_slots",
+            "labels": {"pool": "$1"},
+        }
+        docs = render_chart(
+            values={"statsd": {"enabled": True, "overrideMappings": [override_mapping]}},
+            show_only=["templates/configmaps/statsd-configmap.yaml"],
+        )
+
+        mappings_yml = jmespath.search('data."mappings.yml"', docs[0])
+        mappings_yml_obj = yaml.safe_load(mappings_yml)
+
+        assert 1 == len(mappings_yml_obj["mappings"])
+        assert "airflow_pool_queued_slots" == mappings_yml_obj["mappings"][0]["name"]