You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vincbeck (via GitHub)" <gi...@apache.org> on 2023/05/30 20:39:42 UTC
[GitHub] [airflow] vincbeck opened a new pull request, #31622: Ferruzzi/system tests/fargate logging
vincbeck opened a new pull request, #31622:
URL: https://github.com/apache/airflow/pull/31622
Both system tests `example_eks_with_fargate_in_one_step` and `example_eks_with_fargate_profile` are failing for the same reason. When the operator `EksPodOperator` is used with `get_logs=True`, the operator tries to get log once the POD started. In order to do so, Fargate logging needs to be enabled which can be done by following the instructions [here](https://docs.aws.amazon.com/eks/latest/userguide/fargate-logging.html).
On top of it, when doing so, 90% of the time it fails because of:
```
HTTP response body: b'{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Get \\"https://10.0.5.84:10250/containerLogs/default/run-pod-6rjxgqrs/base?follow=true\\u0026timestamps=true\\": remote error: tls: internal error","code":500}\n'
```
After investigation, it turns out there is delay between when the pod starts and when the [CSR](https://docs.aws.amazon.com/eks/latest/userguide/cert-signing.html) is available, signed and approved. If you try to get logs with the command `kubectl logs <pod-name> -n <namespace>` when the CSR is not available, signed and approved, you'll get the exact same error. If you wait until the CSR is there, you'll get the logs.
Therefore, in order to fix it, I decided to just retry on ApiException which the is the exception we get in such scenario.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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] Fix Fargate logging for AWS system tests [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #31622:
URL: https://github.com/apache/airflow/pull/31622#issuecomment-1793124398
@vincbeck can you share an example traceback that caused you to add tenacity to `consume_logs`?
I am just digging into KPO logging a bit again, and finding that the whole thing has become extremely messy and complicated, and I'm trying to chip away at some of the mess.
And as part of that, the behavior of consume_logs is such that it is already called in a loop if there's an error. So it's a bit odd to also wrap it with tenacity, a kind of mixing of two different retry strategies that makes it a bit confusing.
Now what can be done.... Well please observe that `consume_logs` calls `read_pod_logs`, which I believe is where the APIException that you catch is actually encountered, and which already retries using tenacity. So my thought is we can just update that function to handle your case, and thus move the retry logic closer to where it is needed, and reduce the tenacity wrappers from 2 to 1. But to do this I need more information about specifically where that exception is raised. My suspicion is that it would be raised at `logs = self._client.read_namespaced_pod_log` in `read_pod_logs`. But I recognize it's also possible that it is not raised until the logs consumer is iterated in at `for raw_line in logs:` in `consume_logs`. Thanks for the help.
--
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] vincbeck commented on pull request #31622: Ferruzzi/system tests/fargate logging
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #31622:
URL: https://github.com/apache/airflow/pull/31622#issuecomment-1569068197
@ferruzzi who made most of the work 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] Fix Fargate logging for AWS system tests [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #31622:
URL: https://github.com/apache/airflow/pull/31622#issuecomment-1798855892
Hey, thanks @vincbeck , kind of you to check that.
Ok, i'll make a PR to remove it for now. And then if it comes back, we can revisit.
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] Fix Fargate logging for AWS system tests [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #31622:
URL: https://github.com/apache/airflow/pull/31622#issuecomment-1795251553
Hey @dstandish . I removed the tenacity wrapper around `consume_logs` to get the stack trace and the system test ran successfully twice. So I guess, it should be safe to remove it?
--
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] ferruzzi commented on pull request #31622: Fix Fargate logging for AWS system tests
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #31622:
URL: https://github.com/apache/airflow/pull/31622#issuecomment-1570555493
You've reverted everything except adding the exception. Was that intentional?
--
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] potiuk merged pull request #31622: Fix Fargate logging for AWS system tests
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #31622:
URL: https://github.com/apache/airflow/pull/31622
--
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