Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

Make leader-election configurable: default endpoints object namespace to controller's instead of kube-system #957

Merged
merged 2 commits into from
Aug 30, 2018

Conversation

wongma7
Copy link
Contributor

@wongma7 wongma7 commented Aug 21, 2018

Technically this will not break anything. The worst that will happen is users may have useless endpoints objects left over in the namespace kube-system when they update to this version of the controller. If they've already given cluster permissions over endpoints, they will not see any disruption. However, the purpose of this change is to make it so that they don't have to give cluster permissions over endpoints in the first place, and ideally they will at some point reduce those permissions to a single provisioner-isolated namespace.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 21, 2018
@wongma7 wongma7 force-pushed the leader-election-config branch 4 times, most recently from b418a7b to 6038002 Compare August 23, 2018 20:33
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 23, 2018
@wongma7 wongma7 force-pushed the leader-election-config branch 2 times, most recently from 1d9af79 to 7fec192 Compare August 23, 2018 20:44
@wongma7 wongma7 changed the title [WIP] Make leader-election configurable: default endpoints object namespace to controller's instead of kube-system Make leader-election configurable: default endpoints object namespace to controller's instead of kube-system Aug 23, 2018
@wongma7 wongma7 force-pushed the leader-election-config branch from 7fec192 to 9dc43ca Compare August 23, 2018 21:01
@wongma7
Copy link
Contributor Author

wongma7 commented Aug 23, 2018

/cc @verult
leader election enabled by default. Instead of clusterwide endpoint editing privileges, reduce it to a single namespace. The problem is we cannot know what namespace the provisioner serviceaccount will be created in so we need to instruct users to do something like 9dc43ca#diff-b6908537b8e41b48f4e049f946c70aeeR145

@k8s-ci-robot
Copy link
Contributor

@wongma7: GitHub didn't allow me to request PR reviews from the following users: verult.

Note that only kubernetes-incubator members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @verult
leader election enabled by default. Instead of clusterwide endpoint editing privileges, reduce it to a single namespace. The problem is we cannot know what namespace the provisioner serviceaccount will be created in so we need to instruct users to do something like 9dc43ca#diff-b6908537b8e41b48f4e049f946c70aeeR145

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wongma7 wongma7 force-pushed the leader-election-config branch from 9dc43ca to 8e3bfd3 Compare August 24, 2018 14:04
@verult
Copy link
Contributor

verult commented Aug 28, 2018

/cc @jsafrane who's familiar with leader election in external-attacher

@wongma7 wongma7 merged commit 7613414 into kubernetes-retired:master Aug 30, 2018
@verult
Copy link
Contributor

verult commented Sep 1, 2018

With this PR, every CSI driver deployment has to have its own leader-election role and rolebinding to the controller service account, right? Just want to make sure we don't need to update k8s default bootstrapped RBAC policies.

@wongma7
Copy link
Contributor Author

wongma7 commented Sep 12, 2018

@verult yes. same as external-attacher

SaravanaStorageNetwork added a commit to SaravanaStorageNetwork/openshift-ansible that referenced this pull request Jun 25, 2019
This in turn throws a error when gluster-block pod is provisioned.

So, add ClusterRole to perform read/write with endpoint.

Note: endpoint is added in glusterblock-provisioner as part of
the PR: kubernetes-retired/external-storage#957

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1723366

Signed-off-by: Saravanakumar Arumugam <sarumuga@redhat.com>
SaravanaStorageNetwork added a commit to SaravanaStorageNetwork/gluster-kubernetes that referenced this pull request Jun 26, 2019
endpoint is missing as part of ClusterRole rule.
This in turn throws a error when gluster-block pod is provisioned.

So, add ClusterRole to perform read/write with endpoint.

Note: endpoint is added in glusterblock-provisioner as part of
the PR: kubernetes-retired/external-storage#957

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1723366

Signed-off-by: Saravanakumar <sarumuga@redhat.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/openshift-ansible that referenced this pull request Jun 26, 2019
…a error when gluster-block pod is provisioned.

So, add ClusterRole to perform read/write with endpoint.

Note: endpoint is added in glusterblock-provisioner as part of
the PR: kubernetes-retired/external-storage#957

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1723366

Signed-off-by: Saravanakumar Arumugam <sarumuga@redhat.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/openshift-ansible that referenced this pull request Jun 26, 2019
…a error when gluster-block pod is provisioned.

So, add ClusterRole to perform read/write with endpoint.

Note: endpoint is added in glusterblock-provisioner as part of
the PR: kubernetes-retired/external-storage#957

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1723366

Signed-off-by: Saravanakumar Arumugam <sarumuga@redhat.com>
megian pushed a commit to vshn/openshift-ansible that referenced this pull request Jul 25, 2019
…a error when gluster-block pod is provisioned.

So, add ClusterRole to perform read/write with endpoint.

Note: endpoint is added in glusterblock-provisioner as part of
the PR: kubernetes-retired/external-storage#957

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1723366

Signed-off-by: Saravanakumar Arumugam <sarumuga@redhat.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/lib cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants