Skip to content

fix(helm-chart): fix go-template if statements for log groomer retention values #66012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jscheffl merged 3 commits into from
Apr 28, 2026
Merged

fix(helm-chart): fix go-template if statements for log groomer retention values #66012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f1ce08
fix(helm-chart): fix go-template if statements for log groomer retent…
n-badtke-cg Apr 28, 2026
8ee3a63
refactor(helm-chart): simplify go-template if statements for log groo…
n-badtke-cg Apr 28, 2026
ac5fc4e
test(helm-chart): fix log groomer helm-tests by adding test for logGr…
n-badtke-cg Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions chart/templates/NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,39 +826,39 @@ DEPRECATION WARNING:

{{- end }}

{{- if ne (int .Values.workers.logGroomerSidecar.retentionDays) 15 }}
{{- if ne .Values.workers.logGroomerSidecar.retentionDays nil }}

DEPRECATION WARNING:
`workers.logGroomerSidecar.retentionDays` has been renamed to `workers.celery.logGroomerSidecar.retentionDays`.
Please change your values as support for the old name will be dropped in a future release.

{{- end }}

{{- if ne (int .Values.workers.logGroomerSidecar.retentionMinutes) 0 }}
{{- if ne .Values.workers.logGroomerSidecar.retentionMinutes nil }}

DEPRECATION WARNING:
`workers.logGroomerSidecar.retentionMinutes` has been renamed to `workers.celery.logGroomerSidecar.retentionMinutes`.
Please change your values as support for the old name will be dropped in a future release.

{{- end }}

{{- if ne (int .Values.workers.logGroomerSidecar.frequencyMinutes) 15 }}
{{- if ne .Values.workers.logGroomerSidecar.frequencyMinutes nil }}

DEPRECATION WARNING:
`workers.logGroomerSidecar.frequencyMinutes` has been renamed to `workers.celery.logGroomerSidecar.frequencyMinutes`.
Please change your values as support for the old name will be dropped in a future release.

{{- end }}

{{- if ne (int .Values.workers.logGroomerSidecar.maxSizeBytes) 0 }}
{{- if ne .Values.workers.logGroomerSidecar.maxSizeBytes nil }}

DEPRECATION WARNING:
`workers.logGroomerSidecar.maxSizeBytes` has been renamed to `workers.celery.logGroomerSidecar.maxSizeBytes`.
Please change your values as support for the old name will be dropped in a future release.

{{- end }}

{{- if ne (int .Values.workers.logGroomerSidecar.maxSizePercent) 0 }}
{{- if ne .Values.workers.logGroomerSidecar.maxSizePercent nil }}

DEPRECATION WARNING:
`workers.logGroomerSidecar.maxSizePercent` has been renamed to `workers.celery.logGroomerSidecar.maxSizePercent`.
Expand Down
4 changes: 2 additions & 2 deletions chart/templates/dag-processor/dag-processor-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,11 @@ spec:
args: {{- tpl (toYaml .Values.dagProcessor.logGroomerSidecar.args) . | nindent 12 }}
{{- end }}
env:
{{- if .Values.dagProcessor.logGroomerSidecar.retentionDays }}
{{- if ne .Values.dagProcessor.logGroomerSidecar.retentionDays nil }}
- name: AIRFLOW__LOG_RETENTION_DAYS
value: "{{ .Values.dagProcessor.logGroomerSidecar.retentionDays }}"
{{- end }}
{{- if .Values.dagProcessor.logGroomerSidecar.retentionMinutes }}
{{- if ne .Values.dagProcessor.logGroomerSidecar.retentionMinutes nil }}
- name: AIRFLOW__LOG_RETENTION_MINUTES
value: "{{ .Values.dagProcessor.logGroomerSidecar.retentionMinutes }}"
{{- end }}
Expand Down
4 changes: 2 additions & 2 deletions chart/templates/scheduler/scheduler-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,11 @@ spec:
env:
- name: AIRFLOW_HOME
value: "{{ .Values.airflowHome }}"
{{- if .Values.scheduler.logGroomerSidecar.retentionDays }}
{{- if ne .Values.scheduler.logGroomerSidecar.retentionDays nil }}
- name: AIRFLOW__LOG_RETENTION_DAYS
value: "{{ .Values.scheduler.logGroomerSidecar.retentionDays }}"
{{- end }}
{{- if .Values.scheduler.logGroomerSidecar.retentionMinutes }}
{{- if ne .Values.scheduler.logGroomerSidecar.retentionMinutes nil }}
- name: AIRFLOW__LOG_RETENTION_MINUTES
value: "{{ .Values.scheduler.logGroomerSidecar.retentionMinutes }}"
{{- end }}
Expand Down
4 changes: 2 additions & 2 deletions chart/templates/triggerer/triggerer-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,11 @@ spec:
args: {{- tpl (toYaml .Values.triggerer.logGroomerSidecar.args) . | nindent 12 }}
{{- end }}
env:
{{- if .Values.triggerer.logGroomerSidecar.retentionDays }}
{{- if ne .Values.triggerer.logGroomerSidecar.retentionDays nil }}
- name: AIRFLOW__LOG_RETENTION_DAYS
value: "{{ .Values.triggerer.logGroomerSidecar.retentionDays }}"
{{- end }}
{{- if .Values.triggerer.logGroomerSidecar.retentionMinutes }}
{{- if ne .Values.triggerer.logGroomerSidecar.retentionMinutes nil }}
- name: AIRFLOW__LOG_RETENTION_MINUTES
value: "{{ .Values.triggerer.logGroomerSidecar.retentionMinutes }}"
{{- end }}
Expand Down
4 changes: 2 additions & 2 deletions chart/templates/workers/worker-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,11 @@ spec:
args: {{ tpl (toYaml .Values.workers.logGroomerSidecar.args) . | nindent 12 }}
{{- end }}
env:
{{- if .Values.workers.logGroomerSidecar.retentionDays }}
{{- if ne .Values.workers.logGroomerSidecar.retentionDays nil }}
- name: AIRFLOW__LOG_RETENTION_DAYS
value: "{{ .Values.workers.logGroomerSidecar.retentionDays }}"
{{- end }}
{{- if .Values.workers.logGroomerSidecar.retentionMinutes }}
{{- if ne .Values.workers.logGroomerSidecar.retentionMinutes nil }}
- name: AIRFLOW__LOG_RETENTION_MINUTES
value: "{{ .Values.workers.logGroomerSidecar.retentionMinutes }}"
{{- end }}
Expand Down
18 changes: 12 additions & 6 deletions chart/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2440,12 +2440,14 @@
"retentionDays": {
"description": "Number of days to retain the logs when running the Airflow log groomer sidecar (deprecated, use ``workers.celery.logGroomerSidecar.retentionDays`` instead). Total retention time is ``retentionDays`` + ``retentionMinutes``.",
"type": "integer",
"default": 15
"default": 15,
"minimum": 0
},
"retentionMinutes": {
"description": "Number of minutes to retain the logs when running the Airflow log groomer sidecar (deprecated, use ``workers.celery.logGroomerSidecar.retentionMinutes`` instead). Total retention time is ``retentionDays`` + ``retentionMinutes``.",
"type": "integer",
"default": 0
"default": 0,
"minimum": 0
},
"frequencyMinutes": {
"description": "Number of minutes between attempts to groom the Airflow logs in log groomer sidecar (deprecated, use ``workers.celery.logGroomerSidecar.frequencyMinutes`` instead).",
Expand Down Expand Up @@ -3664,15 +3666,17 @@
"integer",
"null"
],
"default": null
"default": null,
"minimum": 0
},
"retentionMinutes": {
"description": "Number of minutes to retain the logs when running the Airflow log groomer sidecar. Total retention time is ``retentionDays`` + ``retentionMinutes``.",
"type": [
"integer",
"null"
],
"default": null
"default": null,
"minimum": 0
},
"frequencyMinutes": {
"description": "Number of minutes between attempts to groom the Airflow logs in log groomer sidecar.",
Expand Down Expand Up @@ -14400,12 +14404,14 @@
"retentionDays": {
"description": "Number of days to retain the logs when running the Airflow log groomer sidecar. Total retention time is ``retentionDays`` + ``retentionMinutes``.",
"type": "integer",
"default": 15
"default": 15,
"minimum": 0
},
"retentionMinutes": {
"description": "Number of minutes to retain the logs when running the Airflow log groomer sidecar. Total retention time is ``retentionDays`` + ``retentionMinutes``.",
"type": "integer",
"default": 0
"default": 0,
"minimum": 0
},
"frequencyMinutes": {
"description": "Number of minutes between attempts to groom the Airflow logs in log groomer sidecar.",
Expand Down
42 changes: 39 additions & 3 deletions helm-tests/tests/chart_utils/log_groomer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def test_log_groomer_command_and_args_overrides_are_templated(self):
assert jmespath.search("spec.template.spec.containers[1].command", docs[0]) == ["release-name"]
assert jmespath.search("spec.template.spec.containers[1].args", docs[0]) == ["Helm"]

@pytest.mark.parametrize(("retention_days", "retention_result"), [(None, None), (30, "30")])
@pytest.mark.parametrize(("retention_days", "retention_result"), [(None, None), (30, "30"), (0, "0")])
def test_log_groomer_retention_days_overrides(self, retention_days, retention_result):
if self.obj_name == "dag-processor":
values = {
Expand Down Expand Up @@ -218,7 +218,43 @@ def test_log_groomer_retention_days_overrides(self, retention_days, retention_re
== "15"
)
else:
assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 2
assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 3

@pytest.mark.parametrize(("retention_minutes", "retention_result"), [(None, None), (0, "0"), (60, "60")])
def test_log_groomer_retention_minutes_overrides(self, retention_minutes, retention_result):
if self.obj_name == "dag-processor":
values = {
"dagProcessor": {
"enabled": True,
"logGroomerSidecar": {"retentionMinutes": retention_minutes},
}
}
elif self.obj_name == "workers-celery":
values = {"workers": {"celery": {"logGroomerSidecar": {"retentionMinutes": retention_minutes}}}}
else:
values = {f"{self.folder}": {"logGroomerSidecar": {"retentionMinutes": retention_minutes}}}

docs = render_chart(values=values, show_only=self.get_show_only())

if retention_result:
assert (
jmespath.search(
"spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_RETENTION_MINUTES'].value | [0]",
docs[0],
)
== retention_result
)
elif self.obj_name == "workers-celery" and retention_result is None:
# Testing backward compatibility of move from workers to workers.celery
assert (
jmespath.search(
"spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_RETENTION_MINUTES'].value | [0]",
docs[0],
)
== "0"
)
else:
assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 3

@pytest.mark.parametrize(("frequency_minutes", "frequency_result"), [(None, None), (20, "20")])
def test_log_groomer_frequency_minutes_overrides(self, frequency_minutes, frequency_result):
Expand Down Expand Up @@ -254,7 +290,7 @@ def test_log_groomer_frequency_minutes_overrides(self, frequency_minutes, freque
== "15"
)
else:
assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 2
assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 3

@pytest.mark.parametrize(
("max_size_bytes", "max_size_result"), [(None, None), (1234567890, "1234567890")]
Expand Down
Loading