lum1nxus opened a new issue, #66858:
URL: https://github.com/apache/airflow/issues/66858
# 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 (`<release>-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 `<component>.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
(`<release>-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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
