Skip to content

Conversation

@mlmhl
Copy link
Contributor

@mlmhl mlmhl commented Apr 24, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:

The valid leader election types are configmaps and leases, but the default value of flag leaderElectionType is configmap, which is illegal. This PR changes the default value to configmaps.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

The default leader election type will be `configmaps` if not specified in the command line

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 24, 2019
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 24, 2019
@mlmhl
Copy link
Contributor Author

mlmhl commented Apr 24, 2019

/assign @msau42


enableLeaderElection = flag.Bool("leader-election", false, "Enable leader election.")
leaderElectionType = flag.String("leader-election-type", "configmap", "the type of leader election, options are 'configmaps' (default) or 'leases' (recommended). The 'configmaps' option is deprecated in favor of 'leases'.")
leaderElectionType = flag.String("leader-election-type", "configmaps", "the type of leader election, options are 'configmaps' (default) or 'leases' (recommended). The 'configmaps' option is deprecated in favor of 'leases'.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we define constants for these? It could help avoid typos in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, already updated, PTAL :)

@mlmhl mlmhl force-pushed the leader-electino-type branch from 28ded6d to a53ef38 Compare April 25, 2019 11:59
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 25, 2019
@msau42
Copy link
Collaborator

msau42 commented Apr 25, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mlmhl, msau42

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 25, 2019
@k8s-ci-robot k8s-ci-robot merged commit e18bd66 into kubernetes-csi:master Apr 25, 2019
k8s-ci-robot added a commit that referenced this pull request Apr 26, 2019
Cherry pick #144 to release-1.1: fix default leader election type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants