Flag gate for topology and leader election

[verult]

/kind bug
/priority critical-urgent

Fixes #146 and #147

I'm not a fan of how the topology flag is set up, but I'm hitting some hurdles integrating the feature gate mechanism from k8s/k8s here (namely how to properly setup spf13/pflags). Going to try to resolve those issues

[verult]

Depends on #145 to resolve external-snapshotter dependency issue in Travis

[verult]

/assign @jsafrane @saad-ali

[davidz627]

Not sure it's necessary to add this whole new flag gating mechanism. Could we just have it as a normal command line flag or is there a compelling reason not to do that?

Currently v0.4.0 of the provisioner used with a topology supporting driver is not compatible with any Kubernetes cluster except for 1.12 with Alpha Topology support enabled so I would consider this high priority.

/cc @jingxu97
for tracking as this is blocking GCE PD CSI Driver Snapshots Support

[k8s-ci-robot]

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

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

Details

In response to this:

Not sure it's necessary to add this whole new flag gating mechanism. Could we just have it as a normal command line flag or is there a compelling reason not to do that?

Currently v0.4.0 of the provisioner used with a topology supporting driver is not compatible with any Kubernetes cluster except for 1.12 with Alpha Topology support enabled so I would consider this high priority.

/cc @jingxu97
for tracking as this is blocking GCE PD CSI Driver Snapshots Support

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.

[verult]

The simplest approach to adding a command-line flag would involve plumbing the flag parameter to every code path that checks the flag, which doesn't scale very well. We'd need a global source of truth for feature gates, and the library in k8s sets that up nicely so we don't have to write our own.

[verult]

Status update: PR #145 depends on external-snapshotter being tagged, which will be done today

[wongma7]

enableLeaderElection should default false to match external-attacher imo.

[pohly]

enableLeaderElection should default false to match external-attacher imo.

That, and it's also the right default for a StatefulSet (which is what many people and the example use) because then leader election isn't needed.

[verult]

@pohly what kind of configuration would be used if leader election were enabled?

[pohly]

Cheng Xing <notifications@github.com> writes:

@pohly what kind of configuration would be used if leader election were enabled?

I suppose the simplest approach would be a ReplicaSet, which starts a fixed number of pods.

[saad-ali]

Add to comment: enabling may require additional RBAC

[msau42]

Is this change ready to go?

[saad-ali]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saad-ali, verult

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:

• OWNERS [saad-ali]

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

[davidz627]

@verult can a new provisioner version be cut for this?

[verult]

v0.4.1 has been cut!