Skip to content
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

Fail when requested volume does not exist #56

Merged
merged 3 commits into from
Mar 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pkg/azuredisk/azuredisk.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,11 @@ func getDiskName(diskURI string) (string, error) {
}
return matches[1], nil
}

func getResourceGroupFromDiskURI(diskURI string) (string, error) {
fields := strings.Split(diskURI, "/")
if len(fields) != 9 || fields[3] != "resourceGroups" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you also copy the test code of TestGetResourceGroupFromDiskURI? thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

return "", fmt.Errorf("invalid disk URI: %s", diskURI)
}
return fields[4], nil
}
38 changes: 38 additions & 0 deletions pkg/azuredisk/azuredisk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"fmt"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
)

func TestIsManagedDisk(t *testing.T) {
Expand Down Expand Up @@ -91,3 +93,39 @@ func TestGetDiskName(t *testing.T) {
}
}
}

func TestGetResourceGroupFromDiskURI(t *testing.T) {
tests := []struct {
diskURL string
expectedResult string
expectError bool
}{
{
diskURL: "/subscriptions/4be8920b-2978-43d7-axyz-04d8549c1d05/resourceGroups/azure-k8s1102/providers/Microsoft.Compute/disks/andy-mghyb1102-dynamic-pvc-f7f014c9-49f4-11e8-ab5c-000d3af7b38e",
expectedResult: "azure-k8s1102",
expectError: false,
},
{
diskURL: "/4be8920b-2978-43d7-axyz-04d8549c1d05/resourceGroups/azure-k8s1102/providers/Microsoft.Compute/disks/andy-mghyb1102-dynamic-pvc-f7f014c9-49f4-11e8-ab5c-000d3af7b38e",
expectedResult: "",
expectError: true,
},
{
diskURL: "",
expectedResult: "",
expectError: true,
},
}

for _, test := range tests {
result, err := getResourceGroupFromDiskURI(test.diskURL)
assert.Equal(t, result, test.expectedResult, "Expect result not equal with getResourceGroupFromDiskURI(%s) return: %q, expected: %q",
test.diskURL, result, test.expectedResult)

if test.expectError {
assert.NotNil(t, err, "Expect error during getResourceGroupFromDiskURI(%s)", test.diskURL)
} else {
assert.Nil(t, err, "Expect error is nil during getResourceGroupFromDiskURI(%s)", test.diskURL)
}
}
}
18 changes: 17 additions & 1 deletion pkg/azuredisk/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,29 @@ func (d *Driver) ControllerUnpublishVolume(ctx context.Context, req *csi.Control

// ValidateVolumeCapabilities return the capabilities of the volume
func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.ValidateVolumeCapabilitiesRequest) (*csi.ValidateVolumeCapabilitiesResponse, error) {
if len(req.GetVolumeId()) == 0 {
diskURI := req.GetVolumeId()
if len(diskURI) == 0 {
return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request")
}
if req.GetVolumeCapabilities() == nil {
return nil, status.Error(codes.InvalidArgument, "Volume capabilities missing in request")
}

diskName, err := getDiskName(diskURI)
if err != nil {
return nil, status.Error(codes.NotFound, "Volume not found")
}

resourceGroup, err := getResourceGroupFromDiskURI(diskURI)
if err != nil {
return nil, status.Error(codes.NotFound, "Volume not found")
}

_, err = d.cloud.DisksClient.Get(ctx, resourceGroup, diskName)
if err != nil {
return nil, status.Error(codes.NotFound, "Volume not found")
}

for _, cap := range req.VolumeCapabilities {
if cap.GetAccessMode().GetMode() != csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER {
return &csi.ValidateVolumeCapabilitiesResponse{Message: ""}, nil
Expand Down