From 7504c78e3acc46b72698e3b3f72c67b66bf2211c Mon Sep 17 00:00:00 2001 From: Stefan <1188614+stefankeidel@users.noreply.github.com> Date: Wed, 29 Jan 2025 14:27:04 +0100 Subject: [PATCH 1/4] New feature: Add env variable controlling the log grooming frequency --- .../dag-processor-deployment.yaml | 4 +++ .../scheduler/scheduler-deployment.yaml | 4 +++ .../triggerer/triggerer-deployment.yaml | 4 +++ .../templates/workers/worker-deployment.yaml | 4 +++ chart/values.schema.json | 5 ++++ chart/values.yaml | 8 ++++++ scripts/docker/clean-logs.sh | 3 ++- tests/charts/log_groomer.py | 25 +++++++++++++++++++ 8 files changed, 56 insertions(+), 1 deletion(-) mode change 100644 => 100755 scripts/docker/clean-logs.sh diff --git a/chart/templates/dag-processor/dag-processor-deployment.yaml b/chart/templates/dag-processor/dag-processor-deployment.yaml index f4675456b2f6e..1b9d3280bd41c 100644 --- a/chart/templates/dag-processor/dag-processor-deployment.yaml +++ b/chart/templates/dag-processor/dag-processor-deployment.yaml @@ -214,6 +214,10 @@ spec: {{- if .Values.dagProcessor.logGroomerSidecar.retentionDays }} - name: AIRFLOW__LOG_RETENTION_DAYS value: "{{ .Values.dagProcessor.logGroomerSidecar.retentionDays }}" + {{- end }} + {{- if .Values.workers.logGroomerSidecar.frequencyMinutes }} + - name: AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES + value: "{{ .Values.workers.logGroomerSidecar.frequencyMinutes }}" {{- end }} - name: AIRFLOW_HOME value: "{{ .Values.airflowHome }}" diff --git a/chart/templates/scheduler/scheduler-deployment.yaml b/chart/templates/scheduler/scheduler-deployment.yaml index b2d4f7b13eeb7..47df9797ad396 100644 --- a/chart/templates/scheduler/scheduler-deployment.yaml +++ b/chart/templates/scheduler/scheduler-deployment.yaml @@ -272,6 +272,10 @@ spec: {{- if .Values.scheduler.logGroomerSidecar.retentionDays }} - name: AIRFLOW__LOG_RETENTION_DAYS value: "{{ .Values.scheduler.logGroomerSidecar.retentionDays }}" + {{- end }} + {{- if .Values.workers.logGroomerSidecar.frequencyMinutes }} + - name: AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES + value: "{{ .Values.workers.logGroomerSidecar.frequencyMinutes }}" {{- end }} - name: AIRFLOW_HOME value: "{{ .Values.airflowHome }}" diff --git a/chart/templates/triggerer/triggerer-deployment.yaml b/chart/templates/triggerer/triggerer-deployment.yaml index 79be90408f38b..3f3fe58977c0e 100644 --- a/chart/templates/triggerer/triggerer-deployment.yaml +++ b/chart/templates/triggerer/triggerer-deployment.yaml @@ -244,6 +244,10 @@ spec: {{- if .Values.triggerer.logGroomerSidecar.retentionDays }} - name: AIRFLOW__LOG_RETENTION_DAYS value: "{{ .Values.triggerer.logGroomerSidecar.retentionDays }}" + {{- end }} + {{- if .Values.workers.logGroomerSidecar.frequencyMinutes }} + - name: AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES + value: "{{ .Values.workers.logGroomerSidecar.frequencyMinutes }}" {{- end }} - name: AIRFLOW_HOME value: "{{ .Values.airflowHome }}" diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index 26adde1cc0beb..4f120ac563968 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -326,6 +326,10 @@ spec: {{- if .Values.workers.logGroomerSidecar.retentionDays }} - name: AIRFLOW__LOG_RETENTION_DAYS value: "{{ .Values.workers.logGroomerSidecar.retentionDays }}" + {{- end }} + {{- if .Values.workers.logGroomerSidecar.frequencyMinutes }} + - name: AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES + value: "{{ .Values.workers.logGroomerSidecar.frequencyMinutes }}" {{- end }} - name: AIRFLOW_HOME value: "{{ .Values.airflowHome }}" diff --git a/chart/values.schema.json b/chart/values.schema.json index 8a8aa276f7e79..908db7e3efccb 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -12197,6 +12197,11 @@ "type": "integer", "default": 15 }, + "frequencyMinutes": { + "description": "Number of minutes between attempts to groom the Airflow logs in log groomer sidecar.", + "type": "integer", + "default": 15 + }, "env": { "description": "Add additional env vars to log groomer sidecar container (templated).", "items": { diff --git a/chart/values.yaml b/chart/values.yaml index 5c24186fd8eb3..c694dc448bf9a 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -821,6 +821,8 @@ workers: args: ["bash", "/clean-logs"] # Number of days to retain logs retentionDays: 15 + # frequency to attempt to groom logs, in minutes + frequencyMinutes: 15 resources: {} # limits: # cpu: 100m @@ -1023,6 +1025,8 @@ scheduler: args: ["bash", "/clean-logs"] # Number of days to retain logs retentionDays: 15 + # frequency to attempt to groom logs, in minutes + frequencyMinutes: 15 resources: {} # limits: # cpu: 100m @@ -1736,6 +1740,8 @@ triggerer: args: ["bash", "/clean-logs"] # Number of days to retain logs retentionDays: 15 + # frequency to attempt to groom logs, in minutes + frequencyMinutes: 15 resources: {} # limits: # cpu: 100m @@ -1922,6 +1928,8 @@ dagProcessor: args: ["bash", "/clean-logs"] # Number of days to retain logs retentionDays: 15 + # frequency to attempt to groom logs, in minutes + frequencyMinutes: 15 resources: {} # limits: # cpu: 100m diff --git a/scripts/docker/clean-logs.sh b/scripts/docker/clean-logs.sh old mode 100644 new mode 100755 index 866d5baa7b8d2..063b0a985b685 --- a/scripts/docker/clean-logs.sh +++ b/scripts/docker/clean-logs.sh @@ -21,10 +21,11 @@ set -euo pipefail readonly DIRECTORY="${AIRFLOW_HOME:-/usr/local/airflow}" readonly RETENTION="${AIRFLOW__LOG_RETENTION_DAYS:-15}" +readonly FREQUENCY="${AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES:-15}" trap "exit" INT TERM -readonly EVERY=$((15*60)) +readonly EVERY=$((FREQUENCY*60)) echo "Cleaning logs every $EVERY seconds" diff --git a/tests/charts/log_groomer.py b/tests/charts/log_groomer.py index 8ddb9d83e62ec..db60223be1083 100644 --- a/tests/charts/log_groomer.py +++ b/tests/charts/log_groomer.py @@ -191,6 +191,31 @@ def test_log_groomer_retention_days_overrides(self, retention_days, retention_re else: assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 1 + @pytest.mark.parametrize("frequency_minutes, frequency_result", [(None, None), (20, "20")]) + def test_log_groomer_frequency_minutes_overrides(self, frequency_minutes, frequency_result): + if self.obj_name == "dag-processor": + values = { + "dagProcessor": {"enabled": True, "logGroomerSidecar": {"frequencyMinutes": frequency_minutes}} + } + else: + values = {f"{self.folder}": {"logGroomerSidecar": {"frequencyMinutes": frequency_minutes}}} + + docs = render_chart( + values=values, + show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"], + ) + + if frequency_result: + assert ( + jmespath.search("spec.template.spec.containers[1].env[0].name", docs[0]) + == "AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES" + ) + assert frequency_result == jmespath.search( + "spec.template.spec.containers[1].env[0].value", docs[0] + ) + else: + assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 1 + def test_log_groomer_resources(self): if self.obj_name == "dag-processor": values = { From d5064fffbe1196d98467710c704c0530e0bf015f Mon Sep 17 00:00:00 2001 From: Stefan Keidel <1188614+stefankeidel@users.noreply.github.com> Date: Thu, 30 Jan 2025 08:06:29 +0100 Subject: [PATCH 2/4] pre-commit --- Dockerfile | 3 ++- tests/charts/log_groomer.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 6af0174f7310e..4a27e105989b0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1271,10 +1271,11 @@ set -euo pipefail readonly DIRECTORY="${AIRFLOW_HOME:-/usr/local/airflow}" readonly RETENTION="${AIRFLOW__LOG_RETENTION_DAYS:-15}" +readonly FREQUENCY="${AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES:-15}" trap "exit" INT TERM -readonly EVERY=$((15*60)) +readonly EVERY=$((FREQUENCY*60)) echo "Cleaning logs every $EVERY seconds" diff --git a/tests/charts/log_groomer.py b/tests/charts/log_groomer.py index db60223be1083..75bf005e67307 100644 --- a/tests/charts/log_groomer.py +++ b/tests/charts/log_groomer.py @@ -195,7 +195,10 @@ def test_log_groomer_retention_days_overrides(self, retention_days, retention_re def test_log_groomer_frequency_minutes_overrides(self, frequency_minutes, frequency_result): if self.obj_name == "dag-processor": values = { - "dagProcessor": {"enabled": True, "logGroomerSidecar": {"frequencyMinutes": frequency_minutes}} + "dagProcessor": { + "enabled": True, + "logGroomerSidecar": {"frequencyMinutes": frequency_minutes}, + } } else: values = {f"{self.folder}": {"logGroomerSidecar": {"frequencyMinutes": frequency_minutes}}} From 29e6c49d55f32c779c75fb3bcfc8ecef329f2464 Mon Sep 17 00:00:00 2001 From: Stefan Keidel <1188614+stefankeidel@users.noreply.github.com> Date: Thu, 30 Jan 2025 08:36:06 +0100 Subject: [PATCH 3/4] fix: copy+paste errors, adjust test --- .../dag-processor-deployment.yaml | 4 +-- .../scheduler/scheduler-deployment.yaml | 4 +-- .../triggerer/triggerer-deployment.yaml | 4 +-- tests/charts/log_groomer.py | 28 ++++++++----------- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/chart/templates/dag-processor/dag-processor-deployment.yaml b/chart/templates/dag-processor/dag-processor-deployment.yaml index 1b9d3280bd41c..c3654914837e1 100644 --- a/chart/templates/dag-processor/dag-processor-deployment.yaml +++ b/chart/templates/dag-processor/dag-processor-deployment.yaml @@ -215,9 +215,9 @@ spec: - name: AIRFLOW__LOG_RETENTION_DAYS value: "{{ .Values.dagProcessor.logGroomerSidecar.retentionDays }}" {{- end }} - {{- if .Values.workers.logGroomerSidecar.frequencyMinutes }} + {{- if .Values.dagProcessor.logGroomerSidecar.frequencyMinutes }} - name: AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES - value: "{{ .Values.workers.logGroomerSidecar.frequencyMinutes }}" + value: "{{ .Values.dagProcessor.logGroomerSidecar.frequencyMinutes }}" {{- end }} - name: AIRFLOW_HOME value: "{{ .Values.airflowHome }}" diff --git a/chart/templates/scheduler/scheduler-deployment.yaml b/chart/templates/scheduler/scheduler-deployment.yaml index 47df9797ad396..8c4d28f4acdd1 100644 --- a/chart/templates/scheduler/scheduler-deployment.yaml +++ b/chart/templates/scheduler/scheduler-deployment.yaml @@ -273,9 +273,9 @@ spec: - name: AIRFLOW__LOG_RETENTION_DAYS value: "{{ .Values.scheduler.logGroomerSidecar.retentionDays }}" {{- end }} - {{- if .Values.workers.logGroomerSidecar.frequencyMinutes }} + {{- if .Values.scheduler.logGroomerSidecar.frequencyMinutes }} - name: AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES - value: "{{ .Values.workers.logGroomerSidecar.frequencyMinutes }}" + value: "{{ .Values.scheduler.logGroomerSidecar.frequencyMinutes }}" {{- end }} - name: AIRFLOW_HOME value: "{{ .Values.airflowHome }}" diff --git a/chart/templates/triggerer/triggerer-deployment.yaml b/chart/templates/triggerer/triggerer-deployment.yaml index 3f3fe58977c0e..f183c3bbced4e 100644 --- a/chart/templates/triggerer/triggerer-deployment.yaml +++ b/chart/templates/triggerer/triggerer-deployment.yaml @@ -245,9 +245,9 @@ spec: - name: AIRFLOW__LOG_RETENTION_DAYS value: "{{ .Values.triggerer.logGroomerSidecar.retentionDays }}" {{- end }} - {{- if .Values.workers.logGroomerSidecar.frequencyMinutes }} + {{- if .Values.triggerer.logGroomerSidecar.frequencyMinutes }} - name: AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES - value: "{{ .Values.workers.logGroomerSidecar.frequencyMinutes }}" + value: "{{ .Values.triggerer.logGroomerSidecar.frequencyMinutes }}" {{- end }} - name: AIRFLOW_HOME value: "{{ .Values.airflowHome }}" diff --git a/tests/charts/log_groomer.py b/tests/charts/log_groomer.py index 75bf005e67307..0dcc0b960a146 100644 --- a/tests/charts/log_groomer.py +++ b/tests/charts/log_groomer.py @@ -181,15 +181,11 @@ def test_log_groomer_retention_days_overrides(self, retention_days, retention_re ) if retention_result: - assert ( - jmespath.search("spec.template.spec.containers[1].env[0].name", docs[0]) - == "AIRFLOW__LOG_RETENTION_DAYS" - ) - assert retention_result == jmespath.search( - "spec.template.spec.containers[1].env[0].value", docs[0] - ) + assert jmespath.search( + "spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_RETENTION_DAYS'].value | [0]", docs[0] + ) == retention_result else: - assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 1 + assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 2 @pytest.mark.parametrize("frequency_minutes, frequency_result", [(None, None), (20, "20")]) def test_log_groomer_frequency_minutes_overrides(self, frequency_minutes, frequency_result): @@ -209,15 +205,11 @@ def test_log_groomer_frequency_minutes_overrides(self, frequency_minutes, freque ) if frequency_result: - assert ( - jmespath.search("spec.template.spec.containers[1].env[0].name", docs[0]) - == "AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES" - ) - assert frequency_result == jmespath.search( - "spec.template.spec.containers[1].env[0].value", docs[0] - ) + assert jmespath.search( + "spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES'].value | [0]", docs[0] + ) == frequency_result else: - assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 1 + assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 2 def test_log_groomer_resources(self): if self.obj_name == "dag-processor": @@ -270,4 +262,6 @@ def test_log_groomer_has_airflow_home(self): values=values, show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"] ) - assert jmespath.search("spec.template.spec.containers[1].env[1].name", docs[0]) == "AIRFLOW_HOME" + assert jmespath.search( + "spec.template.spec.containers[1].env[?name=='AIRFLOW_HOME'].name | [0]", docs[0] + ) == "AIRFLOW_HOME" From 92458a8eb99ab26f949d239f339465d17014340b Mon Sep 17 00:00:00 2001 From: Stefan Keidel <1188614+stefankeidel@users.noreply.github.com> Date: Mon, 3 Feb 2025 08:41:54 +0100 Subject: [PATCH 4/4] format --- tests/charts/log_groomer.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/charts/log_groomer.py b/tests/charts/log_groomer.py index 0dcc0b960a146..4e07a598301a6 100644 --- a/tests/charts/log_groomer.py +++ b/tests/charts/log_groomer.py @@ -181,9 +181,13 @@ def test_log_groomer_retention_days_overrides(self, retention_days, retention_re ) if retention_result: - assert jmespath.search( - "spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_RETENTION_DAYS'].value | [0]", docs[0] - ) == retention_result + assert ( + jmespath.search( + "spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_RETENTION_DAYS'].value | [0]", + docs[0], + ) + == retention_result + ) else: assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 2 @@ -205,9 +209,13 @@ def test_log_groomer_frequency_minutes_overrides(self, frequency_minutes, freque ) if frequency_result: - assert jmespath.search( - "spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES'].value | [0]", docs[0] - ) == frequency_result + assert ( + jmespath.search( + "spec.template.spec.containers[1].env[?name=='AIRFLOW__LOG_CLEANUP_FREQUENCY_MINUTES'].value | [0]", + docs[0], + ) + == frequency_result + ) else: assert len(jmespath.search("spec.template.spec.containers[1].env", docs[0])) == 2 @@ -262,6 +270,7 @@ def test_log_groomer_has_airflow_home(self): values=values, show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"] ) - assert jmespath.search( - "spec.template.spec.containers[1].env[?name=='AIRFLOW_HOME'].name | [0]", docs[0] - ) == "AIRFLOW_HOME" + assert ( + jmespath.search("spec.template.spec.containers[1].env[?name=='AIRFLOW_HOME'].name | [0]", docs[0]) + == "AIRFLOW_HOME" + )