Repository: kubernetes-sigs/azurefile-csi-driver
Pull request #473 (2020-11-14 05:27:17)

fix: add actimeo and fix NFS account type issue
<!--  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 bug
**What this PR does / why we need it**:
This PR fixed following issue:
 - set `actimeo =30` by default
 - default account type as `Premium_LRS` when using NFS protocol
 - driver crash issue on Windows, details: https://github.com/kubernetes-csi/csi-driver-smb/issues/150

**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**:
```
fix: add actimeo and fix NFS account type issue
```



Patch:
diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go
index 4968f6f96..aa9d748c5 100644
--- a/pkg/azurefile/azurefile.go
+++ b/pkg/azurefile/azurefile.go
@@ -54,9 +54,11 @@ const (
 	fileMode           = "file_mode"
 	dirMode            = "dir_mode"
 	vers               = "vers"
+	actimeo            = "actimeo"
 	defaultFileMode    = "0777"
 	defaultDirMode     = "0777"
 	defaultVers        = "3.0"
+	defaultActimeo     = "30"
 
 	// See https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-shares--directories--files--and-metadata#share-names
 	fileShareNameMinLength = 3
@@ -252,6 +254,7 @@ func appendDefaultMountOptions(mountOptions []string) []string {
 	fileModeFlag := false
 	dirModeFlag := false
 	versFlag := false
+	actimeoFlag := false
 
 	for _, mountOption := range mountOptions {
 		if strings.HasPrefix(mountOption, fileMode) {
@@ -263,6 +266,9 @@ func appendDefaultMountOptions(mountOptions []string) []string {
 		if strings.HasPrefix(mountOption, vers) {
 			versFlag = true
 		}
+		if strings.HasPrefix(mountOption, actimeo) {
+			actimeoFlag = true
+		}
 	}
 
 	allMountOptions := mountOptions
@@ -278,6 +284,10 @@ func appendDefaultMountOptions(mountOptions []string) []string {
 		allMountOptions = append(allMountOptions, fmt.Sprintf("%s=%s", vers, defaultVers))
 	}
 
+	if !actimeoFlag {
+		allMountOptions = append(allMountOptions, fmt.Sprintf("%s=%s", actimeo, defaultActimeo))
+	}
+
 	/* todo: looks like fsGroup is not included in CSI
 	if !gidFlag && fsGroup != nil {
 		allMountOptions = append(allMountOptions, fmt.Sprintf("%s=%d", gid, *fsGroup))
diff --git a/pkg/azurefile/controllerserver.go b/pkg/azurefile/controllerserver.go
index 62207665a..282a0fed4 100644
--- a/pkg/azurefile/controllerserver.go
+++ b/pkg/azurefile/controllerserver.go
@@ -120,6 +120,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	if fsType == nfs || protocol == nfs {
 		protocol = nfs
 		enableHTTPSTrafficOnly = false
+		// default to Premium_LRS
+		sku = string(storage.PremiumLRS)
 		shareProtocol = storage.NFS
 		// NFS protocol does not need account key
 		storeAccountKey = storeAccountKeyFalse
diff --git a/pkg/mounter/safe_mounter_windows.go b/pkg/mounter/safe_mounter_windows.go
index 5ae9f14fc..ca34f18d5 100644
--- a/pkg/mounter/safe_mounter_windows.go
+++ b/pkg/mounter/safe_mounter_windows.go
@@ -207,6 +207,9 @@ func (mounter *CSIProxyMounter) ExistsPath(path string) (bool, error) {
 		&fsv1alpha1.PathExistsRequest{
 			Path: normalizeWindowsPath(path),
 		})
+	if err != nil {
+		return false, err
+	}
 	return isExistsResponse.Exists, err
 }
 


Test patch:
diff --git a/pkg/azurefile/azurefile_test.go b/pkg/azurefile/azurefile_test.go
index 10a30c912..404f67b54 100644
--- a/pkg/azurefile/azurefile_test.go
+++ b/pkg/azurefile/azurefile_test.go
@@ -68,30 +68,45 @@ func TestAppendDefaultMountOptions(t *testing.T) {
 			options: []string{"dir_mode=0777"},
 			expected: []string{"dir_mode=0777",
 				fmt.Sprintf("%s=%s", fileMode, defaultFileMode),
-				fmt.Sprintf("%s=%s", vers, defaultVers)},
+				fmt.Sprintf("%s=%s", vers, defaultVers),
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
 		},
 		{
 			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", vers, defaultVers),
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
 		},
 		{
 			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", dirMode, defaultDirMode),
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
 		},
 		{
-			options: []string{""},
-			expected: []string{"", fmt.Sprintf("%s=%s",
-				fileMode, defaultFileMode),
+			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)},
+		},
+		{
+			options: []string{"actimeo=3"},
+			expected: []string{
+				"actimeo=3",
+				fmt.Sprintf("%s=%s", fileMode, defaultFileMode),
 				fmt.Sprintf("%s=%s", dirMode, defaultDirMode),
 				fmt.Sprintf("%s=%s", vers, defaultVers)},
 		},
 		{
-			options:  []string{"file_mode=0777", "dir_mode=0777"},
-			expected: []string{"file_mode=0777", "dir_mode=0777", fmt.Sprintf("%s=%s", vers, defaultVers)},
+			options: []string{""},
+			expected: []string{"", fmt.Sprintf("%s=%s",
+				fileMode, defaultFileMode),
+				fmt.Sprintf("%s=%s", dirMode, defaultDirMode),
+				fmt.Sprintf("%s=%s", vers, defaultVers),
+				fmt.Sprintf("%s=%s", actimeo, defaultActimeo)},
 		},
 	}