You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "developer-rpai (via GitHub)" <gi...@apache.org> on 2026/09/26 06:02:07 UTC
[PR] Fail deferrable KPO instead of silent SUCCESS when a GC'd pod takes its XCom with it [airflow]
developer-rpai opened a new pull request, #73749:
URL: https://github.com/apache/airflow/pull/73749
## Problem
A deferrable `KubernetesPodOperator` with `do_xcom_push=True` can be marked **SUCCESS without pushing any `return_value` XCom** when the pod is garbage-collected between the trigger firing and the worker resuming the task. Downstream tasks then fail unrecoverably at templating time (`TypeError: the JSON object must be str, bytes or bytearray, not NoneType`); retrying the downstream task can never recover because the upstream XCom does not exist.
## Root cause
In `trigger_reentry()` (`providers/cncf/kubernetes/operators/pod.py`), the 404 handler added by #66716 returns silently whenever the trigger event status is `"success"` — regardless of `do_xcom_push`. The trigger observed the main container finish, but the XCom sidecar died with the GC'd pod, so the returned `None` becomes a successful task with no XCom.
## Fix
Keep the silent-success path only when `do_xcom_push` is off (there, only pod logs are lost). When XCom was expected, raise `PodNotFoundException` instead, so the task fails (and retries, when configured) and recreates the pod along with its XCom sidecar — mirroring the existing behavior for non-success events.
Note: a prior fix attempt (#73131) was closed unmerged during today's enforcement of the new 5-open-PR limit for non-committers, before any maintainer review. This PR re-implements the same approach (fail when `do_xcom_push` is set) with its own regression test.
## Tests
- Added `test_async_trigger_reentry_raises_pod_not_found_on_success_when_xcom_push`: GC'd pod + success event + `do_xcom_push=True` -> `PodNotFoundException`. Fails on `main` before this change.
- Updated `test_async_trigger_reentry_returns_when_pod_gcd_on_success` to pin `do_xcom_push=False`, making the preserved silent-success contract explicit.
- A source-level reproduction script confirmed the unguarded `if event["status"] == "success": return` pattern before the fix and the `do_xcom_push` guard after.
- Sandbox limits: the full provider unit-test suite could not be run in this environment (Airflow + provider test dependencies are not installable here); relying on CI for the suite. `py_compile` passes on both touched files.
## Impact
Correctness: prevents silent data loss (missing XCom) and a wrong task status (SUCCESS for a task whose result was destroyed). No behavior change when `do_xcom_push` is `False`.
Fixes: #73117
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Muse (Meta)
Generated-by: Muse (Meta) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fail deferrable KPO instead of silent SUCCESS when a GC'd pod takes its XCom with it [airflow]
Posted by "Miretpl (via GitHub) via commits" <co...@airflow.apache.org>.
Miretpl commented on code in PR #73749:
URL: https://github.com/apache/airflow/pull/73749#discussion_r4496828929
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/operators/test_pod.py:
##########
@@ -4114,6 +4114,33 @@ def test_async_trigger_reentry_returns_when_pod_gcd_on_success(self, mocked_hook
# await_pod_completion in _clean must not be called with self.pod=None
mock_manager.return_value.await_pod_completion.assert_not_called()
+ @patch(POD_MANAGER_CLASS)
+ @patch(HOOK_CLASS)
+ def test_async_trigger_reentry_raises_pod_not_found_on_success_when_xcom_push(
+ self, mocked_hook, mock_manager
+ ):
+ """Pod GC'd before reentry with do_xcom_push=True must not silently succeed (issue #73117).
+
+ The XCom sidecar died with the pod, so returning success would leave the task
+ SUCCESS with no return_value XCom, unrecoverably breaking downstream tasks.
+ Raising lets the task fail/retry so a fresh pod reproduces the result.
+ """
+ mocked_hook.return_value.get_pod.side_effect = ApiException(status=404, reason="Not Found")
+ k = KubernetesPodOperator(task_id="task", deferrable=True, do_xcom_push=True)
+ context = create_context(k)
+ context["ti"] = MagicMock()
Review Comment:
Could you specify `spec` for `MagicMock`? Any `MagicMock` without a `spec` or `autospec` silently accepts any attribute access
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fail deferrable KPO instead of silent SUCCESS when a GC'd pod takes its XCom with it [airflow]
Posted by "developer-rpai (via GitHub)" <gi...@apache.org>.
developer-rpai commented on PR #73749:
URL: https://github.com/apache/airflow/pull/73749#issuecomment-6060487301
Hi maintainers, just a gentle bump on this one. A deferrable KubernetesPodOperator can be marked SUCCESS without pushing any return_value XCom when the pod gets garbage-collected between the trigger firing and the worker resuming the task. Downstream tasks then fail unrecoverably at templating time since retrying can't recreate the missing XCom. This change makes the operator fail explicitly in that case instead of silently succeeding, so the true task state is visible. CI is green on my end; would appreciate a review when you get a chance. Happy to take a different approach for the GC'd pod case if you prefer.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org