apache / apache/airflow

Helm Chart: dag_bundle_config_list changes roll every Airflow component, even though only the dag-processor consumes the value

Open
#66,858 2 comments 0 reactions 0 assignees View on GitHub
area:helm-chart kind:feature
Dominant language
Python
Stars
46.9k
Forks
17.8k
Avg merge
2d 9h
Merged PRs (30d)
472

Description

# Apache Airflow version
3.1.x (chart `1.18.x`, current `main`)

# What happened?

The Airflow Helm chart stores `config.dag_processor.dag_bundle_config_list` inside the global `airflow.cfg` ConfigMap (`-config`). Every long-running Airflow workload (scheduler, api-server, worker, triggerer, dag-processor, flower) carries a `checksum/airflow-config` pod annotation computed as `sha256sum` of that ConfigMap.

Any time the bundle list changes — most commonly a `tracking_ref` bump emitted by a CI pipeline that promotes a new DAG version into the `GitDagBundle` configuration — the checksum flips and **every** component pod is rolled. In our environment a routine `helm upgrade` from a CI job tears down the scheduler, the api-server, all Celery workers, and the triggerer purely to refresh a value that only the dag-processor actually re-reads.

This is a production-relevant restart blast radius: in-flight Celery tasks are interrupted, the api-server returns 503 for the duration of the rollout, and any DAG run boundary that lands in the rollout window is delayed.

# What you think should happen instead?

Mutations of `dag_bundle_config_list` should only roll the dag-processor. All other Airflow components either do not consume the value at runtime (api-server, flower) or only consume it lazily on bundle access with a per-task version pin (scheduler, worker, triggerer), so a `tracking_ref` change is functionally a no-op for them until they next refresh.

We see strong precedent for "do not annotate a workload with a checksum it does not depend on" in **#63204 "Remove JWT secrets from triggerer, worker and dag-processor"**, which removed `checksum/jwt-secret` annotations from workloads that do not need the JWT secret. The exact same principle applies here.

# Why is only the dag-processor a true consumer?

We traced the runtime behavior in the current `main` branch:

1. **dag-processor** — the only process that actively re-reads `dag_bundle_config_list` from `airflow.cfg`, instantiates each bundle, and calls `bundle.initialize()` on a refresh interval. The initialize path runs `self.repo.git.checkout(self.tracking_ref)` and is the only place where `tracking_ref` is materially used to determine what the bundle points at:

`providers/git/src/airflow/providers/git/bundles/git.py:204`
```python
self.repo.git.checkout(self.tracking_ref)
```

2. **scheduler / api-server** — never call `bundle.initialize()`:
- `airflow-core/src/airflow/jobs/` (scheduler job runners) contains **zero** references to `DagBundlesManager` or `bundle.initialize`.
- `airflow-core/src/airflow/api_fastapi/` (api-server) likewise contains **zero** references to either symbol.
- The single scheduler-side touch point is `dag_processing/dagbag.py:135`, gated by `if conf.getboolean("core", "multi_team")` and used only to read `_bundle_config[bundle_name].team_name` for executor selection — a config lookup, not an initialize.
- All `bundle.initialize()` call sites in `airflow-core/` live in `dag_processing/manager.py` (dag-processor process) and `cli/commands/dag_command.py` / `utils/cli.py` (explicit operator CLI actions).

3. **worker (Celery / KubernetesExecutor) / triggerer** — load bundle code via `BundleVersionLock` against the **persisted `bundle_version`** for the specific `TaskInstance` (`task-sdk/src/airflow/sdk/execution_time/task_runner.py:2002-2004`). The runtime hand-off is:

- `task-sdk/.../task_runner.py:788` calls `DagBundlesManager().get_bundle(...)` to obtain a bundle instance,
- then `task_runner.py:2002-2004` enters `BundleVersionLock(bundle_version=ti.bundle_instance.version, ...)` — i.e. the worker locks to the commit SHA recorded for that TI in the metadata DB, not to whatever `tracking_ref` currently points at.

Inside `GitDagBundle._initialize` the `tracking_ref` checkout is overridden when `self.version` is set:

`providers/git/src/airflow/providers/git/bundles/git.py:204-210`
```python
self.repo.git.checkout(self.tracking_ref)
self._log.debug("bundle initialize", version=self.version)
if self.version:
if not self._has_version(self.repo, self.version):
self.repo.remotes.origin.fetch()
self.repo.head.set_reference(str(self.repo.commit(self.version)))
self.repo.head.reset(index=True, working_tree=True)
```

So a worker that resumes after a `tracking_ref` bump will:
- still find its pinned commit via local repo cache, or
- `origin.fetch()` and pick it up from the remote.

The only failure mode is when the **previous tag** is force-moved or deleted upstream — but that is an external bundle-management problem, not something a pod restart can fix.

The net effect: rolling scheduler/api-server/worker/triggerer on a `dag_bundle_config_list` change buys nothing operationally and causes user-visible disruption.

# How to reproduce

Minimal reproducer using the official chart:

```bash
# 1. Install with a Git bundle pointed at tag v1
helm install airflow apache-airflow/airflow \
--set dagProcessor.enabled=true \
--set "dagProcessor.dagBundleConfigList[0].name=demo" \
--set "dagProcessor.dagBundleConfigList[0].classpath=airflow.providers.git.bundles.git.GitDagBundle" \
--set "dagProcessor.dagBundleConfigList[0].kwargs.repo_url=https://github.com/apache/airflow.git" \
--set "dagProcessor.dagBundleConfigList[0].kwargs.tracking_ref=v1"

# 2. Upgrade with only the tracking_ref changed
helm upgrade airflow apache-airflow/airflow \
--reuse-values \
--set "dagProcessor.dagBundleConfigList[0].kwargs.tracking_ref=v2"

# 3. Observe rollout
kubectl get pods -w
```

Expected (after fix): only the dag-processor pod is recreated.
Actual (today): scheduler, api-server, worker, triggerer, flower, dag-processor are all recreated.

The same symptom is reproducible with any other field of `dagBundleConfigList` (adding a new bundle, changing `subdir`, rotating `git_conn_id`) — anything that mutates the JSON string causes a global restart.

# Solution shapes considered

We have a working local fix and are happy to upstream it. Before opening a PR we would like maintainer guidance on the preferred shape. Three options, in order of increasing surface area:

## A. Documentation-only (smallest)
Document the existing escape hatch in `chart/docs/manage-dag-files.rst`: a Flux `spec.postRenderers` / Argo Kustomize patch that pins `checksum/airflow-config` to a static value on the non-dag-processor workloads. Zero chart code change. Reaches Flux / Argo users but not plain `helm install` users. Operator owns the trade-off explicitly.

## B. Per-component opt-out flag (small)
Add `.restartOnConfigChange: true` (default `true`) for scheduler / api-server / worker / triggerer / flower. When set to `false`, the chart simply omits the `checksum/airflow-config` annotation on that component. Follows the same shape as PR **#18776** (opt-out for hardcoded annotations). Diff: ~5 lines per deployment template + values.yaml/schema + tests.

Caveat: this is a blunt instrument — disabling the annotation also stops *all* `airflow.cfg` changes (e.g. `executor` swap, DB URL rotation, image upgrade alongside a config change) from automatically rolling that component. Operator footgun, but explicit and documented.

## C. Isolated bundle ConfigMap (cleanest, biggest)
Introduce an opt-in `dagProcessor.isolateBundleConfigRestart: false` flag. When `true`:
- `dag_bundle_config_list` is rendered into a **dedicated** ConfigMap (`-dag-bundle-config`) only, not into the global `airflow.cfg`.
- `AIRFLOW__DAG_PROCESSOR__DAG_BUNDLE_CONFIG_LIST` is injected as an env var (via `configMapKeyRef`) into every Airflow component pod, so Python config resolution behaves identically to today.
- Only the dag-processor deployment grows a `checksum/dag-bundle-config` annotation pointing at the new ConfigMap.
- `checksum/airflow-config` remains, and continues to roll every component when *real* `airflow.cfg` keys change (executor, DB, etc.).

This is the same blast-radius surgery as PR **#63204** applied to the bundle list: stop annotating workloads with a checksum they do not depend on, and route the now-isolated value through a narrower ConfigMap.

Local diff size, for reference:
- 4 modified templates (`configmap.yaml`, `_helpers.yaml`, `dag-processor-deployment.yaml`, `values.schema.json`)
- 1 modified default (`values.yaml`)
- 1 new template (`configmaps/dag-bundle-config-configmap.yaml`)
- 1 new test file (16 tests, all green locally)
- 1 newsfragment
- Docs update in `manage-dag-files.rst` describing the runbook for structural bundle changes (operator must `kubectl rollout restart` if they intend a global refresh)

Default is `false`, so this is fully backwards-compatible.

# Why we think C is the right long-term shape

- It mirrors **#63204** precisely: don't carry a checksum dependency on a value the workload does not consume.
- It composes cleanly with #60645 (which introduced the structured `dagBundleConfigList` value) — that PR improved the *authoring* side; this proposal improves the *delivery* side.
- It does not require chart 2.0 or a breaking change: behavior under default `isolateBundleConfigRestart=false` is bit-identical to today.
- The runbook cost is minimal and bounded: the only case where an operator needs a manual `kubectl rollout restart` is a *structural* bundle change (executor swap of `BundleVersionLock` semantics in a future version), which is rare and explicit.

We also considered:

- **Using `_cmd` suffix to read the bundle list from a file** — rejected, `_cmd` is gated to sensitive values only and Airflow does not generically support file-includes in `airflow.cfg`.
- **Computing two checksums (full airflow.cfg + a subset without the bundle list)** — possible but requires re-rendering `airflow.cfg` twice in the chart and produces a confusing pod annotation surface. ConfigMap split is simpler.
- **`tpl`-driven user override of `checksum/airflow-config` via `podAnnotations`** — already works today (YAML last-key-wins lets a user-supplied annotation win) but is an undocumented behavior, fragile across chart upgrades, and silently disables checksum for *all* airflow.cfg changes.

# Ask

Before we open a PR, we would appreciate a maintainer's steer on the preferred shape (A / B / C / something else). We have C implemented and tested locally and are ready to open the PR as-is, but we want to make sure it aligns with the direction of chart 2.0 restructuring discussed in **#61039**.

# Anything else?

This bites us on **every** CI-driven `tracking_ref` bump, which on a busy DAG repo happens 10+ times per day. We are happy to provide additional traces, manifest diffs, or to break C into smaller incremental PRs if that is easier to review.

# Are you willing to submit PR?

- [x] Yes I am willing to submit a PR!

# Code of Conduct

- [x] I agree to follow this project's Code of Conduct

Contributor guide

Open the contributing guide

Research direction

Start by reading the chart templates configmap.yaml, _helpers.yaml, dag-processor-deployment.yaml, values.yaml, and values.schema.json, along with the existing tests and chart/docs/manage-dag-files.rst. Confirm how checksum/airflow-config is rendered and review the proposed A, B, and C approaches with maintainers before choosing a scope. Done means the selected approach is implemented, documented, and covered by the chart tests without unintended rollouts.

Written by the indexing model from the issue text.

Assessment

Tech stack
helm, kubernetes, python
Domain
devops, infrastructure
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.