diff --git a/chart/templates/NOTES.txt b/chart/templates/NOTES.txt index b6f9de44977d5..8349e13302cd2 100644 --- a/chart/templates/NOTES.txt +++ b/chart/templates/NOTES.txt @@ -826,7 +826,7 @@ 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`. @@ -834,7 +834,7 @@ DEPRECATION WARNING: {{- 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`. @@ -842,7 +842,7 @@ DEPRECATION WARNING: {{- 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`. @@ -850,7 +850,7 @@ DEPRECATION WARNING: {{- 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`. @@ -858,7 +858,7 @@ DEPRECATION WARNING: {{- 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`. diff --git a/chart/templates/dag-processor/dag-processor-deployment.yaml b/chart/templates/dag-processor/dag-processor-deployment.yaml index 5afad1e77d916..5cad98175c503 100644 --- a/chart/templates/dag-processor/dag-processor-deployment.yaml +++ b/chart/templates/dag-processor/dag-processor-deployment.yaml @@ -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 }} diff --git a/chart/templates/scheduler/scheduler-deployment.yaml b/chart/templates/scheduler/scheduler-deployment.yaml index 82af2b1201020..231295ad413ea 100644 --- a/chart/templates/scheduler/scheduler-deployment.yaml +++ b/chart/templates/scheduler/scheduler-deployment.yaml @@ -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 }} diff --git a/chart/templates/triggerer/triggerer-deployment.yaml b/chart/templates/triggerer/triggerer-deployment.yaml index 79309611afb2a..dd7c081a05808 100644 --- a/chart/templates/triggerer/triggerer-deployment.yaml +++ b/chart/templates/triggerer/triggerer-deployment.yaml @@ -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 }} diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index 70d0506d50474..d2e24c3a0f9fb 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -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 }} diff --git a/chart/values.schema.json b/chart/values.schema.json index b10fe6a5ae49e..41ab8e16750d3 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -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).", @@ -3664,7 +3666,8 @@ "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``.", @@ -3672,7 +3675,8 @@ "integer", "null" ], - "default": null + "default": null, + "minimum": 0 }, "frequencyMinutes": { "description": "Number of minutes between attempts to groom the Airflow logs in log groomer sidecar.", @@ -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.", diff --git a/helm-tests/tests/chart_utils/log_groomer.py b/helm-tests/tests/chart_utils/log_groomer.py index 36a2c1eb7ae2b..c5018fd482d28 100644 --- a/helm-tests/tests/chart_utils/log_groomer.py +++ b/helm-tests/tests/chart_utils/log_groomer.py @@ -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 = { @@ -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): @@ -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")]