-
Notifications
You must be signed in to change notification settings - Fork 373
Add command line arguments to configure leader election options #643
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
Conversation
|
Looks good once the go fmt issue is fixed |
|
/lgtm |
|
It might be good to make these changes in the other sidecars too so the leader election options stay in sync. /assign @xing-yang for approver |
|
@ggriffiths yup thats the plan! Thanks for the quick review! |
|
/assign @xing-yang |
|
Should there be "--" in front of these new options? |
My understanding is that these are tunable options when starting the external-provisioner so whoever deploys the CSI driver can configure them? |
Fixed the description! And redeployed with the
That's correct, they can be set before creating the pod. I've also added the parameters to the documentation page. |
|
@RaunakShah, I see the following in PR description: If these options are tunable, why you said "there's no way for a customer/storage plugin to configure those values"? Do you mean, without this change, they can't be configured; but with this change, they are tunable? Maybe separate "the current situation" and "with this change" into two paragraphs? |
@xing-yang This is correct, apologies for the confusion. I've updated the PR description with more clarity! |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RaunakShah, xing-yang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@RaunakShah Thanks for the PR. Do you mind making the similar changes to other csi sidecars? Thanks! |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds command line arguments for leader election options - Lease Duration, Renew Deadline and Retry Period. Currently the defaults are 15, 10 and 5 seconds respectively. Without this change, there's no way for a customer/storage plugin to configure those values.
This PR makes those parameters configurable. Storage plugins deploying external-provisioner can tune those parameters in their YAMLs accordingly.
Which issue(s) this PR fixes:
Fixes #556
Special notes for your reviewer:
Testing:
Created Provisioner with new arguments and verified that Pod is running:
Does this PR introduce a user-facing change?: