Repository: kubernetes-sigs/azurefile-csi-driver
Pull request #487 (2020-12-05 12:00:54)

feat: add mfsymlinks mountOption as default
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind feature
**What this PR does / why we need it**:
feat: add mfsymlinks mountOption as default
https://docs.microsoft.com/en-us/azure/storage/files/storage-troubleshoot-linux-file-connection-problems#cannot-create-symbolic-links---ln-failed-to-create-symbolic-link-t-operation-not-supported

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Requirements**:
- [ ] uses [conventional commit messages](https://www.conventionalcommits.org/)
  <!-- Common commit types:
        build: Build 🏭
        chore: Maintenance 🔧
        ci: Continuous Integration 💜
        docs: Documentation 📘
        feat: Features 🌈
        fix: Bug Fixes 🐞
        perf: Performance Improvements 🚀
        refactor: Code Refactoring 💎
        revert: Revert Change ◀️
        style: Code Style 🎶
        security: Security Fix 🛡️
        test: Testing 💚 -->
- [ ] includes documentation
- [ ] adds unit tests
- [ ] tested upgrade from previous version

**Special notes for your reviewer**:


**Release note**:
```
feat: add mfsymlinks mountOption as default
```


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

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/azurefile-csi-driver/pull/487#" title="Author self-approved">andyzhangx</a>*

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).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

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

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

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":[]} -->
k8s-ci-robot: @andyzhangx: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-azurefile-csi-driver-verify | 8cad457ac31766b8e51701a42c3a54d705b5bfcd | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azurefile-csi-driver/487/pull-azurefile-csi-driver-verify/1335192424732954624) | `/test pull-azurefile-csi-driver-verify`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=azurefile-csi-driver&pr=487). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Aandyzhangx). 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/deploy/example/storageclass-azurefile-csi-mountoptions.yaml b/deploy/example/storageclass-azurefile-csi-mountoptions.yaml
deleted file mode 100644
index 0b650215e..000000000
--- a/deploy/example/storageclass-azurefile-csi-mountoptions.yaml
+++ /dev/null
@@ -1,19 +0,0 @@
----
-apiVersion: storage.k8s.io/v1
-kind: StorageClass
-metadata:
-  name: azurefile-csi
-provisioner: file.csi.azure.com
-parameters:
-  skuName: Standard_LRS  # available values: Standard_LRS, Standard_GRS, Standard_ZRS, Standard_RAGRS, Premium_LRS
-reclaimPolicy: Delete
-volumeBindingMode: Immediate
-allowVolumeExpansion: true
-mountOptions:
-  - dir_mode=0777
-  - file_mode=0777
-  - uid=0
-  - gid=0
-  - mfsymlinks
-  - cache=strict  # https://linux.die.net/man/8/mount.cifs
-  - nosharesock
diff --git a/deploy/example/storageclass-azurefile-csi.yaml b/deploy/example/storageclass-azurefile-csi.yaml
index 526c5db77..e5c8aa74f 100644
--- a/deploy/example/storageclass-azurefile-csi.yaml
+++ b/deploy/example/storageclass-azurefile-csi.yaml
@@ -9,3 +9,12 @@ parameters:
   skuName: Standard_LRS  # available values: Standard_LRS, Standard_GRS, Standard_ZRS, Standard_RAGRS, Premium_LRS
 reclaimPolicy: Delete
 volumeBindingMode: Immediate
+mountOptions:
+  - dir_mode=0777
+  - file_mode=0777
+  - uid=0
+  - gid=0
+  - mfsymlinks
+  - cache=strict  # https://linux.die.net/man/8/mount.cifs
+  - nosharesock  # reduce probability of reconnect race
+  - actimeo=30  # reduce latency for metadata-heavy workload
diff --git a/deploy/example/storageclass-azurefile-existing-share.yaml b/deploy/example/storageclass-azurefile-existing-share.yaml
index 34ff52f63..3d656b97e 100644
--- a/deploy/example/storageclass-azurefile-existing-share.yaml
+++ b/deploy/example/storageclass-azurefile-existing-share.yaml
@@ -10,3 +10,12 @@ parameters:
   shareName: SHARE_NAME
 reclaimPolicy: Delete
 volumeBindingMode: Immediate
+mountOptions:
+  - dir_mode=0777
+  - file_mode=0777
+  - uid=0
+  - gid=0
+  - mfsymlinks
+  - cache=strict  # https://linux.die.net/man/8/mount.cifs
+  - nosharesock  # reduce probability of reconnect race
+  - actimeo=30  # reduce latency for metadata-heavy workload
diff --git a/deploy/example/storageclass-azurefile-secret.yaml b/deploy/example/storageclass-azurefile-secret.yaml
index 08c407f39..87d110b92 100644
--- a/deploy/example/storageclass-azurefile-secret.yaml
+++ b/deploy/example/storageclass-azurefile-secret.yaml
@@ -12,3 +12,12 @@ parameters:
   csi.storage.k8s.io/node-stage-secret-namespace: default
   csi.storage.k8s.io/controller-expand-secret-name: azure-secret
   csi.storage.k8s.io/controller-expand-secret-namespace: default
+mountOptions:
+  - dir_mode=0777
+  - file_mode=0777
+  - uid=0
+  - gid=0
+  - mfsymlinks
+  - cache=strict  # https://linux.die.net/man/8/mount.cifs
+  - nosharesock  # reduce probability of reconnect race
+  - actimeo=30  # reduce latency for metadata-heavy workload
diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go
index aa9d748c5..7b7b75634 100644
--- a/pkg/azurefile/azurefile.go
+++ b/pkg/azurefile/azurefile.go
@@ -55,6 +55,7 @@ const (
 	dirMode            = "dir_mode"
 	vers               = "vers"
 	actimeo            = "actimeo"
+	mfsymlinks         = "mfsymlinks"
 	defaultFileMode    = "0777"
 	defaultDirMode     = "0777"
 	defaultVers        = "3.0"
@@ -255,6 +256,7 @@ func appendDefaultMountOptions(mountOptions []string) []string {
 	dirModeFlag := false
 	versFlag := false
 	actimeoFlag := false
+	mfsymlinksFlag := false
 
 	for _, mountOption := range mountOptions {
 		if strings.HasPrefix(mountOption, fileMode) {
@@ -269,6 +271,9 @@ func appendDefaultMountOptions(mountOptions []string) []string {
 		if strings.HasPrefix(mountOption, actimeo) {
 			actimeoFlag = true
 		}
+		if strings.HasPrefix(mountOption, mfsymlinks) {
+			mfsymlinksFlag = true
+		}
 	}
 
 	allMountOptions := mountOptions
@@ -288,6 +293,10 @@ func appendDefaultMountOptions(mountOptions []string) []string {
 		allMountOptions = append(allMountOptions, fmt.Sprintf("%s=%s", actimeo, defaultActimeo))
 	}
 
+	if !mfsymlinksFlag {
+		allMountOptions = append(allMountOptions, mfsymlinks)
+	}
+
 	/* todo: looks like fsGroup is not included in CSI
 	if !gidFlag && fsGroup != nil {
 		allMountOptions = append(allMountOptions, fmt.Sprintf("%s=%d", gid, *fsGroup))


Test patch:
diff --git a/pkg/azurefile/azurefile_test.go b/pkg/azurefile/azurefile_test.go
index 404f67b54..8201258b5 100644
--- a/pkg/azurefile/azurefile_test.go
+++ b/pkg/azurefile/azurefile_test.go
@@ -69,28 +69,36 @@ func TestAppendDefaultMountOptions(t *testing.T) {
 			expected: []string{"dir_mode=0777",
 				fmt.Sprintf("%s=%s", fileMode, defaultFileMode),
 				fmt.Sprintf("%s=%s", vers, defaultVers),
-				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo),
+				mfsymlinks,
+			},
 		},
 		{
 			options: []string{"file_mode=0777"},
 			expected: []string{"file_mode=0777",
 				fmt.Sprintf("%s=%s", dirMode, defaultDirMode),
 				fmt.Sprintf("%s=%s", vers, defaultVers),
-				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo),
+				mfsymlinks,
+			},
 		},
 		{
 			options: []string{"vers=2.1"},
 			expected: []string{"vers=2.1",
 				fmt.Sprintf("%s=%s", fileMode, defaultFileMode),
 				fmt.Sprintf("%s=%s", dirMode, defaultDirMode),
-				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo),
+				mfsymlinks,
+			},
 		},
 		{
 			options: []string{"file_mode=0777", "dir_mode=0777"},
 			expected: []string{
 				"file_mode=0777", "dir_mode=0777",
 				fmt.Sprintf("%s=%s", vers, defaultVers),
-				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo),
+				mfsymlinks,
+			},
 		},
 		{
 			options: []string{"actimeo=3"},
@@ -98,7 +106,19 @@ func TestAppendDefaultMountOptions(t *testing.T) {
 				"actimeo=3",
 				fmt.Sprintf("%s=%s", fileMode, defaultFileMode),
 				fmt.Sprintf("%s=%s", dirMode, defaultDirMode),
-				fmt.Sprintf("%s=%s", vers, defaultVers)},
+				fmt.Sprintf("%s=%s", vers, defaultVers),
+				mfsymlinks,
+			},
+		},
+		{
+			options: []string{mfsymlinks},
+			expected: []string{
+				mfsymlinks,
+				fmt.Sprintf("%s=%s", fileMode, defaultFileMode),
+				fmt.Sprintf("%s=%s", dirMode, defaultDirMode),
+				fmt.Sprintf("%s=%s", vers, defaultVers),
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo),
+			},
 		},
 		{
 			options: []string{""},
@@ -106,7 +126,9 @@ func TestAppendDefaultMountOptions(t *testing.T) {
 				fileMode, defaultFileMode),
 				fmt.Sprintf("%s=%s", dirMode, defaultDirMode),
 				fmt.Sprintf("%s=%s", vers, defaultVers),
-				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo),
+				mfsymlinks,
+			},
 		},
 	}