From 298995b47312babc295a923b49af43c563cb0738 Mon Sep 17 00:00:00 2001 From: Timo Reimann Date: Sun, 15 Dec 2019 09:32:23 +0100 Subject: [PATCH] Check volume exists during NodeUnpublishVolume The CSI spec mandates that we return NOT_FOUND if the volume isn't found [1]. Otherwise, the calling sidecar will go into an endless retry loop and block other operations on the volume. A test for this case should go into github.com/kubernetes-csi/csi-test but was yet missing. A PR has been submitted [2] and this change has been tested against it. As soon as upstream approves, we can follow up with another PR to incorporate an updated csi-sanity package. [1]: https://github.com/container-storage-interface/spec/blob/4731db0e0bc53238b93850f43ab05d9355df0fd9/spec.md#nodeunpublishvolume-errors [2]: https://github.com/kubernetes-csi/csi-test/pull/242 --- driver/node.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/driver/node.go b/driver/node.go index 0a430f2dc..55f5174a3 100644 --- a/driver/node.go +++ b/driver/node.go @@ -25,6 +25,7 @@ limitations under the License. package driver import ( + "net/http" "context" "path/filepath" @@ -266,6 +267,15 @@ func (d *Driver) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublish }) ll.Info("node unpublish volume called") + // TODO(treimann): Add a test to https://github.com/kubernetes-csi/csi-test/blob/master/pkg/sanity/node.go + _, resp, err := d.storage.GetVolume(ctx, req.VolumeId) + if err != nil { + if resp != nil && resp.StatusCode == http.StatusNotFound { + return nil, status.Errorf(codes.NotFound, "volume %q not found", req.VolumeId) + } + return nil, status.Errorf(codes.Internal, "failed to get volume %q: %s", req.VolumeId, err) + } + mounted, err := d.mounter.IsMounted(req.TargetPath) if err != nil { return nil, err