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

Single/Multi-Namespace mode for helm chart #11034

Merged
merged 5 commits into from
Oct 5, 2020

Conversation

dimberman
Copy link
Contributor

Users should not REQUIRE a ClusterRole/ClusterRolebinding
to run airflow via helm. This change will allow "single" and "multi"
namespace modes so users can add airflow to managed kubernetes clusters


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

@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Sep 20, 2020
@dimberman dimberman requested review from potiuk, ashb and kaxil September 20, 2020 17:44
@dimberman
Copy link
Contributor Author

cc: @sjmiller609

kind: ClusterRole
{{- else }}
kind: Role
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you create a Role, you have to specify the namespace it belongs in.

ClusterRole, by contrast, is a non-namespaced resource.

https://kubernetes.io/docs/reference/access-authn-authz/rbac/

Can you add namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mik-laj fixed 👍

@mik-laj
Copy link
Member

mik-laj commented Sep 20, 2020

Thanks for addressing this issue. I've also encountered this problem recently when I was creating an environment for multiple teams on one cluster.

For now, I've added this option, but I think it's worth solving this problem.

  set {
    name  = "allowPodLaunching"
    value = "false"
  }

# yamllint enable rule:line-length

multiNamespaceMode: 'False'
Copy link
Member

@mik-laj mik-laj Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also update README.md? The minimum is updating the reference documentation, but I'd be happy if you would describe more about what "Multi namespace mode" means.

Reference documentation will be easier to maintain if validation for values.yml has been added.
#10664

@mik-laj
Copy link
Member

mik-laj commented Sep 21, 2020

Can you do rebase and update values.json?

@dimberman
Copy link
Contributor Author

@mik-laj rebased

@mik-laj
Copy link
Member

mik-laj commented Sep 22, 2020

Can you add new item to values.schema.json?

@dimberman
Copy link
Contributor Author

@mik-laj added

Users should not REQUIRE a ClusterRole/ClusterRolebinding
to run airflow via helm. This change will allow "single" and "multi"
namespace modes so users can add airflow to managed kubernetes clusters
@dimberman dimberman merged commit 93475e9 into apache:master Oct 5, 2020
@dimberman dimberman deleted the multi-namespace-helm branch October 5, 2020 17:38
FloChehab pushed a commit to MeilleursAgents/airflow that referenced this pull request Oct 20, 2020
* Followup to apache#11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False
dimberman pushed a commit that referenced this pull request Oct 20, 2020
* Followup to #11034
* Was not referencing the correct kind of resources if multiNamespaceMode = False
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
* Multi-Namespace mode for helm chart

Users should not REQUIRE a ClusterRole/ClusterRolebinding
to run airflow via helm. This change will allow "single" and "multi"
namespace modes so users can add airflow to managed kubernetes clusters

* add namespace to role

* add rolebinding too

* add docs

* add values.schema.json change

(cherry picked from commit 93475e9)
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 the type:improvement Changelog: Improvements label Nov 15, 2020
@potiuk potiuk added this to the Airflow 1.10.13 milestone Nov 15, 2020
potiuk pushed a commit that referenced this pull request Nov 16, 2020
* Multi-Namespace mode for helm chart

Users should not REQUIRE a ClusterRole/ClusterRolebinding
to run airflow via helm. This change will allow "single" and "multi"
namespace modes so users can add airflow to managed kubernetes clusters

* add namespace to role

* add rolebinding too

* add docs

* add values.schema.json change

(cherry picked from commit 93475e9)
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
* Multi-Namespace mode for helm chart

Users should not REQUIRE a ClusterRole/ClusterRolebinding
to run airflow via helm. This change will allow "single" and "multi"
namespace modes so users can add airflow to managed kubernetes clusters

* add namespace to role

* add rolebinding too

* add docs

* add values.schema.json change

(cherry picked from commit 93475e9)
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
* Multi-Namespace mode for helm chart

Users should not REQUIRE a ClusterRole/ClusterRolebinding
to run airflow via helm. This change will allow "single" and "multi"
namespace modes so users can add airflow to managed kubernetes clusters

* add namespace to role

* add rolebinding too

* add docs

* add values.schema.json change

(cherry picked from commit 93475e9)
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
* Multi-Namespace mode for helm chart

Users should not REQUIRE a ClusterRole/ClusterRolebinding
to run airflow via helm. This change will allow "single" and "multi"
namespace modes so users can add airflow to managed kubernetes clusters

* add namespace to role

* add rolebinding too

* add docs

* add values.schema.json change

(cherry picked from commit 93475e9)
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:improvement Changelog: Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants