Skip to content

Fix KubernetesPodOperator succeeding after SIGTERM/on_kill interrupt - #71533

Open
sankalpsthakur wants to merge 2 commits into
apache:mainfrom
sankalpsthakur:fix/71202-kpo-sigterm-success
Open

Fix KubernetesPodOperator succeeding after SIGTERM/on_kill interrupt#71533
sankalpsthakur wants to merge 2 commits into
apache:mainfrom
sankalpsthakur:fix/71202-kpo-sigterm-success

Conversation

@sankalpsthakur

@sankalpsthakur sankalpsthakur commented Aug 13, 2026

Copy link
Copy Markdown

stripped False
len 4046
fier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->

When KubernetesExecutor task pods and KubernetesPodOperator child pods are interrupted together (spot drain / dual SIGTERM), the task instance was finalized as SUCCESS even though the child never finished.

Root cause: task_runner SIGTERM handling calls on_kill(), which sets _killed and deletes the child. cleanup() then returned immediately so execute() completed normally. The supervisor records SUCCESS for a 0-exit with no terminal-state message. Interrupting only the child still failed (404 / PodFailed) because on_kill() never ran.

This does not revert #69034 (warm-shutdown supervisor). SIGTERM on the supervisor is still not treated as task failure. The change is KPO-only: after on_kill(), cleanup still skips a second delete (avoids 404 retries) but raises AirflowException so the TI is failed / up_for_retry.

Summary

  • KubernetesPodOperator.cleanup() raises if on_kill() already ran, instead of returning as success.
  • Unit tests cover the _killed cleanup path and the execute_sync race (SIGTERM/on_kill during wait, then execute continues).

Validation

  • Published apache-airflow-providers-cncf-kubernetes==10.21.0: cleanup() after _killed=True returned None (the SUCCESS bug).
  • Same call against this change: AirflowException: Pod sleep-worker was interrupted before it completed.
  • Tests added:
    • test_cleanup_raises_when_on_kill_already_ran
    • test_execute_sync_on_kill_during_wait_is_not_success
  • Full test_pod.py file is db_test (breeze). Local proof used Airflow 3.1.3 + overlaid operator module, no live cluster.

Fixes #71202

AI/LLM disclosure

  • AI coding tools (including Grok and/or Codex agent-assisted editing) were used to help draft or modify code and this PR description.
  • I reviewed the complete change, understand the reasoning, and ran the reported local tests before submitting.
  • This submission is original work of authorship under the project CLA / contributor terms; AI output was not pasted unreviewed.

Was generative AI tooling used to co-author this PR?
  • Yes (Grok / Cursor agent)

Generated-by: Grok (Cursor agent) following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@boring-cyborg

boring-cyborg Bot commented Aug 13, 2026

Copy link
Copy Markdown

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

cleanup() returned early after on_kill(), so execute() looked like a
clean success when SIGTERM hit the task process (spot drain / dual
interrupt) even though the child pod never finished.

Fixes apache#71202

Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>
check-no-new-airflow-exceptions rejects a fourth raise AirflowException
in pod.py. ruff-format also wants the raise on a single line.

Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>
@sankalpsthakur
sankalpsthakur force-pushed the fix/71202-kpo-sigterm-success branch from ad89bdc to 4f8b7a5 Compare August 17, 2026 08:03
@sankalpsthakur

Copy link
Copy Markdown
Author

Rebased onto main and used PodInterrupted (plus ruff-format) so static checks should pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues

Projects

None yet

1 participant