yssxr commented on issue #39693: URL: https://github.com/apache/airflow/issues/39693#issuecomment-5256859799
Picking this up. Still happens on main (12e3f08d54). `await_pod_completion` only breaks out early on base container completion if istio or the xcom sidecar is involved: https://github.com/apache/airflow/blob/12e3f08d54785b22b5a99983b8097a04e5fb3c0e/providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/utils/pod_manager.py#L826 A plain sidecar sets neither, so it just sits there waiting for the pod phase to go terminal, which won't happen while the sidecar's still alive. Worth mentioning before I start though: you can't just fix that line on its own. `cleanup()` branches on the same condition to decide whether the task failed: https://github.com/apache/airflow/blob/12e3f08d54785b22b5a99983b8097a04e5fb3c0e/providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py#L1333 ```python if istio_enabled or self.do_xcom_push: failed = not container_is_succeeded(remote_pod, self.base_container_name) else: failed = pod_phase != PodPhase.SUCCEEDED ``` If `await_pod_completion` starts returning early, the pod it hands back still has phase Running, cleanup drops into the else, and the task gets marked failed. So you'd be trading the hang for a bogus failure. Both have to move together. Which sort of suggests `istio_enabled` and `do_xcom_push` are the same thing wearing two hats: there's a sidecar, so trust the base container rather than the pod phase. It just never got generalised. So that's what I'm planning to do at both sites, with istio and the xcom sidecar falling out as ordinary cases of it. Two things that come with that, flagging now rather than in review: - it changes behaviour for existing multi-container pods that currently wait on pod phase - `test_await_pod_completion_waits_for_pod_phase_without_sidecars` from #64962 asserts the current behaviour, so it needs rewriting If that compat risk is a nope, the alternative is an opt-in arg, but that seems like a rough default to leave sitting there. Shout if you'd rather I went that way, otherwise I'll open a PR with tests covering both sites. -- 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]
