diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index 340c9aaea3e13..92f30253b236c 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -35,6 +35,7 @@ {{- if or (contains "CeleryExecutor" .Values.executor) (contains "CeleryKubernetesExecutor" .Values.executor) }} --- {{- $persistence := or .Values.workers.persistence.enabled }} +{{- $logGroomerRequirePersistence := .Values.workers.logGroomerSidecar.requirePersistence }} {{- $keda := .Values.workers.keda.enabled }} {{- $hpa := and .Values.workers.hpa.enabled (not .Values.workers.keda.enabled) }} {{- $nodeSelector := or .Values.workers.nodeSelector .Values.nodeSelector }} @@ -348,7 +349,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 and .Values.workers.logGroomerSidecar.enabled (or $persistence (not $logGroomerRequirePersistence)) }} - name: worker-log-groomer image: {{ template "airflow_image" . }} imagePullPolicy: {{ .Values.images.airflow.pullPolicy }} diff --git a/chart/values.schema.json b/chart/values.schema.json index 6ee2b5217231a..c85923b2120b2 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2445,6 +2445,11 @@ "type": "boolean", "default": true }, + "requirePersistence": { + "description": "Whether persistence is required for log groomer sidecar. When false, the log groomer can run on Deployments (without persistence) using an ``emptyDir`` volume for logs. Note: This parameter is transitional for chart 1.2x. In chart 2.x, log groomer will work without persistence by default and this parameter will be removed (deprecated, use ``workers.celery.logGroomerSidecar.requirePersistence`` instead).", + "type": "boolean", + "default": true + }, "command": { "description": "Command to use when running the Airflow log groomer sidecar (templated) (deprecated, use ``workers.celery.logGroomerSidecar.command`` instead).", "type": [ @@ -3669,6 +3674,14 @@ ], "default": null }, + "requirePersistence": { + "description": "Whether persistence is required for log groomer sidecar. When false, the log groomer can run on Deployments (without persistence) using an ``emptyDir`` volume for logs. Note: This parameter is transitional for chart 1.2x. In chart 2.x, log groomer will work without persistence by default and this parameter will be removed.", + "type": [ + "boolean", + "null" + ], + "default": null + }, "command": { "description": "Command to use when running the Airflow log groomer sidecar (templated).", "type": [ diff --git a/chart/values.yaml b/chart/values.yaml index aad52cb3d6260..14125f3368e05 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1148,6 +1148,13 @@ workers: # (deprecated, use `workers.celery.logGroomerSidecar.enabled` instead) enabled: true + # Whether persistence is required for log groomer sidecar. When false, the log groomer + # can run on Deployments (without persistence) using an `emptyDir` volume for logs. + # Note: This parameter is transitional for chart 1.2x. In chart 2.x, log groomer will + # work without persistence by default and this parameter will be removed. + # (deprecated, use `workers.celery.logGroomerSidecar.requirePersistence` instead) + requirePersistence: true + # Command to use when running the Airflow Celery worker log groomer sidecar (templated) # (deprecated, use `workers.celery.logGroomerSidecar.command` instead) command: ~ @@ -1559,6 +1566,12 @@ workers: # Whether to deploy the Airflow Celery worker log groomer sidecar enabled: ~ + # Whether persistence is required for log groomer sidecar. When false, the log groomer + # can run on Deployments (without persistence) using an `emptyDir` volume for logs. + # Note: This parameter is transitional for chart 1.2x. In chart 2.x, log groomer will + # work without persistence by default and this parameter will be removed. + requirePersistence: ~ + # Command to use when running the Airflow Celery worker log groomer sidecar (templated) command: ~ 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 3e31d2d454efe..5c4f07fddf000 100644 --- a/helm-tests/tests/helm_tests/airflow_core/test_worker.py +++ b/helm-tests/tests/helm_tests/airflow_core/test_worker.py @@ -2352,6 +2352,78 @@ class TestWorkerCeleryLogGroomer(LogGroomerTestBase): obj_name = "workers-celery" folder = "workers" + @pytest.mark.parametrize( + ("persistence_enabled", "log_groomer_enabled", "require_persistence", "expect_log_groomer"), + [ + # With requirePersistence=true (default behavior, backwards compatible) + # persistence default (true) + logGroomer default (true) + requirePersistence default (true) -> renders + (None, None, None, True), + # persistence default (true) + logGroomer false + requirePersistence default (true) -> no render + (None, False, None, False), + # persistence true + logGroomer default (true) + requirePersistence default (true) -> renders + (True, None, None, True), + # persistence true + logGroomer true + requirePersistence default (true) -> renders + (True, True, None, True), + # persistence true + logGroomer false + requirePersistence default (true) -> no render + (True, False, None, False), + # persistence false + logGroomer default (true) + requirePersistence default (true) -> NO render + (False, None, None, False), + # persistence false + logGroomer true + requirePersistence default (true) -> NO render + (False, True, None, False), + # persistence false + logGroomer true + requirePersistence true -> NO render + (False, True, True, False), + # persistence false + logGroomer false + requirePersistence default (true) -> no render + (False, False, None, False), + # With requirePersistence=false (new behavior) + # persistence false + logGroomer true + requirePersistence false -> renders + (False, True, False, True), + # persistence false + logGroomer default (true) + requirePersistence false -> renders + (False, None, False, True), + # persistence true + logGroomer true + requirePersistence false -> renders + (True, True, False, True), + ], + ) + def test_log_groomer_with_persistence_and_require_persistence_combinations( + self, persistence_enabled, log_groomer_enabled, require_persistence, expect_log_groomer + ): + """Test log groomer sidecar rendering with various persistence, enabled, and requirePersistence combinations.""" + celery_values = {} + if persistence_enabled is not None: + celery_values["persistence"] = {"enabled": persistence_enabled} + + log_groomer_values = {} + if log_groomer_enabled is not None: + log_groomer_values["enabled"] = log_groomer_enabled + if require_persistence is not None: + log_groomer_values["requirePersistence"] = require_persistence + if log_groomer_values: + celery_values["logGroomerSidecar"] = log_groomer_values + + docs = render_chart( + values={ + "executor": "CeleryExecutor", + "workers": {"celery": celery_values} if celery_values else {}, + }, + show_only=["templates/workers/worker-deployment.yaml"], + ) + + # 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, ( + f"Expected log groomer with persistence={persistence_enabled}, " + f"enabled={log_groomer_enabled}, requirePersistence={require_persistence}" + ) + assert len(containers) == 2 + else: + assert "worker-log-groomer" not in container_names, ( + f"Did not expect log groomer with persistence={persistence_enabled}, " + f"enabled={log_groomer_enabled}, requirePersistence={require_persistence}" + ) + assert len(containers) == 1 + class TestWorkerKedaAutoScaler: """Tests worker keda auto scaler."""