You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dirrao (via GitHub)" <gi...@apache.org> on 2024/08/01 13:17:31 UTC
[PR] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
dirrao opened a new pull request, #41186:
URL: https://github.com/apache/airflow/pull/41186
In certain scenarios (k8s executor) we see pods are stuck with the provider failed reason. We need to mark those tasks as failed.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on PR #41186:
URL: https://github.com/apache/airflow/pull/41186#issuecomment-2266653139
> tests are failing
Now test cases are passing.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1712565491
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,16 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
As per our observation this is permanent and never succeeded.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on PR #41186:
URL: https://github.com/apache/airflow/pull/41186#issuecomment-2266549204
> tests are failing
The test related backward compatibility. Not sure is this needed or not. Need someone to confirm the same.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham merged PR #41186:
URL: https://github.com/apache/airflow/pull/41186
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1705642762
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,11 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
+ self.log.error("Event: %s ProviderFailed, annotations: %s", pod_name, annotations_string)
Review Comment:
I think we should be a little more verbose in this error message. Just `ProviderFailed` isn't intuitive enough imo.
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,11 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
Can you add a comment as to how this can happen maybe? It seems to be specific to Azure. Doesn't have to be a big comment, but something.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1712565491
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,16 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
The pod provider failure is permanent. It will never succeeds. This is what we have observed.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1712565491
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,16 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
The pod provider failure is permanent. It will never succeeds.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on PR #41186:
URL: https://github.com/apache/airflow/pull/41186#issuecomment-2270992198
@jedcunningham / @hussein-awala
Can you review it whenever you are free?
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #41186:
URL: https://github.com/apache/airflow/pull/41186#issuecomment-2265754222
tests are failing
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1721286663
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,16 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
@jedcunningham ^^
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1706966058
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,11 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
Updated the comment below line
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1706967078
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,11 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
+ self.log.error("Event: %s ProviderFailed, annotations: %s", pod_name, annotations_string)
Review Comment:
Updated the comment.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1728421220
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,16 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
Thanks.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1712552339
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,16 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
The reason I ask is we tolerate a lot of possibly temporary non-runnable pods already, e.g. if there are no nodes with space for the pod, and simply rely on the queued timeout to handle them. If it can be temporary, giving the pod a shot of getting going is probably the better behavior here.
--
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] K8s Executor: failing the task in case the watcher receives an event with the reason ProviderFailed [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #41186:
URL: https://github.com/apache/airflow/pull/41186#discussion_r1712549622
##########
airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py:
##########
@@ -220,6 +220,16 @@ def process_status(
# However, need to free the executor slot from the current executor.
self.log.info("Event: pod %s adopted, annotations: %s", pod_name, annotations_string)
self.watcher_queue.put((pod_name, namespace, ADOPTED, annotations, resource_version))
+ elif hasattr(pod.status, "reason") and pod.status.reason == "ProviderFailed":
Review Comment:
I take it the status is still pending here, so is the "ProviderFailed" reason permanent and will never actually run, or can it resolve itself?
--
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