Make leader-election flag consistent with other sidecar #402

Madhu-1 · 2020-02-05T06:12:51Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
This PR removes the --enable-leader-election and leader-election-type flag and add leader-election flag as other sidecar containers.

Which issue(s) this PR fixes:

Fixes #401

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

ACTION REQUIRED: This introduces a new flag `leader-election` which is the same as in other sidecar containers and also removes the `enable-leader-election` and `leader-election-type` flag

k8s-ci-robot · 2020-02-05T06:12:59Z

Hi @Madhu-1. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Madhu-1 · 2020-02-05T06:13:36Z

/assign @msau42
/assign @jsafrane

jsafrane · 2020-02-05T11:52:40Z

/ok-to-test

jsafrane · 2020-02-05T11:55:32Z

cmd/csi-provisioner/csi-provisioner.go

-	enableLeaderElection    = flag.Bool("enable-leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")
+	_ = deprecatedflags.Add("enable-leader-election")
+
+	enableLeaderElection = flag.Bool("leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")


This is a breaking change that requires bump to v2.0.
@msau42, can we expect that the next release will be 2.0?

@Madhu-1, depending on the answer to above question, would it be possible to support both enable-leader-election and leader-election at the same time until we're ready for v2.0? It's OK to throw error if both are specified.

@jsafrane yes I can do that, will wait for @msau42 response

I just released a 1.6, and mentioned this in the deprecation notes. We can proceed and just break it.

msau42 · 2020-04-02T21:39:02Z

cmd/csi-provisioner/csi-provisioner.go

-	enableLeaderElection    = flag.Bool("enable-leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")
+	_ = deprecatedflags.Add("enable-leader-election")
+
+	enableLeaderElection = flag.Bool("leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")


I just released a 1.6, and mentioned this in the deprecation notes. We can proceed and just break it.

msau42 · 2020-04-02T21:39:26Z

cmd/csi-provisioner/csi-provisioner.go

@@ -64,7 +64,10 @@ var (
 	operationTimeout     = flag.Duration("timeout", 10*time.Second, "Timeout for waiting for creation or deletion of a volume")
 	_                    = deprecatedflags.Add("provisioner")

-	enableLeaderElection    = flag.Bool("enable-leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")
+	_ = deprecatedflags.Add("enable-leader-election")


I think we can just remove the old arg.

msau42 · 2020-04-02T21:39:45Z

cmd/csi-provisioner/csi-provisioner.go

+	_ = deprecatedflags.Add("enable-leader-election")
+
+	enableLeaderElection = flag.Bool("leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")
+
 	leaderElectionType      = flag.String("leader-election-type", "endpoints", "the type of leader election, options are 'endpoints' (default) or 'leases' (strongly recommended). The 'endpoints' option is deprecated in favor of 'leases'.")


And remove the "type" arg. We will only support leases going forward.

This PR removes the `--enable-leader-election` flag and add `leader-election` flag as other sidecar containers.

Madhu-1 · 2020-04-03T10:49:47Z

@msau42 changes done PTAL

msau42 · 2020-04-03T15:34:45Z

/lgtm
/approve

k8s-ci-robot · 2020-04-03T15:34:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Madhu-1, msau42

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [msau42]

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

msau42 · 2020-04-10T20:36:11Z

@Madhu-1 seems like the travis pr job is stuck. Can you try pushing a change to see if that helps the job restart?

msau42 · 2020-04-15T18:10:31Z

Travis pull jobs have been disabled on this repo now. Force merging.

…go_modules/google.golang.org/grpc-1.52.0 Bump google.golang.org/grpc from 1.51.0 to 1.52.0

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 5, 2020

k8s-ci-robot requested review from lpabon and sbezverk February 5, 2020 06:13

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 5, 2020

k8s-ci-robot assigned jsafrane and msau42 Feb 5, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 5, 2020

jsafrane reviewed Feb 5, 2020

View reviewed changes

Madhu-1 mentioned this pull request Feb 5, 2020

Arg "--csiTimeout" is not consistent with other sidecars kubernetes-csi/external-resizer#72

Closed

msau42 reviewed Apr 2, 2020

View reviewed changes

Make leader-election flag consistent with other sidecar

69ce21c

This PR removes the `--enable-leader-election` flag and add `leader-election` flag as other sidecar containers.

Madhu-1 force-pushed the fix-401 branch from e3b975b to 69ce21c Compare April 3, 2020 10:50

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 3, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 3, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 3, 2020

msau42 merged commit aa1ce81 into kubernetes-csi:master Apr 15, 2020

k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 31, 2020

kbsonlong pushed a commit to kbsonlong/external-provisioner that referenced this pull request Dec 29, 2023

Merge pull request kubernetes-csi#402 from kubernetes-csi/dependabot/…

a61ab4e

…go_modules/google.golang.org/grpc-1.52.0 Bump google.golang.org/grpc from 1.51.0 to 1.52.0

Madhu-1 mentioned this pull request Apr 3, 2024

REQUEST: New membership for Madhu-1 kubernetes/org#4867

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make leader-election flag consistent with other sidecar #402

Make leader-election flag consistent with other sidecar #402

Madhu-1 commented Feb 5, 2020 •

edited by msau42

Loading

k8s-ci-robot commented Feb 5, 2020

Madhu-1 commented Feb 5, 2020

jsafrane commented Feb 5, 2020

jsafrane Feb 5, 2020

Madhu-1 Feb 5, 2020

msau42 Apr 2, 2020

msau42 Apr 2, 2020

msau42 Apr 2, 2020

msau42 Apr 2, 2020

Madhu-1 commented Apr 3, 2020

msau42 commented Apr 3, 2020

k8s-ci-robot commented Apr 3, 2020

msau42 commented Apr 10, 2020

msau42 commented Apr 15, 2020

Make leader-election flag consistent with other sidecar #402

Make leader-election flag consistent with other sidecar #402

Conversation

Madhu-1 commented Feb 5, 2020 • edited by msau42 Loading

k8s-ci-robot commented Feb 5, 2020

Madhu-1 commented Feb 5, 2020

jsafrane commented Feb 5, 2020

jsafrane Feb 5, 2020

Choose a reason for hiding this comment

Madhu-1 Feb 5, 2020

Choose a reason for hiding this comment

msau42 Apr 2, 2020

Choose a reason for hiding this comment

msau42 Apr 2, 2020

Choose a reason for hiding this comment

msau42 Apr 2, 2020

Choose a reason for hiding this comment

msau42 Apr 2, 2020

Choose a reason for hiding this comment

Madhu-1 commented Apr 3, 2020

msau42 commented Apr 3, 2020

k8s-ci-robot commented Apr 3, 2020

msau42 commented Apr 10, 2020

msau42 commented Apr 15, 2020

Madhu-1 commented Feb 5, 2020 •

edited by msau42

Loading