You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/09/30 07:47:06 UTC
[GitHub] [airflow] windmark opened a new issue, #26796: Incorrect await_container_completion in KubernetesPodOperator
windmark opened a new issue, #26796:
URL: https://github.com/apache/airflow/issues/26796
### Apache Airflow version
2.4.0
### What happened
The [await_container_completion](
https://github.com/apache/airflow/blob/2.4.0/airflow/providers/cncf/kubernetes/utils/pod_manager.py#L259) function has a negated condition
```
while not self.container_is_running(pod=pod, container_name=container_name):
```
that causes our Airflow tasks running <1 s to never be completed, causing an infinite loop.
I see this was addressed and released in https://github.com/apache/airflow/pull/23883, but later reverted in https://github.com/apache/airflow/pull/24474. How come it was reverted? The thread on that revert PR with comments from @jedcunningham and @potiuk didn't really address why the fix was reverted.
### What you think should happen instead
Pods finishing within 1s should be properly handled.
### How to reproduce
_No response_
### Operating System
Linux
### Versions of Apache Airflow Providers
_No response_
### Deployment
Other 3rd-party Helm chart
### Deployment details
_No response_
### Anything else
_No response_
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] windmark commented on issue #26796: Incorrect await_container_completion in KubernetesPodOperator
Posted by GitBox <gi...@apache.org>.
windmark commented on issue #26796:
URL: https://github.com/apache/airflow/issues/26796#issuecomment-1263354809
> It caused failure of tests and we were unable to point the source so we had to revert #24479
>
> https://apache-airflow.slack.com/archives/CCPRP7943/p1655306336391429 https://apache-airflow.slack.com/archives/CCPRP7943/p1655300699968739
>
> Quoting Ash from slack:
>
> > That'll learn us for merging a bug fix without any new tests
>
> Feel free to raise PR with tests :)
I see, thanks for the links. Though it's quite a severe bug if the condition incorrectly is negated when waiting for completed pods. Let me read up on the threads.
--
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
[GitHub] [airflow] eladkal closed issue #26796: Incorrect await_container_completion in KubernetesPodOperator
Posted by GitBox <gi...@apache.org>.
eladkal closed issue #26796: Incorrect await_container_completion in KubernetesPodOperator
URL: https://github.com/apache/airflow/issues/26796
--
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
[GitHub] [airflow] eladkal commented on issue #26796: Incorrect await_container_completion in KubernetesPodOperator
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #26796:
URL: https://github.com/apache/airflow/issues/26796#issuecomment-1263320215
It caused failure of tests and we were unable to point the source so we had to revert
https://github.com/apache/airflow/pull/24479
https://apache-airflow.slack.com/archives/CCPRP7943/p1655306336391429
https://apache-airflow.slack.com/archives/CCPRP7943/p1655300699968739
Quoting Ash from slack:
> That'll learn us for merging a bug fix without any new tests
Feel free to raise PR with tests :)
--
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