Skip to content
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

fix pod launcher rolebinding in helm chart #11675

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

FloChehab
Copy link
Contributor

Hello,

This PR is a tiny followup to #11034: if multiNamespaceMode = False then we should reference a Role and not a ClusterRole in the roleRef of pod-launcher-rolebinding.yaml.

Also,
with #11034 & this PR, we should be able to close #10190.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

* Followup to apache#11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False
@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Oct 20, 2020
@FloChehab
Copy link
Contributor Author

FloChehab commented Oct 20, 2020

Also, there is an issue in the values.yaml https://github.com/apache/airflow/blob/master/chart/values.yaml#L643

'False' string is not falsy for helm, so this it's not working. Should be a plain false.

@dimberman dimberman merged commit 3391c90 into apache:master Oct 20, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 20, 2020

Awesome work, congrats on your first merged pull request!

@FloChehab FloChehab deleted the fix-multinamespace-helm branch October 21, 2020 08:36
@FloChehab
Copy link
Contributor Author

Thanks @dimberman .
How would you like to proceed for my comment above: #11675 (comment) ? (it is working for the value above: https://github.com/apache/airflow/blob/master/chart/values.yaml#L640 but not internally for helm)

michalmisiewicz pushed a commit to michalmisiewicz/airflow that referenced this pull request Oct 30, 2020
* Followup to apache#11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False
potiuk pushed a commit that referenced this pull request Nov 15, 2020
* Followup to #11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False

(cherry picked from commit 3391c90)
@potiuk potiuk added this to the Airflow 1.10.13 milestone Nov 15, 2020
@potiuk potiuk added the type:bug-fix Changelog: Bug Fixes label Nov 15, 2020
potiuk pushed a commit that referenced this pull request Nov 16, 2020
* Followup to #11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False

(cherry picked from commit 3391c90)
potiuk pushed a commit that referenced this pull request Nov 16, 2020
* Followup to #11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False

(cherry picked from commit 3391c90)
kaxil pushed a commit that referenced this pull request Nov 18, 2020
* Followup to #11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False

(cherry picked from commit 3391c90)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
* Followup to apache#11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False

(cherry picked from commit 3391c90)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:helm-chart Airflow Helm Chart type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants