-
Notifications
You must be signed in to change notification settings - Fork 336
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
Don't process PVCs for in-tree volumes #350
Conversation
Request for provisioning of in-tree volume plugins that were migrated to CSI will always have CSI driver name in annStorageProvisioner.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane 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:
Approvers can indicate their approval by writing |
/assign @ddebroy @davidz627 |
The additionalProvisionerNames is still necessary to keep this check at https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/blob/master/controller/controller.go#L1234 happy for migration scenarios. Note that check in lib-external-provisioner depends on the provisioner set in storage class rather than annotations. Without populating additionalProvisionerNames, we end up with the following error message for migration scenarios.
We discussed this a bit at #341 (comment) I agree with the spirit of the PR. Maybe we can refactor https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/blob/master/controller/controller.go#L1234 to allow for this? |
87ff997
to
edff2d6
Compare
I see. I reworked the PR to still pass additional provisioners to the lib, but rejecting PVCs in |
edff2d6
to
38accee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
38accee
to
624e1d3
Compare
Good idea, added. |
/lgtm |
master: update release-tools
If I understand code in
ProvisionExt
, PVCs that require migrated in-tree provisioner in their StorageClass have theirvolume.beta.kubernetes.io/storage-provisioner
annotation set to CSI driver name.Therefore we do not need the provisioner library to listen for PVCs with in-tree provisioner at all and save some (little) CPU in unrelated PVC processing. I keep the check in
ProvisionExt
, just in case.@ddebroy, PTAL carefuly.
/kind cleanup
Does this PR introduce a user-facing change?: