Chart: Use stable API versions where available #17211

cablespaghetti · 2021-07-25T19:17:18Z

Uses stable versions of Ingress, PodDisruptionBudget and CronJob APIs where available. There is no change for users of Kubernetes versions older than 1.19. Also tidies up some inconsistencies between the two Ingresses and removes variables on the flower Ingress which didn't do anything.

boring-cyborg · 2021-07-25T19:17:21Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

Pay attention to the quality of your code (flake8, mypy and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack

potiuk · 2021-07-26T19:58:37Z

CC: @jedcunningham @kaxil WDYT ?

github-actions · 2021-07-27T05:29:23Z

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

jedcunningham

Overall LGTM, thanks @cablespaghetti! Can you add test coverage in chart/tests as well though?

cablespaghetti · 2021-08-04T08:33:47Z

Overall LGTM, thanks @cablespaghetti! Can you add test coverage in chart/tests as well though?

Sorry for the delay in this. I will get to this as soon as I can. I was having some trouble getting a working dev environment on my M1 Mac initially but I'm sure it won't be that hard to resolve.

renan · 2021-08-24T12:00:02Z

New versions of Ingress controllers (eg: ingress-nginx) are dropping support for v1beta1 and Airflow is blocking us from updating to them.

Is there an ETA for this PR?

kaxil · 2021-08-24T12:52:41Z

New versions of Ingress controllers (eg: ingress-nginx) are dropping support for v1beta1 and Airflow is blocking us from updating to them.

Is there an ETA for this PR?

As soon as @cablespaghetti adds the tests :)

cablespaghetti · 2021-08-24T13:51:19Z

Really sorry this has taken so long, I have been super busy both at work and outside. I will put in some time to try and get this done tomorrow.

cablespaghetti · 2021-08-25T16:33:23Z

Ok hopefully I've done enough now. Also upgraded the default it tests against to 1.22. Rerunning everything locally to check this hasn't broken anything at the moment.

edit: They passed 👍

cablespaghetti · 2021-08-25T16:36:54Z

chart/values.yaml

@@ -120,24 +129,24 @@ ingress:
    annotations: {}

    # The path for the flower Ingress
-    path: ""
+    path: "/"


Both path and pathType are mandatory in the stable api. I don't think setting it by default is likely break many people's config but thought I'd point it out.

cablespaghetti · 2021-09-05T18:07:36Z

I appreciate I can't complain as I took a very long time to complete the work on this PR. However just a reminder to @kaxil and @jedcunningham that this should be ready to go now. 👍

chart/values.schema.json

Signed-off-by: Sam Weston <sam.weston@tails.com>

cablespaghetti · 2021-09-10T09:39:34Z

Ok it is quite rightly erroring since I changed the default for Ingress path from "" to "/". I'll change that in the schema and update the UPDATING.rst file. The other error seems to be about an unrelated SQLAlchemy deprecation though so I'll ignore that rather than make an unrelated change in this PR.

Signed-off-by: Sam Weston <sam.weston@tails.com>

cablespaghetti · 2021-09-10T15:35:18Z

I don't understand the test failure we've currently got. 😞

jedcunningham · 2021-09-10T15:40:41Z

I don't understand the test failure we've currently got. 😞

Most likely a flaky failure, let's try it again.

boring-cyborg · 2021-09-16T17:56:42Z

Awesome work, congrats on your first merged pull request!

jedcunningham · 2021-09-16T17:56:58Z

Thanks @cablespaghetti! Congrats on your first commit.

Partial Commit Extracted From: https://github.com/apache/airflow

cablespaghetti requested review from ashb, dimberman, jedcunningham and kaxil as code owners July 25, 2021 19:17

boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Jul 25, 2021

cablespaghetti force-pushed the feature/ingress-v1 branch from d1b4d14 to f9f52c2 Compare July 25, 2021 19:24

mik-laj approved these changes Jul 27, 2021

View reviewed changes

github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Jul 27, 2021

jedcunningham requested changes Jul 27, 2021

View reviewed changes

jedcunningham added this to the Airflow Helm Chart 1.2.0 milestone Aug 4, 2021

cablespaghetti force-pushed the feature/ingress-v1 branch from f9f52c2 to 7b9596d Compare August 25, 2021 13:45

cablespaghetti requested a review from jedcunningham August 25, 2021 16:29

cablespaghetti commented Aug 25, 2021

View reviewed changes

jedcunningham reviewed Sep 8, 2021

View reviewed changes

chart/values.schema.json Show resolved Hide resolved

samweston-tails added 7 commits September 9, 2021 14:27

Make Helm changes required for Ingress v1

75287e7

Signed-off-by: Sam Weston <sam.weston@tails.com>

Tidying up of ingress resources and changing default of pathType

228eb7a

Signed-off-by: Sam Weston <sam.weston@tails.com>

Use stable APIs introduced in Kubernetes 1.21

bc57359

Signed-off-by: Sam Weston <sam.weston@tails.com>

Tidy up newline in Ingress annotations

d1ca990

Signed-off-by: Sam Weston <sam.weston@tails.com>

Get existing Ingress tests working against 1.22 API version

693652e

Signed-off-by: Sam Weston <sam.weston@tails.com>

Add tests for all conditional API versions

eb74596

Signed-off-by: Sam Weston <sam.weston@tails.com>

Add tests for ingressClassName

bbc5a51

Signed-off-by: Sam Weston <sam.weston@tails.com>

Address flake8 error and add note to UPDATING.rst

1e799f3

Signed-off-by: Sam Weston <sam.weston@tails.com>

cablespaghetti force-pushed the feature/ingress-v1 branch from 7d38afa to 1e799f3 Compare September 9, 2021 13:33

samweston-tails added 2 commits September 10, 2021 10:54

Update JSON schema to account for change in Ingress path default

8159213

Signed-off-by: Sam Weston <sam.weston@tails.com>

Fix RST linting

fbe979a

Signed-off-by: Sam Weston <sam.weston@tails.com>

jedcunningham approved these changes Sep 10, 2021

View reviewed changes

jedcunningham closed this Sep 10, 2021

jedcunningham reopened this Sep 10, 2021

jedcunningham closed this Sep 15, 2021

jedcunningham reopened this Sep 15, 2021

jedcunningham merged commit 73044af into apache:main Sep 16, 2021

cognifloyd pushed a commit to cognifloyd/stackstorm-k8s that referenced this pull request Feb 16, 2022

Chart: Use stable API versions where available (apache/airflow#17211)

6a54d7a

Partial Commit Extracted From: https://github.com/apache/airflow

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chart: Use stable API versions where available #17211

Chart: Use stable API versions where available #17211

cablespaghetti commented Jul 25, 2021

boring-cyborg bot commented Jul 25, 2021

potiuk commented Jul 26, 2021

github-actions bot commented Jul 27, 2021

jedcunningham left a comment

cablespaghetti commented Aug 4, 2021

renan commented Aug 24, 2021

kaxil commented Aug 24, 2021

cablespaghetti commented Aug 24, 2021 •

edited

Loading

cablespaghetti commented Aug 25, 2021 •

edited

Loading

cablespaghetti Aug 25, 2021

cablespaghetti commented Sep 5, 2021

cablespaghetti commented Sep 10, 2021

cablespaghetti commented Sep 10, 2021

jedcunningham commented Sep 10, 2021

boring-cyborg bot commented Sep 16, 2021

jedcunningham commented Sep 16, 2021 •

edited

Loading

Chart: Use stable API versions where available #17211

Chart: Use stable API versions where available #17211

Conversation

cablespaghetti commented Jul 25, 2021

boring-cyborg bot commented Jul 25, 2021

potiuk commented Jul 26, 2021

github-actions bot commented Jul 27, 2021

jedcunningham left a comment

Choose a reason for hiding this comment

cablespaghetti commented Aug 4, 2021

renan commented Aug 24, 2021

kaxil commented Aug 24, 2021

cablespaghetti commented Aug 24, 2021 • edited Loading

cablespaghetti commented Aug 25, 2021 • edited Loading

cablespaghetti Aug 25, 2021

Choose a reason for hiding this comment

cablespaghetti commented Sep 5, 2021

cablespaghetti commented Sep 10, 2021

cablespaghetti commented Sep 10, 2021

jedcunningham commented Sep 10, 2021

boring-cyborg bot commented Sep 16, 2021

jedcunningham commented Sep 16, 2021 • edited Loading

cablespaghetti commented Aug 24, 2021 •

edited

Loading

cablespaghetti commented Aug 25, 2021 •

edited

Loading

jedcunningham commented Sep 16, 2021 •

edited

Loading