Skip to content

Conversation

@AutomationDev85
Copy link
Contributor

Overview

We observed Kubernetes API errors in KubernetesPodOperator when a pod is evicted. In this state the xcom sidecar container has already terminated, so exec/read attempts fail. The previous exception did not clearly indicate that the sidecar was no longer running, making it hard to understand why no XCom output could be read. The new exception message explicitly states that the sidecar is not running, clarifying why XCom retrieval failed.

Change Summary

  • KubernetesPodOperator now checks that the xcom sidecar container is running before attempting to read the XCom output.

@boring-cyborg boring-cyborg bot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels Jan 9, 2026
@SameerMesiah97
Copy link
Contributor

SameerMesiah97 commented Jan 11, 2026

Looks good to me overall. This is a tightly scoped, minimal fix with a new test (test_extract_xcom_sidecar_terminated) that exercises the new guard when container_is_running = False.

The existing tests already cover the container_is_running = True path indirectly, which seems acceptable here given the simplicity of the guard. I also appreciated that container_is_running is explicitly patched to True in the existing def extract_xcom tests to avoid unintended short-circuiting.

The only non-blocking suggestion I have is to mention these test additions/adjustments in the PR description, as that context helps reduce review friction. And, for future reference, I would advise you to squash commit when you rebase with one descriptive commit if the change is relatively minor. No need to do this now as there might be requests for changes.

@jscheffl
Copy link
Contributor

The only non-blocking suggestion I have is to mention these test additions/adjustments in the PR description, as that context helps reduce review friction. And, for future reference, I would advise you to squash commit when you rebase with one descriptive commit if the change is relatively minor. No need to do this now as there might be requests for changes.

No, in my view squashing is not needed. During merge all commits are squashed so if more commits are coming it is fine to collect and rebase. In my even makes it simpler in a re-review what has been changed after last review cycle. If always all commits are squashed I always need to make a full review...

@jscheffl jscheffl merged commit b39a319 into apache:main Jan 12, 2026
106 checks passed
iharsh02 pushed a commit to iharsh02/airflow that referenced this pull request Jan 13, 2026
…d xcom (apache#60319)

* check sidecar running before trying to read xcom

* Rename test

---------

Co-authored-by: AutomationDev85 <AutomationDev85>
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

Development

Successfully merging this pull request may close these issues.

4 participants