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

Prefix leader election lock name with "external-provisioner-leader" #295

Closed
verult opened this issue Jun 17, 2019 · 10 comments
Closed

Prefix leader election lock name with "external-provisioner-leader" #295

verult opened this issue Jun 17, 2019 · 10 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Milestone

Comments

@verult
Copy link
Contributor

verult commented Jun 17, 2019

CSI has many controllers with leader election functionality. It's best to disambiguate between all the different locks used for LE with a provisioner-specific prefix.

However, a rolling upgrade to a version that includes the change will result in the possibility that two different locks are used at a given time, resulting in two leaders temporarily.

Should this be reconsidered when 2.0 rolls out?

@msau42
Copy link
Collaborator

msau42 commented Jun 17, 2019

We should make lock namespace configurable too.

@msau42
Copy link
Collaborator

msau42 commented Jun 17, 2019

We can make it an option to not break backwards compatibility.

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 17, 2019
@verult
Copy link
Contributor Author

verult commented Jun 18, 2019

For a small bug like this I don't think adding a new flag option is justified. It could potentially make the provisioner a little harder to use, and exposing anything in the user interface carries some permanence.

I'm personally OK with leaving the fix until 2.0, and make sure every other CSI controller prefixes before moving to 1.0.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 16, 2019
@verult
Copy link
Contributor Author

verult commented Sep 16, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 16, 2019
@msau42
Copy link
Collaborator

msau42 commented Oct 4, 2019

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Oct 4, 2019
@msau42 msau42 added this to the 2.0 milestone Dec 24, 2019
@msau42
Copy link
Collaborator

msau42 commented Apr 2, 2020

/help

@k8s-ci-robot
Copy link
Contributor

@msau42:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

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.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Apr 2, 2020
@msau42
Copy link
Collaborator

msau42 commented Jun 25, 2020

After some discussion we decided not to do this, with the consideration that we may want to combine all the sidecars in the future.
/close

@k8s-ci-robot
Copy link
Contributor

@msau42: Closing this issue.

In response to this:

After some discussion we decided not to do this, with the consideration that we may want to combine all the sidecars in the future.
/close

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.

kbsonlong pushed a commit to kbsonlong/external-provisioner that referenced this issue Dec 29, 2023
master: update release-tools + dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

4 participants