Repository: kubernetes-sigs/azurefile-csi-driver
Pull request #483 (2020-12-02 07:41:41)

fix: return error for creating raw block volume(not supported)
**What type of PR is this?**
/kind bug

**What this PR does / why we need it**:
This fixes an issue that when we call the CreateVolume with block volume capability, the controller service should validate that and return an error.

**Which issue(s) this PR fixes**:
Fixes # https://github.com/kubernetes-sigs/azurefile-csi-driver/issues/482


**Release note**:
```
FIX: Return error for creating raw block volume(not supported)
```

/cc @msau42 
/assign @andyzhangx 

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/azurefile-csi-driver/pull/483#" title="Author self-approved">Jiawei0227</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **feiskyer** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @feiskyer` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fazurefile-csi-driver).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["feiskyer"]} -->
k8s-ci-robot: @Jiawei0227: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-azurefile-csi-driver-integration | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-integration/1334040029361606656) | `/test pull-azurefile-csi-driver-integration`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=azurefile-csi-driver&pr=483). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AJiawei0227). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/azurefile-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @Jiawei0227: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-azurefile-csi-driver-integration | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-integration/1334040029361606656) | `/test pull-azurefile-csi-driver-integration`
pull-azurefile-csi-driver-e2e-windows | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-e2e-windows/1334040029428715520) | `/test pull-azurefile-csi-driver-e2e-windows`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=azurefile-csi-driver&pr=483). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AJiawei0227). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/azurefile-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
andyzhangx: /retest
k8s-ci-robot: @Jiawei0227: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-azurefile-csi-driver-e2e-windows | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-e2e-windows/1334040029428715520) | `/test pull-azurefile-csi-driver-e2e-windows`
pull-azurefile-csi-driver-integration | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-integration/1334125802723217408) | `/test pull-azurefile-csi-driver-integration`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=azurefile-csi-driver&pr=483). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AJiawei0227). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/azurefile-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @Jiawei0227: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-azurefile-csi-driver-integration | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-integration/1334125802723217408) | `/test pull-azurefile-csi-driver-integration`
pull-azurefile-csi-driver-e2e-windows | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-e2e-windows/1334125802744188928) | `/test pull-azurefile-csi-driver-e2e-windows`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=azurefile-csi-driver&pr=483). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AJiawei0227). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/azurefile-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/azurefile-csi-driver/pull/483#" title="Author self-approved">Jiawei0227</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **andyzhangx** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @andyzhangx` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fazurefile-csi-driver).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["andyzhangx"]} -->
k8s-ci-robot: @Jiawei0227: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-azurefile-csi-driver-e2e-windows | fc693dc353b7b57e34da7b406260c34f027c841f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-e2e-windows/1334125802744188928) | `/test pull-azurefile-csi-driver-e2e-windows`
pull-azurefile-csi-driver-integration | 045afc7a33429768e8cb7fdcfdfa627f5f2f0290 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/483/pull-azurefile-csi-driver-integration/1334201536300978176) | `/test pull-azurefile-csi-driver-integration`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=azurefile-csi-driver&pr=483). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AJiawei0227). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/azurefile-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/pkg/azurefile/controllerserver.go b/pkg/azurefile/controllerserver.go
index 4cf347fa5..0676c2ea4 100644
--- a/pkg/azurefile/controllerserver.go
+++ b/pkg/azurefile/controllerserver.go
@@ -44,6 +44,26 @@ const (
 	azureFileCSIDriverName = "azurefile_csi_driver"
 )
 
+var (
+	volumeCaps = []csi.VolumeCapability_AccessMode{
+		{
+			Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+		},
+		{
+			Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY,
+		},
+		{
+			Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY,
+		},
+		{
+			Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_SINGLE_WRITER,
+		},
+		{
+			Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER,
+		},
+	}
+)
+
 // CreateVolume provisions an azure file
 func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {
 	if err := d.ValidateControllerServiceRequest(csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME); err != nil {
@@ -55,8 +75,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		return nil, status.Error(codes.InvalidArgument, "CreateVolume Name must be provided")
 	}
 	volumeCapabilities := req.GetVolumeCapabilities()
-	if len(volumeCapabilities) == 0 {
-		return nil, status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities must be provided")
+	if err := isValidVolumeCapabilities(volumeCapabilities); err != nil {
+		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("CreateVolume Volume capabilities not valid: %v", err))
 	}
 
 	capacityBytes := req.GetCapacityRange().GetRequiredBytes()
@@ -704,3 +724,28 @@ func (d *Driver) snapshotExists(ctx context.Context, sourceVolumeID, snapshotNam
 
 	return false, azfile.ShareItem{}, nil
 }
+
+// isValidVolumeCapabilities validates the given VolumeCapability array is valid
+func isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) error {
+	if len(volCaps) == 0 {
+		return fmt.Errorf("CreateVolume Volume capabilities must be provided")
+	}
+	hasSupport := func(cap *csi.VolumeCapability) error {
+		if blk := cap.GetBlock(); blk != nil {
+			return fmt.Errorf("driver only supports mount access type volume capability")
+		}
+		for _, c := range volumeCaps {
+			if c.GetMode() == cap.AccessMode.GetMode() {
+				return nil
+			}
+		}
+		return fmt.Errorf("driver does not support access mode %v", cap.AccessMode.GetMode())
+	}
+
+	for _, c := range volCaps {
+		if err := hasSupport(c); err != nil {
+			return err
+		}
+	}
+	return nil
+}


Test patch:
diff --git a/pkg/azurefile/controllerserver_test.go b/pkg/azurefile/controllerserver_test.go
index 269b004b7..3ddeb4f7d 100644
--- a/pkg/azurefile/controllerserver_test.go
+++ b/pkg/azurefile/controllerserver_test.go
@@ -125,7 +125,40 @@ func TestCreateVolume(t *testing.T) {
 						csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME,
 					})
 
-				expectedErr := status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities must be provided")
+				expectedErr := status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities not valid: CreateVolume Volume capabilities must be provided")
+				_, err := d.CreateVolume(ctx, req)
+				if !reflect.DeepEqual(err, expectedErr) {
+					t.Errorf("Unexpected error: %v", err)
+				}
+			},
+		},
+		{
+			name: "Invalid volume capabilities",
+			testFunc: func(t *testing.T) {
+				req := &csi.CreateVolumeRequest{
+					Name:          "random-vol-name-vol-cap-invalid",
+					CapacityRange: stdCapRange,
+					VolumeCapabilities: []*csi.VolumeCapability{
+						{
+							AccessType: &csi.VolumeCapability_Block{
+								Block: &csi.VolumeCapability_BlockVolume{},
+							},
+							AccessMode: &csi.VolumeCapability_AccessMode{
+								Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+							},
+						},
+					},
+					Parameters: nil,
+				}
+
+				ctx := context.Background()
+				d := NewFakeDriver()
+				d.AddControllerServiceCapabilities(
+					[]csi.ControllerServiceCapability_RPC_Type{
+						csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME,
+					})
+
+				expectedErr := status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities not valid: driver only supports mount access type volume capability")
 				_, err := d.CreateVolume(ctx, req)
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("Unexpected error: %v", err)
diff --git a/test/integration/run-test.sh b/test/integration/run-test.sh
index 3ffa77223..76ddaf466 100755
--- a/test/integration/run-test.sh
+++ b/test/integration/run-test.sh
@@ -62,22 +62,22 @@ fi
 
 # Begin to run CSI functions one by one
 echo 'Create volume test:'
-readonly value=$("$CSC_BIN" controller new --endpoint "$endpoint" --cap 1,block "$volname" --req-bytes 2147483648 --params "$params")
+readonly value=$("$CSC_BIN" controller new --endpoint "$endpoint" --cap 1,mount,cifs "$volname" --req-bytes 2147483648 --params "$params")
 sleep 15
 
 readonly volumeid=$(echo "$value" | awk '{print $1}' | sed 's/"//g')
 echo "Got volume id: $volumeid"
 
-"$CSC_BIN" controller validate-volume-capabilities --endpoint "$endpoint" --cap 1,block "$volumeid"
+"$CSC_BIN" controller validate-volume-capabilities --endpoint "$endpoint" --cap 1,mount,cifs "$volumeid"
 
 if [[ "$cloud" != 'AzureChinaCloud' ]]; then
   # azure file mount/unmount on travis VM does not work against AzureChinaCloud
   echo "stage volume test:"
-  "$CSC_BIN" node stage --endpoint "$endpoint" --cap 1,block --staging-target-path "$staging_target_path" "$volumeid"
+  "$CSC_BIN" node stage --endpoint "$endpoint" --cap 1,mount,cifs --staging-target-path "$staging_target_path" "$volumeid"
   sleep 2
 
   echo 'Mount volume test:'
-  "$CSC_BIN" node publish --endpoint "$endpoint" --cap 1,block --staging-target-path "$staging_target_path" --target-path "$target_path" "$volumeid"
+  "$CSC_BIN" node publish --endpoint "$endpoint" --cap 1,mount,cifs --staging-target-path "$staging_target_path" --target-path "$target_path" "$volumeid"
   sleep 2
 
   echo 'Unmount volume test:'