[cinder-csi-plugin] reject non-multiattach publish on another node#3115
[cinder-csi-plugin] reject non-multiattach publish on another node#3115gnom4ik wants to merge 1 commit into
Conversation
|
|
|
Welcome @gnom4ik! |
|
Hi @gnom4ik. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
This PR has local unit test coverage and was also verified against a real OpenStack-backed Kubernetes cluster. Tested:
Manual verification:
Could a Kubernetes org member please run /ok-to-test? |
What this PR does / why we need it:
This fixes non-multiattach Cinder volume publish behavior when the volume is already attached to a different node.
Before this change,
AttachVolumetreated an existing attachment as idempotent only when it was already attached to the requested instance. For a non-multiattach volume attached to another instance, the driver still attempted a new Nova volume attach. In some OpenStack environments this can leave the volume with the oldattachedentry and an additionalattachingentry for the new node.This change makes the driver return
FailedPreconditionbefore calling Nova attach when a non-multiattach volume already has an attachment to another node.ControllerPublishVolumenow preserves that gRPC status code instead of wrapping it asInternal.Which issue this PR fixes(if applicable):
None
Special notes for reviewers:
Tested against a real OpenStack-backed Kubernetes cluster (
training) with a patchedcinder-csi-plugincontroller image.Verification scenario:
VolumeAttachmentfor the same PV targeting another worker node.FailedPrecondition.attachingattachment.Local tests:
go test ./pkg/csi/cinder/openstackgo test ./pkg/csi/cinder -run 'TestControllerPublishVolume|TestControllerPublishVolumePreservesFailedPrecondition'Note:
go test ./pkg/csi/cinder/...was not fully runnable on macOS arm64 because an existing node expansion test depends on block-device functionality that is not implemented for Darwin.Release note: