You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by js...@apache.org on 2026/04/28 20:55:37 UTC

(airflow) branch main updated: Allow worker log groomer sidecar without persistence (#65852)

This is an automated email from the ASF dual-hosted git repository.

jscheffl pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new d9c58044962 Allow worker log groomer sidecar without persistence (#65852)
d9c58044962 is described below

commit d9c580449625f1e09bcc2dec7f83acfb7b287a7d
Author: Eric Xiao <er...@wealthsimple.com>
AuthorDate: Tue Apr 28 16:55:24 2026 -0400

    Allow worker log groomer sidecar without persistence (#65852)
    
    Previously, the worker log groomer sidecar only rendered when
    `workers.persistence.enabled` was true (i.e., StatefulSet mode).
    This prevented using the log groomer on Deployments with emptyDir
    volumes for logs.
    
    This change removes the persistence requirement from the log groomer
    condition. Now the log groomer renders whenever `logGroomerSidecar.enabled`
    is true, regardless of the persistence setting.
    
    This is useful for deployments that:
    - Use emptyDir for local logs with remote logging (S3, GCS, etc.)
    - Want to clean up local log files to prevent ephemeral storage exhaustion
    - Prefer Deployments over StatefulSets for workers
    
    Users who don't want the log groomer can disable it by setting
    `workers.logGroomerSidecar.enabled: false`.
    
    Co-authored-by: Claude Opus 4.5 <no...@anthropic.com>
---
 chart/templates/workers/worker-deployment.yaml     |  2 +-
 .../tests/helm_tests/airflow_core/test_worker.py   | 54 ++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml
index 70d0506d504..281fe5a7380 100644
--- a/chart/templates/workers/worker-deployment.yaml
+++ b/chart/templates/workers/worker-deployment.yaml
@@ -338,7 +338,7 @@ spec:
         {{- if and .Values.dags.gitSync.enabled (not .Values.dags.persistence.enabled) }}
           {{- include "git_sync_container" . | nindent 8 }}
         {{- end }}
-        {{- if and $persistence .Values.workers.logGroomerSidecar.enabled }}
+        {{- if .Values.workers.logGroomerSidecar.enabled }}
         - name: worker-log-groomer
           image: {{ template "airflow_image" . }}
           imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
diff --git a/helm-tests/tests/helm_tests/airflow_core/test_worker.py b/helm-tests/tests/helm_tests/airflow_core/test_worker.py
index e0d498a6ad6..e9997e9be89 100644
--- a/helm-tests/tests/helm_tests/airflow_core/test_worker.py
+++ b/helm-tests/tests/helm_tests/airflow_core/test_worker.py
@@ -2332,6 +2332,60 @@ class TestWorkerCeleryLogGroomer(LogGroomerTestBase):
     obj_name = "workers-celery"
     folder = "workers"
 
+    @pytest.mark.parametrize(
+        ("persistence_enabled", "log_groomer_enabled", "expect_log_groomer"),
+        [
+            # persistence default (true) + logGroomer default (true) -> renders
+            (None, None, True),
+            # persistence default (true) + logGroomer false -> no render
+            (None, False, False),
+            # persistence true + logGroomer default (true) -> renders
+            (True, None, True),
+            # persistence true + logGroomer true -> renders
+            (True, True, True),
+            # persistence true + logGroomer false -> no render
+            (True, False, False),
+            # persistence false + logGroomer default (true) -> renders
+            (False, None, True),
+            # persistence false + logGroomer true -> renders
+            (False, True, True),
+            # persistence false + logGroomer false -> no render
+            (False, False, False),
+        ],
+    )
+    def test_log_groomer_with_persistence_combinations(
+        self, persistence_enabled, log_groomer_enabled, expect_log_groomer
+    ):
+        """Test log groomer sidecar rendering with various persistence and enabled combinations."""
+        celery_values = {}
+        if persistence_enabled is not None:
+            celery_values["persistence"] = {"enabled": persistence_enabled}
+        if log_groomer_enabled is not None:
+            celery_values["logGroomerSidecar"] = {"enabled": log_groomer_enabled}
+
+        docs = render_chart(
+            values={
+                "executor": "CeleryExecutor",
+                "workers": {"celery": celery_values} if celery_values else {},
+            },
+            show_only=["templates/workers/worker-deployment.yaml"],
+        )
+
+        # Verify resource kind based on persistence
+        expected_kind = "StatefulSet" if persistence_enabled is not False else "Deployment"
+        assert jmespath.search("kind", docs[0]) == expected_kind
+
+        # Verify log groomer presence
+        containers = jmespath.search("spec.template.spec.containers", docs[0])
+        container_names = [c["name"] for c in containers]
+
+        if expect_log_groomer:
+            assert "worker-log-groomer" in container_names
+            assert len(containers) == 2
+        else:
+            assert "worker-log-groomer" not in container_names
+            assert len(containers) == 1
+
 
 class TestWorkerKedaAutoScaler:
     """Tests worker keda auto scaler."""