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/08/11 14:57:17 UTC

[GitHub] [airflow] potiuk opened a new pull request, #25678: Convert Helm tests to use the new Python Breeeze

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

   This PR converts the Helm tests to use the new Python Breeze.
   It has all the features of previous Kind breeze command and more.
   
   All the commands are now grupped under k8s group and they are very
   easy to use locally (even easier than the previous version).
   
   The CI part is also converted and simplified - i.e. the upgrade
   test is now much faster (only tests one upgrade per job and it
   runs withing the original Helm/Kubernetes tests jobs so it will
   not have the cluster creation overhead.
   
   Most importantly - this is almost the last step before we can get
   rid of the old legacy breeze code and one that we can get rid of
   the `./breeze-legacy` script because all functionality from the
   old breeze has been moved to the Python version with this change.
   
   This removl allows us also to remove a lot of the common library
   bash code that is not used any more anywhere - even in CI.
   
   The only change left is running regular tests in parallel.
   
   Closes: #23085
   
   <!--
   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


[GitHub] [airflow] potiuk commented on pull request #25678: Convert Helm tests to use the new Python Breeze

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

   cc: @eladkal @Bowrna @edithturn -> Really close (I am looking at Bowrna's PR next).


-- 
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 commented on pull request #25678: Convert Helm tests to use the new Python Breeze

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

   Gently poking here .... - Less `bash` == GOOD


-- 
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 #25678: Convert Helm tests to use the new Python Breeze

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


-- 
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 commented on pull request #25678: Convert Helm tests to use the new Python Breeeze

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

   Almost the last one to replace old breeze. This one is big but it replaces the `kind` functionality used for Helm tests. 
   
   I still will have to test the parallel runs and ci  but wanted to push it now.


-- 
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 commented on pull request #25678: Convert Helm tests to use the new Python Breeeze

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

   There you go @edithturn (and @Bowrna ) we are getting really close to complete it with that one 


-- 
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 commented on pull request #25678: Convert Helm tests to use the new Python Breeze

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

   ALL GREEN!  Finally!


-- 
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 commented on pull request #25678: Convert Helm tests to use the new Python Breeeze

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

   cc: @rossturk - this one also removes the breeze-legacy and we are getting very, very close to replacing it completely (but I already removed all the movie references from BREEZE.rst.


-- 
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 commented on pull request #25678: Convert Helm tests to use the new Python Breeze

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

   I believe this one should be already green and mergable, and it is what I wanted it to be. It's WAY nicer comparing to what we had before and as explained in #26091 we will be able to further improve the development experience of Airlfow running on K8S for everyone as a follow-up.
   
   @dstandish @ephraimbuddy  @jedcunningham  - I think you were one of the users there (or maybe non-users because of not-so-great experience).
   
   This Python-Breeze version has a lot of improvements comparing to the legacy bash one:
   
   * The CI has 1-1 mapping to the local development commands. You follow exactly the same sequence locally to run tests locally as you do in CI (screenshot below)
   
   * The commands are consistent with the rest of the commands of breeze , and allow both local single-cluster running as well as CI running in parallel 
   
   * The documentation is simplified - some of the duplication was removed and it's much clearer now how to approach it IMHO
   
   * Also the commands to setup testable Airlfow instance are pretty much "wizard-like" (@rossturk that is the result of our discussions). When you create cluster, it tells you "NEXT STEP:" that you should configure it and tells you the command, when you configure it it asks you to build k8s as "NEXT STEP" and tells the command, and so on. It basically guides the user step-by-step on what to do next. You can literally re-run all k8c tests by running `breeze k8s create-cluster` and following the "NEXT STEPS" instructions. 
   
   * The "K8S upgrade" step is now part of the regular jobs. We save a lot of time for setting up the clusters and deploying airlfow again as they are already set-up in previous steps and we simply need to add one "upgrade" step with some changed valiues (@ephraimbuddy  - you will be happy). This makes the tests less resource hungry and faster.
   
   * I found and fixed a few stability issues that plagued us with the Helm tests. They should be much more solid now in general. That included necessary increase in our CI machine - to add more inotify watchers on system level for example (and few other lower-level stuff).
   
   * It's all Python
   
   * The output of CI is really nice and tells exactly what happens
   
   Example succesful run with multiple clusters in parallel:
   
   https://github.com/apache/airflow/runs/8114807752?check_suite_focus=true
   
   Some screenshots:
   
   Steps in the CI job:
   
   ![image](https://user-images.githubusercontent.com/595491/187769369-399bdac4-f1fe-4c78-8c89-fd6294483b69.png)
   
   Job summary:
   
   ![image](https://user-images.githubusercontent.com/595491/187770006-00503ec1-22eb-4d5f-b2f8-ddcba91686d3.png)
   
   
   Job details after unfolding:
   
   ![image](https://user-images.githubusercontent.com/595491/187770172-da012851-877b-4c7c-b31a-09d9ec330828.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.

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 commented on pull request #25678: Convert Helm tests to use the new Python Breeze

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

   I know it's big, but I could not figure a way how to split it - because it basically removes bash and replaces it with Python in Breeze and there are enough differences that it is hard to get this one half-done.


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