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 2021/01/01 18:31:15 UTC

[GitHub] [airflow] potiuk opened a new pull request #13424: Increases timeouts of integration test setup

potiuk opened a new pull request #13424:
URL: https://github.com/apache/airflow/pull/13424


   We started to get recently much slower startup of the integration
   docker compose environment, likely because we are starting more
   of those services. Seems that the environment gets up and runnign
   when it gets enough time to warm up.
   
   This change attempts to give more time for the warm-up, also it
   puts RabbitMQ check at the very end as this seems to be the
   service that creates most problems - especially with sqlite
   database which does not add initialization time.
   
   <!--
   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 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/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753404925


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#discussion_r550801510



##########
File path: scripts/in_container/check_environment.sh
##########
@@ -21,6 +21,15 @@ EXIT_CODE=0
 
 DISABLED_INTEGRATIONS=""
 
+function run_nc() {
+    local host=${1}
+    local port=${2}
+    local ip
+    ip=$(python -c "import socket; print(socket.gethostbyname('${host}'))")

Review comment:
       Yep. Just pushed 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753372101


   Hey Everyone.
   
   I think this one will finally get rid of the pesky `DNS fwd/rev mismatch` error that came back recently (especially with rabbitmq).
   I managed to recreate it locally and I believe it should be fixed with this PR (disabling reverse IP lookup in netcat + resolving the host name in python).
   
   Quoting @ashb "It's DNS. It's always DNS".
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753384533


   > Does this problem also occur with Kerberos? It seems to me that the network is differently configured.
   
   I saw it only with mysql and rabbitmq but never with Kerberos. 
   I locally had rabbitmq /others in a different network and experienced the same behaviour (race condition leading to different reverse lookup). 
   
   I am a bit lost to what the question was leading to @mik-laj ? Are you point to some other problem you think might be the cause?
   Any other solution to the problem? Do you think it is wrong solution ? 
   
   IMHO it does not really matter which network it is. Netcat performing reverse lookup but it is not needed, really. And doing forward-lookup only seems to solve the problem for the CI setup. 
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk merged pull request #13424: Change timeouts and disables reverse IP lookup for integrations

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13424:
URL: https://github.com/apache/airflow/pull/13424


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753383482


   Does this problem also occur with Kerberos? It seems to me that the network is differently configured.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj edited a comment on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753386669


   > It looks like there is a race condition with docker compose
   that causes services that started fast enough (before DNS)
   to get a different reverse-DNS IP lookup (usually it is
   just <SERVICE> but sometimes it is
   <DOCKER_COMPOSE_APP>_<SERVICE>_1_<NETWORK>)
   
   ```
     DNS fwd/rev mismatch: rabbitmq != docker-compose_rabbitmq_1.docker-compose_default
     rabbitmq [172.25.0.8] 5672 (amqp) : Connection refused
      sent 0, rcvd 0
   ```
   
   I think we have two separate messages here. The first is a warning that always occurs if one host have multiple DNS names and run netcat in very verbose mode (``-vv``). The second is a real error message that contains information about the problemr that happened.
   
   Take a look at an example.
   ```bash
   $ nc -zvv kdc-server-example-com 88
   DNS fwd/rev mismatch: kdc-server-example-com != docker-compose_kdc-server-example-com_1.example.com
   kdc-server-example-com [10.5.0.2] 88 (kerberos) open
    sent 0, rcvd 0
   $  echo $?
   0
   $ nc -z kdc-server-example-com 88
   $ echo $?
   0
   ```
   This warning does not affect communication in most cases and is only relevant in very restrictive network configurations, e.g. in the case of Kerberos, which tries to validate the message source a little more.
   
   So if we want to improve CI stability, this change will not help us in anything.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753390787


   we can start using a different hostname for this check.
   ```
   $ nc -zvv kdc-server-example-com.example.com 88
   DNS fwd/rev mismatch: kdc-server-example-com.example.com != docker-compose_kdc-server-example-com_1.example.com
   kdc-server-example-com.example.com [10.5.0.2] 88 (kerberos) open
    sent 0, rcvd 0
   $ nc -zvv docker-compose_kdc-server-example-com_1.example.com 88
   docker-compose_kdc-server-example-com_1.example.com [10.5.0.2] 88 (kerberos) open
    sent 0, rcvd 0
   ```


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj edited a comment on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753392893


   I checked and Mongo actually starts 7 seconds after this check, so no matter what DNS entries we use, it won't help with this problem.
   <img width="986" alt="Screenshot 2021-01-01 at 23 12 22" src="https://user-images.githubusercontent.com/12058428/103447078-d5ba0a00-4c86-11eb-880f-854ef008a9ea.png">
   <img width="1000" alt="Screenshot 2021-01-01 at 23 09 59" src="https://user-images.githubusercontent.com/12058428/103447060-a0152100-4c86-11eb-9cd3-564e72577ebd.png">
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753386669


   > It looks like there is a race condition with docker compose
   that causes services that started fast enough (before DNS)
   to get a different reverse-DNS IP lookup (usually it is
   just <SERVICE> but sometimes it is
   <DOCKER_COMPOSE_APP>_<SERVICE>_1_<NETWORK>)
   
   ```
     DNS fwd/rev mismatch: rabbitmq != docker-compose_rabbitmq_1.docker-compose_default
     rabbitmq [172.25.0.8] 5672 (amqp) : Connection refused
      sent 0, rcvd 0
   ```
   
   I think we have two separate messages here. The first is a warning that always occurs if one host have multiple DNS names and run netcat in very verbose mode (``-vv``). The second is a real error message that contains information about the error that happened.
   
   Take a look at an example.
   ```bash
   $ nc -zvv kdc-server-example-com 88
   DNS fwd/rev mismatch: kdc-server-example-com != docker-compose_kdc-server-example-com_1.example.com
   kdc-server-example-com [10.5.0.2] 88 (kerberos) open
    sent 0, rcvd 0
   $  echo $?
   0
   $ nc -z kdc-server-example-com 88
   $ echo $?
   0
   ```
   This warning does not affect communication in most cases and is only relevant in very restrictive network configurations, e.g. in the case of Kerberos, which tries to validate the message source a little more.
   
   So if we want to improve CI stability, this change will not help us in anything.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753388091


   > So if we want to improve CI stability, this change will not help us in anything.
   
   It actually helped when I rebased on top of it in https://github.com/apache/airflow/pull/13422
   This PR was consistently failing before : 
   
   https://github.com/apache/airflow/runs/1633824804?check_suite_focus=true
   
   It might be it's the increased timeout that helped. But at the very least we will stop having misleading messages (and the problem seems to be solved). 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753406438


   I updated the description to put more emphasis on both:
   
   1) increasing the time to solve the problem (most likely this is what helps eventually)
   2) removing the reverse lookup error to make future investigation easier for anyone else. Many people looked at the problem so far an no-one fixed it before.  There is for example this problem which is mysql and has the same or similar problem:
   
   https://github.com/apache/airflow/runs/1634173114?check_suite_focus=true
   
   Yeah. there is a 'connection refused' in this case, but I think just writing the message next to the actual problem (which seems to be indeterministic) makes the problem so much harder to investigate. 
   
   I think increasing the time AND avoiding reverse lookup is best way of solving the problem now and help anyone to investigate it in the future.
   
   
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj edited a comment on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753386669


   > It looks like there is a race condition with docker compose
   that causes services that started fast enough (before DNS)
   to get a different reverse-DNS IP lookup (usually it is
   just <SERVICE> but sometimes it is
   <DOCKER_COMPOSE_APP>_<SERVICE>_1_<NETWORK>)
   
   ```
     DNS fwd/rev mismatch: rabbitmq != docker-compose_rabbitmq_1.docker-compose_default
     rabbitmq [172.25.0.8] 5672 (amqp) : Connection refused
      sent 0, rcvd 0
   ```
   
   I think we have two separate messages here. The first is a warning that always occurs if one host have multiple DNS names and run netcat in very verbose mode (``-vv``). The second is a real error message that contains information about the problemr that happened.
   
   Take a look at an example.
   ```bash
   $ nc -zvv kdc-server-example-com 88
   DNS fwd/rev mismatch: kdc-server-example-com != docker-compose_kdc-server-example-com_1.example.com
   kdc-server-example-com [10.5.0.2] 88 (kerberos) open
    sent 0, rcvd 0
   $  echo $?
   0
   $ nc -z kdc-server-example-com 88
   $ echo $?
   0
   ```
   ```
   nc -z kdc-server-example-com 9000
   $ echo $?
   1
   $ nc -zv kdc-server-example-com 9000
   DNS fwd/rev mismatch: kdc-server-example-com != docker-compose_kdc-server-example-com_1.example.com
   kdc-server-example-com [10.5.0.2] 9000 (?) : Connection refused
   $ nc -zvv kdc-server-example-com 9000
   DNS fwd/rev mismatch: kdc-server-example-com != docker-compose_kdc-server-example-com_1.example.com
   kdc-server-example-com [10.5.0.2] 9000 (?) : Connection refused
    sent 0, rcvd 0
    ```
   This warning does not affect communication in most cases and is only relevant in very restrictive network configurations, e.g. in the case of Kerberos, which tries to validate the message source a little more.
   
   So if we want to improve CI stability, this change will not help us in anything.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#discussion_r550804978



##########
File path: scripts/in_container/check_environment.sh
##########
@@ -21,6 +21,21 @@ EXIT_CODE=0
 
 DISABLED_INTEGRATIONS=""
 
+# We want to perform only forward lookup of the service IP address. Netcat when run without -n
+# Performs both forward and reverse lookup and fails if the reverse lookup name does not match
+# original name even if the host is reachable via IP. This happens randomly with docker-compose
+# in Github Actions (apparently it is some race condition when one service starts before DNS)
+# Since we are not using reverse lookup elsewhere we can perform forward lookup in python
+# And use the IP in NC and add '-n' switch to disable any DNS use.

Review comment:
       ```suggestion
   # and use the IP in NC and add '-n' switch to disable any DNS use.
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753392893


   I checked and Mongo actually starts 10 seconds after this check, so no matter what DNS entries we use, it won't help with this problem.
   <img width="986" alt="Screenshot 2021-01-01 at 23 12 22" src="https://user-images.githubusercontent.com/12058428/103447078-d5ba0a00-4c86-11eb-880f-854ef008a9ea.png">
   <img width="1000" alt="Screenshot 2021-01-01 at 23 09 59" src="https://user-images.githubusercontent.com/12058428/103447060-a0152100-4c86-11eb-9cd3-564e72577ebd.png">
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#discussion_r550804986



##########
File path: scripts/in_container/prod/entrypoint_prod.sh
##########
@@ -21,6 +21,21 @@ AIRFLOW_COMMAND="${1}"
 
 set -euo pipefail
 
+# We want to perform only forward lookup of the service IP address. Netcat when run without -n
+# Performs both forward and reverse lookup and fails if the reverse lookup name does not match
+# original name even if the host is reachable via IP. This happens randomly with docker-compose
+# in Github Actions (apparently it is some race condition when one service starts before DNS)
+# Since we are not using reverse lookup elsewhere we can perform forward lookup in python
+# And use the IP in NC and add '-n' switch to disable any DNS use.

Review comment:
       ```suggestion
   # and use the IP in NC and add '-n' switch to disable any DNS use.
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753400309


   > I checked another correct build and it was just a lot more lucky.
   
   What do you propose then? Do you want to make your own PR that will fix it? I am happy to review it. My goal is to fix the problem and seem this change fixes it and unblocks PR from contributors.
   
   Do you want to dig deeper and narrow down the problem (feel free if you want to do it) But I propose to merge this one and start focusing on important stuff (unless you would like to follow up and try another solution, but I propose to merge it first and if you have some hours to spare - feel free to dig deeper.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj edited a comment on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753392893


   I checked and Mongo actually starts 5 seconds after this check, so no matter what DNS entries we use, it won't help with this problem.
   <img width="986" alt="Screenshot 2021-01-01 at 23 12 22" src="https://user-images.githubusercontent.com/12058428/103447078-d5ba0a00-4c86-11eb-880f-854ef008a9ea.png">
   <img width="1000" alt="Screenshot 2021-01-01 at 23 09 59" src="https://user-images.githubusercontent.com/12058428/103447060-a0152100-4c86-11eb-9cd3-564e72577ebd.png">
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753389049


   Do you have other constructive proposals how to solve the problem @mik-laj ? Or are you suggesting we should not merge this one? What is your idea of solving 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753390227


   > Do you have other constructive proposals how to solve the problem @mik-laj ? Or are you suggesting we should not merge this one? What is your idea of solving it?
   
   I don't think there is a need to merge this change as it doesn't fix any real problem.  Increasing the timeout seems to me to have a greater effect on stability. We recently added another integration and apparently we are CPU constrained and all services cannot run in a limited time. Apache Pinot is a database that can take up a bit more resources.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#discussion_r550821290



##########
File path: scripts/in_container/check_environment.sh
##########
@@ -21,6 +21,21 @@ EXIT_CODE=0
 
 DISABLED_INTEGRATIONS=""
 
+# We want to perform only forward lookup of the service IP address. Netcat when run without -n
+# Performs both forward and reverse lookup and fails if the reverse lookup name does not match
+# original name even if the host is reachable via IP. This happens randomly with docker-compose
+# in Github Actions (apparently it is some race condition when one service starts before DNS)
+# Since we are not using reverse lookup elsewhere we can perform forward lookup in python
+# and use the IP in NC and add '-n' switch to disable any DNS use.

Review comment:
       I updated the description with a little different emphasis.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#issuecomment-753394017


   I checked another correct build and it was just a lot more lucky.
   https://github.com/apache/airflow/runs/1633421539
   ```
     RabbitMQ: ...................OK.  
   ```
   19 dots, when the maximum is 20.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#discussion_r550819382



##########
File path: scripts/in_container/check_environment.sh
##########
@@ -21,6 +21,21 @@ EXIT_CODE=0
 
 DISABLED_INTEGRATIONS=""
 
+# We want to perform only forward lookup of the service IP address. Netcat when run without -n
+# Performs both forward and reverse lookup and fails if the reverse lookup name does not match
+# original name even if the host is reachable via IP. This happens randomly with docker-compose
+# in Github Actions (apparently it is some race condition when one service starts before DNS)
+# Since we are not using reverse lookup elsewhere we can perform forward lookup in python
+# and use the IP in NC and add '-n' switch to disable any DNS use.

Review comment:
       ```suggestion
   # We want to perform only forward lookup of the service IP address to remove the irrelevant
   # log message - "DNS fwd/rev mismatch"
   #
   # Long explanation:
   # Netcat when run without -n, performs both forward and reverse lookup and fails if the reverse lookup name
   # does not match original name even if the host is reachable via IP. Then it displays message 
   # "DNS fwd/rev mismatch",  which introduces additional noise in the log.
   # To mitigate, we are not using reverse lookup elsewhere we can perform forward lookup in python
   # and use the IP in NC and add '-n' switch to disable any DNS use.
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#discussion_r550819609



##########
File path: scripts/in_container/prod/entrypoint_prod.sh
##########
@@ -21,6 +21,21 @@ AIRFLOW_COMMAND="${1}"
 
 set -euo pipefail
 
+# We want to perform only forward lookup of the service IP address. Netcat when run without -n
+# Performs both forward and reverse lookup and fails if the reverse lookup name does not match
+# original name even if the host is reachable via IP. This happens randomly with docker-compose
+# in Github Actions (apparently it is some race condition when one service starts before DNS)
+# Since we are not using reverse lookup elsewhere we can perform forward lookup in python
+# and use the IP in NC and add '-n' switch to disable any DNS use.

Review comment:
       ```suggestion
   # We want to perform only forward lookup of the service IP address to remove the irrelevant
   # log message - "DNS fwd/rev mismatch"
   #
   # Long explanation:
   # Netcat when run without -n, performs both forward and reverse lookup and fails if the reverse lookup name
   # does not match original name even if the host is reachable via IP. Then it displays message
   # "DNS fwd/rev mismatch",  which introduces additional noise in the log.
   # To mitigate, we are not using reverse lookup elsewhere we can perform forward lookup in python
   # and use the IP in NC and add '-n' switch to disable any DNS use.
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13424: Disables reverse IP lookup for environment check

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13424:
URL: https://github.com/apache/airflow/pull/13424#discussion_r550800822



##########
File path: scripts/in_container/check_environment.sh
##########
@@ -21,6 +21,15 @@ EXIT_CODE=0
 
 DISABLED_INTEGRATIONS=""
 
+function run_nc() {
+    local host=${1}
+    local port=${2}
+    local ip
+    ip=$(python -c "import socket; print(socket.gethostbyname('${host}'))")

Review comment:
       Could you add a comment here (shorter version of the pr desc) saying why we need this.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org