xchwan commented on PR #75945: URL: https://github.com/apache/airflow/pull/75945#issuecomment-6731535653
@aaron-y-chen
Thanks for the review. I fixed the three points and checked each one on a
kind cluster, with the chart from
before the fix and the fixed chart, using the same values:
```yaml
dagProcessor:
enableDefault: true
podDisruptionBudget: {enabled: true}
sets:
- name: set-a
command: [airflow]
args: [dag-processor, --bundle-name, bundle_a]
bundleNames: [bundle_a]
- name: set-b
bundleNames: [bundle_b]
serviceAccount: {create: true, name: team-b}
```
**1. Default PDB also selected the sets' Pods**
- Before: the default PDB selected all 3 Pods (`expectedPods: 3`), and
evicting the set-a Pod failed with
`This pod has more than one PodDisruptionBudget, which the eviction
subresource does not support.`
- After: the default PDB excludes sets that have their own PDB (`NotIn` on
`dag-processor-set`). Each PDB selects
exactly one Pod (`expectedPods: 1` for all three), and evicting the
default, set-a and set-b Pods all succeeds.
- Test: `test_each_pod_is_selected_by_one_pdb` matches every Deployment's
Pod labels against every PDB selector.
**2. A set's `serviceAccount` override was never created**
- Before: set-b's Deployment referenced `team-b`, no such ServiceAccount
existed, and a new Pod could not be
created: `error looking up service account airflow/team-b: serviceaccount
"team-b" not found`.
- After: the ServiceAccount template renders one account per distinct name,
so `team-b` is created and set-b runs.
Sets that use the same name with the same settings share one object; the
same name with different `annotations`
or `automountServiceAccountToken` fails at render time with a "conflicting
configuration" error.
- Tests: `test_set_service_account_override_is_created`,
`test_shared_service_account_is_rendered_once`,
`test_conflicting_service_account_config_fails`.
**3. `bundleNames` assumed the last arg is a shell command**
- Before: with `command: [airflow]` the args became `["dag-processor",
"--bundle-name", "bundle_a --bundle-name 'bundle_a'"]`
and the Pod went into `CrashLoopBackOff`.
- After: the flags are part of the default `args` in `values.yaml`, like
`workers.celery.queue`, so custom
`command` and `args` are passed through untouched. The Pod runs and parses
only `bundle_a`. If `bundleNames` is
set and `args` has no `--bundle-name`, the render fails instead of parsing
every bundle:
`` `dagProcessor.args` does not select bundle "bundle_a" ``.
- Tests: `test_custom_command_and_args_are_passed_through`,
`test_bundle_names_not_passed_by_custom_args_fail`.
**Other cases I ran on the cluster with the fixed chart**
- No sets: the default PDB selector is unchanged (no `NotIn`), evicting the
Pod succeeds, and the processor parses
both bundles.
- Sets only (`enableDefault: false`): no default Deployment or PDB; each set
has its own PDB selecting one Pod, and
evicting each set's Pod succeeds.
- Only the default and set-a have a PDB (set-b has none): the default PDB
selects 2 Pods (default and set-b), set-a's
PDB selects 1, and evicting the set-a and set-b Pods succeeds.
- A set overriding its own PDB (`maxUnavailable: 0` on set-a): evicting the
set-a Pod is refused (429), while
evicting the set-b and default Pods succeeds.
- Two sets sharing one ServiceAccount name with the same settings: one
`shared-sa` is created and both Deployments
use it. A set with its own name and annotation gets that ServiceAccount
created with the annotation. A set with
`create: false` pointing at an existing account creates nothing extra.
- Custom `args` containing a template expression (`{{ .Release.Name }}`): it
is rendered into the args and the Pod
runs and parses only `bundle_a`.
- Rejected at upgrade time: a `bundleNames` entry that is not defined,
`dagBundleConfigList` inside a set,
conflicting ServiceAccount settings, and custom `args` without
`--bundle-name`.
- After restoring the original values, both Dags (one per bundle) ran to
`success`.
**Stability and tests**
- No restarts: the API server, scheduler, PostgreSQL and the three Dag
processor Pods were ready with 0 restarts
in 6 samples over 5 minutes.
- `test_dag_processor_sets.py`: 29 passed. Full chart suite: 2307 passed.
Both were run after the last change.
- `breeze build-docs helm-chart` succeeds, and the updated section of
`manage-dag-files` renders.
---
Generated-by: Claude Code (Sonnet 5.5)
--
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]
