Skip to content

Commit

Permalink
fix(helm): render global volumes and volume mounts into cleanup job (#…
Browse files Browse the repository at this point in the history
…40191) (#42268)

* fix(helm): render global volumes and volume mounts in cleanup job (#64056)

* fix(helm): allow to deactivate cleanup job history (#64056)
  • Loading branch information
dada-engineer authored Sep 17, 2024
1 parent 354467f commit 20a481c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
13 changes: 10 additions & 3 deletions chart/templates/cleanup/cleanup-cronjob.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ spec:
schedule: "{{ tpl .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
concurrencyPolicy: Forbid
{{- if .Values.cleanup.failedJobsHistoryLimit }}
{{- if not ( eq .Values.cleanup.failedJobsHistoryLimit nil) }}
failedJobsHistoryLimit: {{ .Values.cleanup.failedJobsHistoryLimit }}
{{- end }}
{{- if .Values.cleanup.successfulJobsHistoryLimit }}
{{- if not (eq .Values.cleanup.successfulJobsHistoryLimit nil) }}
successfulJobsHistoryLimit: {{ .Values.cleanup.successfulJobsHistoryLimit }}
{{- end }}
jobTemplate:
Expand Down Expand Up @@ -105,10 +105,17 @@ spec:
env:
{{- include "standard_airflow_environment" . | indent 12 }}
{{- include "container_extra_envs" (list . .Values.cleanup.env) | indent 12 }}
volumeMounts: {{- include "airflow_config_mount" . | nindent 16 }}
volumeMounts:
{{- include "airflow_config_mount" . | nindent 16 }}
{{- if .Values.volumeMounts }}
{{- toYaml .Values.volumeMounts | nindent 16 }}
{{- end }}
resources: {{- toYaml .Values.cleanup.resources | nindent 16 }}
volumes:
- name: config
configMap:
name: {{ template "airflow_config" . }}
{{- if .Values.volumes }}
{{- toYaml .Values.volumes | nindent 12 }}
{{- end }}
{{- end }}
34 changes: 34 additions & 0 deletions helm_tests/airflow_aux/test_cleanup_pods.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ def test_should_create_cronjob_for_enabled_cleanup(self):
"subPath": "airflow.cfg",
"readOnly": True,
} in jmespath.search("spec.jobTemplate.spec.template.spec.containers[0].volumeMounts", docs[0])
assert "successfulJobsHistoryLimit" not in docs[0]["spec"]
assert "failedJobsHistoryLimit" not in docs[0]["spec"]

def test_should_pass_validation_with_v1beta1_api(self):
render_chart(
Expand Down Expand Up @@ -327,6 +329,20 @@ def test_should_set_job_history_limits(self):
assert 2 == jmespath.search("spec.failedJobsHistoryLimit", docs[0])
assert 4 == jmespath.search("spec.successfulJobsHistoryLimit", docs[0])

def test_should_set_zero_job_history_limits(self):
docs = render_chart(
values={
"cleanup": {
"enabled": True,
"failedJobsHistoryLimit": 0,
"successfulJobsHistoryLimit": 0,
},
},
show_only=["templates/cleanup/cleanup-cronjob.yaml"],
)
assert 0 == jmespath.search("spec.failedJobsHistoryLimit", docs[0])
assert 0 == jmespath.search("spec.successfulJobsHistoryLimit", docs[0])

def test_no_airflow_local_settings(self):
docs = render_chart(
values={
Expand Down Expand Up @@ -355,6 +371,24 @@ def test_airflow_local_settings(self):
"readOnly": True,
} in jmespath.search("spec.jobTemplate.spec.template.spec.containers[0].volumeMounts", docs[0])

def test_global_volumes_and_volume_mounts(self):
docs = render_chart(
values={
"cleanup": {"enabled": True},
"volumes": [{"name": "test-volume", "emptyDir": {}}],
"volumeMounts": [{"name": "test-volume", "mountPath": "/test"}],
},
show_only=["templates/cleanup/cleanup-cronjob.yaml"],
)
assert {
"name": "test-volume",
"mountPath": "/test",
} in jmespath.search("spec.jobTemplate.spec.template.spec.containers[0].volumeMounts", docs[0])
assert {
"name": "test-volume",
"emptyDir": {},
} in jmespath.search("spec.jobTemplate.spec.template.spec.volumes", docs[0])


class TestCleanupServiceAccount:
"""Tests cleanup of service accounts."""
Expand Down

0 comments on commit 20a481c

Please sign in to comment.