Add `--min-completed-minutes` to `cleanup-pods` to prevent KPO race condition by noamst-monday · Pull Request #70595 · apache/airflow

noamst-monday · 2026-07-28T11:48:25Z

Problem

KubernetesPodOperator in synchronous (deferrable=False) mode calls PodManager.await_pod_completion to confirm the pod reached a terminal phase. This method polls read_pod every 2 seconds (pod_manager.py:839, hardcoded time.sleep(2)).

When a pod transitions to Succeeded, there is up to a 2-second window before PodManager.await_pod_completion observes the terminal phase. The airflow kubernetes cleanup-pods command currently deletes Succeeded/Failed/Evicted pods immediately — there is no minimum-age guard for terminal states.

If the cleanup job fires during that window, PodManager.await_pod_completion's next read_pod call returns 404 and the task is marked FAILED, even though the pod completed successfully (exit code 0).

A --min-pending-minutes guard already exists for Pending pods (default 30 m, minimum 5 m). No equivalent exists for terminal states.

Real-world reproduction

Confirmed via Kubernetes API server audit logs on a production EKS cluster running apache-airflow-providers-cncf-kubernetes==10.19.0 on Airflow 3.2.2. The cleanup CronJob was set to run every 5 minutes.

Timeline of a representative failure:

13:15:12Z  PodManager polls pod       → phase Running
13:15:14Z  Container exits            → exit code 0 (pod phase → Succeeded)
13:15:17Z  cleanup-pods deletes pod   → 3 s after completion
13:15:18Z  PodManager polls pod       → 404 Not Found → task FAILED

Airflow task log excerpt:

[INFO]  Pod run-o1sxc2on has phase Running
[ERROR] Task failed with exception
ApiException: (404) Reason: Not Found
HTTP response body: {"message":"pods \"run-o1sxc2on\" not found","reason":"NotFound","code":404}

Audit log at deletion time confirmed phase: Succeeded, containerStatuses[0].state.terminated.exitCode: 0, finishedAt: 13:15:14Z.

Solution

Add --min-completed-minutes (default 1) to the cleanup-pods command. When set, Succeeded/Failed/Evicted pods are skipped unless their completion time exceeds the threshold. Set to 0 to restore the previous immediate-deletion behaviour.

Completion time is derived from the latest finished_at across both container_statuses and init_container_statuses. Falls back to the latest conditions[*].last_transition_time (updated at the terminal transition), then to creation_timestamp as a last resort.

Changes

File	Change
`cli/definition.py`	Add `ARG_MIN_COMPLETED_MINUTES` (default `1`, `allow_zero=True`)
`cli/kubernetes_command.py`	Add `_get_pod_completion_time()`; gate terminal-pod deletion by age
`tests/.../test_kubernetes_command.py`	`TestGetPodCompletionTime` with real k8s model objects; 4 integration-style tests for the new flag
`docs/changelog.rst`	Not modified — maintained by release manager

CLI reference docs (cli-ref.rst) use .. argparse:: and pick up the new flag automatically.

Testing

pytest providers/cncf/kubernetes/tests/unit/cncf/kubernetes/cli/test_kubernetes_command.py -q
# 19 passed

Usage

# Helm chart — keep default of 1 minute:
cleanup:
  args: ["bash", "-c", "exec airflow kubernetes cleanup-pods --namespace {{ .Release.Namespace }}"]

# Or set explicitly:
airflow kubernetes cleanup-pods --namespace airflow --min-completed-minutes 5

Was generative AI tooling used to co-author this PR?

Yes (Claude Sonnet 4.6, Opus 5)

Generated-by: Claude Sonnet 4.6 following the guidelines

Read the Pull Request Guidelines for more information.

…tion The ``airflow kubernetes cleanup-pods`` command currently deletes Succeeded/Failed/Evicted pods immediately, with no minimum-age guard for terminal states. ``KubernetesPodOperator`` in synchronous mode polls pod status every ~2 seconds via ``await_pod_completion``. If the cleanup job fires in the window between the pod reaching ``Succeeded`` and KPO's next poll, KPO receives a 404 and fails the task -- even though the pod completed successfully (exit code 0). A ``--min-pending-minutes`` guard already exists for Pending pods (default 30 m, floor 5 m). No equivalent exists for terminal states. This commit adds ``--min-completed-minutes`` (default ``0``, which preserves the existing behaviour). When set to any positive value, Succeeded/Failed/Evicted pods are skipped unless their completion time is older than the threshold. Completion time is derived from the latest ``containerStatuses[*].state.terminated.finishedAt`` timestamp (falls back to ``metadata.creationTimestamp`` for pods that were evicted before any container started). Root-cause investigation ------------------------ This was confirmed via Kubernetes API server audit logs on a production EKS cluster. Timeline for an affected KPO task: 13:15:12Z KPO polls pod → phase Running 13:15:14Z Container exits with code 0 (pod transitions to Succeeded) 13:15:17Z cleanup-pods CronJob deletes the pod (3 s after completion) 13:15:18Z KPO polls pod → 404 Not Found → task marked FAILED The pod had succeeded; the task failure was a false positive caused entirely by the race. Multiple production DAGs exhibited the same pattern with the cleanup CronJob set to run every 5 minutes. Reducing the CronJob frequency is a partial mitigation (lowers the probability) but does not eliminate the race. Setting ``--min-completed-minutes=5`` gives KPO a 5-minute window to observe the terminal phase -- 150x wider than the 2 s poll interval -- closing the race completely in practice. Changes ------- * ``definition.py`` – add ``ARG_MIN_COMPLETED_MINUTES``; wire into cleanup-pods args tuple * ``kubernetes_command.py`` – add ``_get_pod_completion_time()`` helper; gate terminal-pod deletion by age when ``min_completed_minutes > 0`` * ``test_kubernetes_command.py`` – 4 new unit tests covering: - Succeeded pod too young → not deleted - Succeeded pod old enough → deleted - Default (0) preserves immediate-deletion behaviour - Failed/Never pod too young → not deleted * ``changelog.rst`` – entry under 10.21.0 CLI docs (``cli-ref.rst``) are auto-generated via ``.. argparse::`` and will pick up the new flag automatically.

boring-cyborg · 2026-07-28T11:48:32Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack

- Set default to 1 minute (no reason to keep the race by default) - Shorten --min-completed-minutes help text - Fix _get_pod_completion_time: scan init_container_statuses too; fall back to max(conditions.last_transition_time) instead of creation_timestamp (which predates actual completion) - Add TestGetPodCompletionTime with real k8s model objects covering main-only, init-only, both, conditions fallback, and creation_timestamp last-resort cases - Fix test_cleanup_min_completed_zero_deletes_immediately to pass --min-completed-minutes=0 explicitly now that default is 1

The guard must not fall through to creation_timestamp for pods whose containers never reached a terminated state — that path reports an inflated age and deletes immediately, the unsafe direction. Freezing time keeps every case at a realistic completion offset instead of a timestamp in the future.

_get_pod_completion_time guards three levels of the container state, but the fallback cases only reached the first two, so a terminated status carrying no finishedAt went unexercised. Modelling the non-terminated case as a waiting container also matches what k8s reports for a pod evicted before its containers started.

boring-cyborg · 2026-07-30T16:57:13Z

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

noamst-monday requested review from hussein-awala, jedcunningham and jscheffl as code owners July 28, 2026 11:48

boring-cyborg Bot added area:providers kind:documentation provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels Jul 28, 2026

noamst-monday added 2 commits July 28, 2026 14:48

Update changelog with PR number apache#70595

9a1de45

Fix ruff D213 docstring format in _get_pod_completion_time

2df806e

noamst-monday marked this pull request as draft July 28, 2026 11:52

noamst-monday added 2 commits July 28, 2026 14:56

Use exact poll interval (2 s) in --min-completed-minutes help text

c32ebfb

Improve --min-completed-minutes help text clarity

90e28ce

noamst-monday marked this pull request as ready for review July 28, 2026 13:24

Merge branch 'main' into add-min-completed-minutes-cleanup-pods

dafde57

eladkal reviewed Jul 28, 2026

View reviewed changes

Comment thread providers/cncf/kubernetes/docs/changelog.rst

noamst-monday commented Jul 28, 2026

View reviewed changes

Comment thread providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/cli/definition.py Outdated

noamst-monday commented Jul 28, 2026

View reviewed changes

Comment thread providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/cli/kubernetes_command.py Outdated

Revert changelog.rst — auto-generated by release manager

098f5fe

jedcunningham requested changes Jul 28, 2026

View reviewed changes

noamst-monday marked this pull request as draft July 29, 2026 10:09

noamst-monday added 2 commits July 29, 2026 13:16

noamst-monday marked this pull request as ready for review July 29, 2026 10:42

Use a tuple for parametrize names in cleanup-pods tests

3fa1c84

jedcunningham reviewed Jul 29, 2026

View reviewed changes

Comment thread providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/cli/kubernetes_command.py

Name the pending-pod age condition in cleanup-pods

891c95f

jedcunningham approved these changes Jul 30, 2026

View reviewed changes

Comment thread providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/cli/kubernetes_command.py

eladkal approved these changes Jul 30, 2026

View reviewed changes

eladkal merged commit eca4816 into apache:main Jul 30, 2026
108 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `--min-completed-minutes` to `cleanup-pods` to prevent KPO race condition - #70595

Add `--min-completed-minutes` to `cleanup-pods` to prevent KPO race condition#70595
eladkal merged 12 commits into
apache:mainfrom
noamst-monday:add-min-completed-minutes-cleanup-pods

noamst-monday commented Jul 28, 2026 •

edited

Loading

Uh oh!

boring-cyborg Bot commented Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

boring-cyborg Bot commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

noamst-monday commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Real-world reproduction

Related

Solution

Changes

Testing

Usage

Was generative AI tooling used to co-author this PR?

Uh oh!

boring-cyborg Bot commented Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

boring-cyborg Bot commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

noamst-monday commented Jul 28, 2026 •

edited

Loading