-
Notifications
You must be signed in to change notification settings - Fork 14.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Helm chart fixes in pod template #11511
Conversation
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/master/CONTRIBUTING.rst)
|
c50d424
to
5c1a1e9
Compare
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*. |
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*. |
Hello @fllaca, Can you please rebase this one to latest master. We fixed (hopefully) a problem with queues of jobs for GitHub actions and I think when you rebase, it shoudl run much faster (more info on devlist shortly). |
5c1a1e9
to
159178e
Compare
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*. |
path: spec.containers[0].image | ||
value: dummy_image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find a way to assert that this value should be the default defaultAirflowRepository
without hardcoding the test with apache/airflow:1.10.12
(copying from values.yaml). If that hardcoded value in the assertion is fine, I can change the test back to equal
assert.
c210eb2
to
150fa81
Compare
Hi @potiuk! thanks for taking a look to this. I just rebased and added some helm test. One of the kubernetes test is failing ( |
150fa81
to
70fb4d1
Compare
- default pod_template image to `defaultAirflowRepository:defaultAirflowTag` - fix never-ending git-sync init containers - fix broken reference to volume
70fb4d1
to
d2b4b64
Compare
A very interesting change. I did not know the I also see that you made sure that this change had good unit test coverage. We are working to modernize our unit tests to overcome some of the limitations of Helm-unittest. One of its limitations even you have encountered - #11511 (comment) |
Awesome work, congrats on your first merged pull request! |
* Helm chart fixes in pod template - default pod_template image to `defaultAirflowRepository:defaultAirflowTag` - fix never-ending git-sync init containers - fix broken reference to volume * Fix helm chart test (cherry picked from commit 52b4733)
* Helm chart fixes in pod template - default pod_template image to `defaultAirflowRepository:defaultAirflowTag` - fix never-ending git-sync init containers - fix broken reference to volume * Fix helm chart test (cherry picked from commit 52b4733)
* Helm chart fixes in pod template - default pod_template image to `defaultAirflowRepository:defaultAirflowTag` - fix never-ending git-sync init containers - fix broken reference to volume * Fix helm chart test (cherry picked from commit 52b4733)
* Helm chart fixes in pod template - default pod_template image to `defaultAirflowRepository:defaultAirflowTag` - fix never-ending git-sync init containers - fix broken reference to volume * Fix helm chart test (cherry picked from commit 52b4733)
) * Helm chart fixes in pod template - default pod_template image to `defaultAirflowRepository:defaultAirflowTag` - fix never-ending git-sync init containers - fix broken reference to volume * Fix helm chart test (cherry picked from commit 52b4733)
I came with some little issues while testing the chart in this repo locally, here I'm bundling some fixes to them:
defaultAirflowRepository:defaultAirflowTag