Allow plugin to return OK if NodeUnpublishVolume cannot find a volume #433
Comments
timoreimann
added a commit
to timoreimann/csi-test
that referenced
this issue
May 20, 2020
The "should fail when the volume is missing" test for NodeUnpublishVolume verifies that a NotFound error occurs when the endpoint is invoked for a missing volume. However, this expectation really only holds for volume types that are locally attached, such as local disks or iscsi. Specifically, it would not hold for network-attached storage where it is fine to return Ok if the mount point does not exist. This change introduces a flag to gate execution of the test, defaulting it not running since that was the behavior we have had before. Once container-storage-interface/spec#433 is addressed, we can update the tests acccordingly.
timoreimann
added a commit
to timoreimann/csi-test
that referenced
this issue
May 20, 2020
The "should fail when the volume is missing" test for NodeUnpublishVolume verifies that a NotFound error occurs when the endpoint is invoked for a missing volume. However, this expectation really only holds for volume types that are locally attached, such as local disks or iscsi. Specifically, it would not hold for network-attached storage where it is fine to return Ok if the mount point does not exist. container-storage-interface/spec#433 was filed to improve the spec in this regard. For now we remove the test as it seems to block more people than it helps.
|
Would love to hear what folks think of this proposal. Thanks in advance. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In #375, the spec was changed to allow plugins to return OK during
ControllerPublishVolumeif a volume is not found, thereby solving #373. A similar situation can arise forNodeUnpublishVolumewhen, say, a previously mounted volume has already been unmounted out-of-band, and the plugin decides that there simply is no more work to do. For such cases, it should not be okay to return OK fromNodeUnpublishVolumeas well.I propose we carry over the corresponding amendments done in the context of
ControllerPublishVolumetoNodeUnpublishVolume. I'd be happy to carry out the spec update if this request finds consent.This issue was motivated by a discussion that happened on kubernetes-csi/csi-test#258.
/cc @pohly @msau42 @jsafrane @maennchen
The text was updated successfully, but these errors were encountered: