From c833f6c418241fcc22a57f919845fdab30d6fb52 Mon Sep 17 00:00:00 2001 From: Berenger Nguyen Nhon Date: Tue, 14 Mar 2023 17:36:04 +0100 Subject: [PATCH 1/8] add cleanup.jobAnnotations --- chart/templates/cleanup/cleanup-cronjob.yaml | 3 +++ chart/values.schema.json | 8 ++++++++ chart/values.yaml | 2 ++ 3 files changed, 13 insertions(+) diff --git a/chart/templates/cleanup/cleanup-cronjob.yaml b/chart/templates/cleanup/cleanup-cronjob.yaml index 2a7dd3d608181..43371855cca25 100644 --- a/chart/templates/cleanup/cleanup-cronjob.yaml +++ b/chart/templates/cleanup/cleanup-cronjob.yaml @@ -41,6 +41,9 @@ metadata: {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} + {{- if .Values.cleanup.jobAnnotations }} + annotations: {{- .Values.cleanup.jobAnnotations | toYaml | nindent 4 }} + {{- end }} spec: schedule: "{{ .Values.cleanup.schedule }}" # The cron job does not allow concurrent runs; if it is time for a new job run and the previous job run hasn't finished yet, the cron job skips the new job run diff --git a/chart/values.schema.json b/chart/values.schema.json index 4131ec60d7863..dd9251a77bb42 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -5090,6 +5090,14 @@ "exec airflow kubernetes cleanup-pods --namespace={{ .Release.Namespace }}" ] }, + "jobAnnotations": { + "description": "Annotations to add to the cleanup cronjob.", + "type": "object", + "default": {}, + "additionalProperties": { + "type": "string" + } + }, "nodeSelector": { "description": "Select certain nodes for cleanup pods.", "type": "object", diff --git a/chart/values.yaml b/chart/values.yaml index 7b157acda6765..92c076a2f8782 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1772,6 +1772,8 @@ cleanup: # Args to use when running the cleanup cronjob (templated). args: ["bash", "-c", "exec airflow kubernetes cleanup-pods --namespace={{ .Release.Namespace }}"] + # jobAnnotations are annotations on the cleanup CronJob + jobAnnotations: {} # Select certain nodes for airflow cleanup pods. nodeSelector: {} From b1907036a2e0a664d6c2214a3ec248442737c0e4 Mon Sep 17 00:00:00 2001 From: Berenger Nguyen Nhon Date: Tue, 14 Mar 2023 17:43:17 +0100 Subject: [PATCH 2/8] add statsd.annotations --- chart/templates/statsd/statsd-deployment.yaml | 3 +++ chart/values.schema.json | 8 ++++++++ chart/values.yaml | 3 +++ 3 files changed, 14 insertions(+) diff --git a/chart/templates/statsd/statsd-deployment.yaml b/chart/templates/statsd/statsd-deployment.yaml index 4772a1f3c6db8..c8216102281ad 100644 --- a/chart/templates/statsd/statsd-deployment.yaml +++ b/chart/templates/statsd/statsd-deployment.yaml @@ -38,6 +38,9 @@ metadata: {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} + {{- if .Values.statsd.annotations }} + annotations: {{- toYaml .Values.statsd.annotations | nindent 4 }} + {{- end }} spec: replicas: 1 {{- if $revisionHistoryLimit }} diff --git a/chart/values.schema.json b/chart/values.schema.json index dd9251a77bb42..5d055386bd086 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -4245,6 +4245,14 @@ "type": "string" } }, + "annotations": { + "description": "Annotations to add to the StatsD deployment.", + "type": "object", + "default": {}, + "additionalProperties": { + "type": "string" + } + }, "args": { "description": "Args to use when running statsd-exporter (templated).", "type": [ diff --git a/chart/values.yaml b/chart/values.yaml index 92c076a2f8782..e07b74c1df611 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1470,6 +1470,9 @@ statsd: # Arguments for StatsD exporter command. args: ["--statsd.mapping-config=/etc/statsd-exporter/mappings.yml"] + # Annotations to add to the StatsD Deployment. + annotations: {} + # Create ServiceAccount serviceAccount: # Specifies whether a ServiceAccount should be created From abc8f6f414074c4dc80c82b89ff5346875872d50 Mon Sep 17 00:00:00 2001 From: Berenger Nguyen Nhon Date: Tue, 14 Mar 2023 17:43:37 +0100 Subject: [PATCH 3/8] helm chart unit tests for added annotations --- tests/charts/test_annotations.py | 50 ++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/charts/test_annotations.py b/tests/charts/test_annotations.py index 841dd5d3010ba..f2d774f592f8d 100644 --- a/tests/charts/test_annotations.py +++ b/tests/charts/test_annotations.py @@ -408,3 +408,53 @@ def test_precedence(self, values, show_only, expected_annotations): for k, v in expected_annotations.items(): assert k in annotations assert v == annotations[k] + +class TestPerComponentControllerAnnotations: + @pytest.mark.parametrize( + "values,show_only,expected_annotations", + [ + ( + { + "cleanup": { + "enabled": True, + "jobAnnotations": { + "example": "cleanup" + } + } + }, + "templates/cleanup/cleanup-cronjob.yaml", + { + "example": "cleanup" + } + ), + ( + { + "statsd": { + "enabled": True, + "annotations": { + "example": "statsd" + } + } + }, + "templates/statsd/statsd-deployment.yaml", + { + "example": "statsd" + } + ), + ] + ) + def test_annotations_are_added(self, values, show_only, expected_annotations): + k8s_objects = render_chart( + values=values, + show_only=[show_only], + ) + # This test relies on the convention that the helm chart puts a single + # controller (Deployment/StatefulSet/Job/Cronjob) in its own .yaml file + # so by specifying `show_only`, we should only get a single k8s_object + # here - the target object that we hope to test on. + assert len(k8s_objects) == 1 + obj = k8s_objects[0] + + for k, v in expected_annotations.items(): + assert k in obj["metadata"]["annotations"] + assert v == obj["metadata"]["annotations"][k] From 6fea20c8b4e88569f3bc8f752186ef37da5faa8b Mon Sep 17 00:00:00 2001 From: Berenger Nguyen Nhon Date: Wed, 15 Mar 2023 16:03:09 +0100 Subject: [PATCH 4/8] move annotations test to component specific module --- tests/charts/test_annotations.py | 50 -------------------------------- tests/charts/test_statsd.py | 12 ++++++++ 2 files changed, 12 insertions(+), 50 deletions(-) diff --git a/tests/charts/test_annotations.py b/tests/charts/test_annotations.py index f2d774f592f8d..841dd5d3010ba 100644 --- a/tests/charts/test_annotations.py +++ b/tests/charts/test_annotations.py @@ -408,53 +408,3 @@ def test_precedence(self, values, show_only, expected_annotations): for k, v in expected_annotations.items(): assert k in annotations assert v == annotations[k] - -class TestPerComponentControllerAnnotations: - @pytest.mark.parametrize( - "values,show_only,expected_annotations", - [ - ( - { - "cleanup": { - "enabled": True, - "jobAnnotations": { - "example": "cleanup" - } - } - }, - "templates/cleanup/cleanup-cronjob.yaml", - { - "example": "cleanup" - } - ), - ( - { - "statsd": { - "enabled": True, - "annotations": { - "example": "statsd" - } - } - }, - "templates/statsd/statsd-deployment.yaml", - { - "example": "statsd" - } - ), - ] - ) - def test_annotations_are_added(self, values, show_only, expected_annotations): - k8s_objects = render_chart( - values=values, - show_only=[show_only], - ) - # This test relies on the convention that the helm chart puts a single - # controller (Deployment/StatefulSet/Job/Cronjob) in its own .yaml file - # so by specifying `show_only`, we should only get a single k8s_object - # here - the target object that we hope to test on. - assert len(k8s_objects) == 1 - obj = k8s_objects[0] - - for k, v in expected_annotations.items(): - assert k in obj["metadata"]["annotations"] - assert v == obj["metadata"]["annotations"][k] diff --git a/tests/charts/test_statsd.py b/tests/charts/test_statsd.py index 7e7dbbab7c90d..3e92ae3079ed3 100644 --- a/tests/charts/test_statsd.py +++ b/tests/charts/test_statsd.py @@ -222,3 +222,15 @@ def test_statsd_args_can_be_overridden(self): ) assert jmespath.search("spec.template.spec.containers[0].args", docs[0]) == args + + def test_should_add_component_specific_annotations(self): + docs = render_chart( + values={ + "statsd": { + "annotations": {"test_annotation": "test_annotation_value"}, + }, + }, + show_only=["templates/statsd/statsd-deployment.yaml"], + ) + assert "annotations" in jmespath.search("metadata", docs[0]) + assert jmespath.search("metadata.annotations", docs[0])["test_annotation"] == "test_annotation_value" From 4e299eb815dca9afa6b25e0f4779ed01724e267b Mon Sep 17 00:00:00 2001 From: Berenger Nguyen Nhon Date: Wed, 15 Mar 2023 16:10:13 +0100 Subject: [PATCH 5/8] pre-commit hook format --- tests/charts/test_cleanup_pods.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/charts/test_cleanup_pods.py b/tests/charts/test_cleanup_pods.py index f6b15a5f6b462..c7d3956506ca0 100644 --- a/tests/charts/test_cleanup_pods.py +++ b/tests/charts/test_cleanup_pods.py @@ -200,6 +200,33 @@ def test_should_add_component_specific_labels(self): == "test_label_value" ) + def test_should_add_component_specific_annotations(self): + docs = render_chart( + values={ + "cleanup": { + "enabled": True, + "jobAnnotations": {"test_cronjob_annotation": "test_cronjob_annotation_value"}, + "podAnnotations": {"test_pod_annotation": "test_pod_annotation_value"}, + }, + }, + show_only=["templates/cleanup/cleanup-cronjob.yaml"], + ) + + assert "test_cronjob_annotation" in jmespath.search("metadata.annotations", docs[0]) + assert ( + "test_cronjob_annotation_value" + == jmespath.search("metadata.annotations", docs[0])["test_cronjob_annotation"] + ) + assert "test_pod_annotation" in jmespath.search( + "spec.jobTemplate.spec.template.metadata.annotations", docs[0] + ) + assert ( + "test_pod_annotation_value" + == jmespath.search("spec.jobTemplate.spec.template.metadata.annotations", docs[0])[ + "test_pod_annotation" + ] + ) + def test_cleanup_resources_are_configurable(self): resources = { "requests": { From c5bc4e6517e739d7b3ec0328873fdebb7c0576ea Mon Sep 17 00:00:00 2001 From: Berenger Nguyen Nhon Date: Wed, 15 Mar 2023 16:11:02 +0100 Subject: [PATCH 6/8] pre-commit hook format --- tests/charts/test_statsd.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/charts/test_statsd.py b/tests/charts/test_statsd.py index 3e92ae3079ed3..4358342cd52c4 100644 --- a/tests/charts/test_statsd.py +++ b/tests/charts/test_statsd.py @@ -228,9 +228,15 @@ def test_should_add_component_specific_annotations(self): values={ "statsd": { "annotations": {"test_annotation": "test_annotation_value"}, + "podAnnotations": {"test_pod_annotation": "test_pod_annotation_value"}, }, }, show_only=["templates/statsd/statsd-deployment.yaml"], ) assert "annotations" in jmespath.search("metadata", docs[0]) assert jmespath.search("metadata.annotations", docs[0])["test_annotation"] == "test_annotation_value" + assert "test_pod_annotation" in jmespath.search("spec.template.metadata.annotations", docs[0]) + assert ( + jmespath.search("spec.template.metadata.annotations", docs[0])["test_pod_annotation"] + == "test_pod_annotation_value" + ) From 6ceb8bf6d44e291f6dcb78f7d047b3f3101db2d0 Mon Sep 17 00:00:00 2001 From: bnguyennhon <78416241+bnguyennhon@users.noreply.github.com> Date: Thu, 16 Mar 2023 09:45:26 +0100 Subject: [PATCH 7/8] Update chart/templates/cleanup/cleanup-cronjob.yaml Co-authored-by: Hussein Awala --- chart/templates/cleanup/cleanup-cronjob.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chart/templates/cleanup/cleanup-cronjob.yaml b/chart/templates/cleanup/cleanup-cronjob.yaml index 43371855cca25..adbce47a8a724 100644 --- a/chart/templates/cleanup/cleanup-cronjob.yaml +++ b/chart/templates/cleanup/cleanup-cronjob.yaml @@ -41,8 +41,8 @@ metadata: {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} - {{- if .Values.cleanup.jobAnnotations }} - annotations: {{- .Values.cleanup.jobAnnotations | toYaml | nindent 4 }} + {{- with .Values.cleanup.jobAnnotations }} + annotations: {{- toYaml . | nindent 4 }} {{- end }} spec: schedule: "{{ .Values.cleanup.schedule }}" From cb6409a3380ea57110100a26e11817c5689a10ca Mon Sep 17 00:00:00 2001 From: bnguyennhon <78416241+bnguyennhon@users.noreply.github.com> Date: Thu, 16 Mar 2023 09:46:00 +0100 Subject: [PATCH 8/8] Update chart/templates/statsd/statsd-deployment.yaml Co-authored-by: Hussein Awala --- chart/templates/statsd/statsd-deployment.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chart/templates/statsd/statsd-deployment.yaml b/chart/templates/statsd/statsd-deployment.yaml index c8216102281ad..856243b80add4 100644 --- a/chart/templates/statsd/statsd-deployment.yaml +++ b/chart/templates/statsd/statsd-deployment.yaml @@ -38,8 +38,8 @@ metadata: {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} - {{- if .Values.statsd.annotations }} - annotations: {{- toYaml .Values.statsd.annotations | nindent 4 }} + {{- with .Values.statsd.annotations }} + annotations: {{- toYaml . | nindent 4 }} {{- end }} spec: replicas: 1