Repository: kubernetes-sigs/azurefile-csi-driver
Pull request #1765 (2024-03-19 07:17:30)

fix: mount stuck 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**:
fix: mount stuck issue

**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: mount stuck issue
```



Patch:
diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go
index 49e8d7085..994b48c91 100644
--- a/pkg/azurefile/azurefile.go
+++ b/pkg/azurefile/azurefile.go
@@ -1036,7 +1036,7 @@ func (d *Driver) copyFileShare(ctx context.Context, req *csi.CreateVolumeRequest
 			_, percent, _ := d.azcopy.GetAzcopyJob(dstFileShareName, authAzcopyEnv)
 			return fmt.Errorf("timeout waiting for copy blob container %s to %s complete, current copy percent: %s%%", srcFileShareName, dstFileShareName, percent)
 		}
-		copyErr := fileutil.WaitForExecCompletion(time.Duration(d.waitForAzCopyTimeoutMinutes)*time.Minute, execFuncWithAuth, timeoutFunc)
+		copyErr := fileutil.WaitUntilTimeout(time.Duration(d.waitForAzCopyTimeoutMinutes)*time.Minute, execFuncWithAuth, timeoutFunc)
 		if accountSASToken == "" && copyErr != nil && strings.Contains(copyErr.Error(), authorizationPermissionMismatch) {
 			klog.Warningf("azcopy list failed with AuthorizationPermissionMismatch error, should assign \"Storage File Data SMB Share Elevated Contributor\" role to controller identity, fall back to use sas token, original error: %v", copyErr)
 			d.azcopySasTokenCache.Set(accountName, "")
@@ -1052,7 +1052,7 @@ func (d *Driver) copyFileShare(ctx context.Context, req *csi.CreateVolumeRequest
 				}
 				return nil
 			}
-			copyErr = fileutil.WaitForExecCompletion(time.Duration(d.waitForAzCopyTimeoutMinutes)*time.Minute, execFuncWithSasToken, timeoutFunc)
+			copyErr = fileutil.WaitUntilTimeout(time.Duration(d.waitForAzCopyTimeoutMinutes)*time.Minute, execFuncWithSasToken, timeoutFunc)
 		}
 		if copyErr != nil {
 			klog.Warningf("CopyFileShare(%s, %s, %s) failed with error: %v", resourceGroupName, accountName, dstFileShareName, copyErr)
diff --git a/pkg/azurefile/nodeserver.go b/pkg/azurefile/nodeserver.go
index d6b2eab3b..ba710ddd4 100644
--- a/pkg/azurefile/nodeserver.go
+++ b/pkg/azurefile/nodeserver.go
@@ -27,7 +27,6 @@ import (
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/volume/util"
 
@@ -36,6 +35,7 @@ import (
 
 	"golang.org/x/net/context"
 
+	volumehelper "sigs.k8s.io/azurefile-csi-driver/pkg/util"
 	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 	"sigs.k8s.io/cloud-provider-azure/pkg/metrics"
 )
@@ -339,9 +339,11 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 		if err := prepareStagePath(cifsMountPath, d.mounter); err != nil {
 			return nil, status.Errorf(codes.Internal, "prepare stage path failed for %s with error: %v", cifsMountPath, err)
 		}
-		if err := wait.PollImmediate(1*time.Second, 2*time.Minute, func() (bool, error) {
-			return true, SMBMount(d.mounter, source, cifsMountPath, mountFsType, mountOptions, sensitiveMountOptions)
-		}); err != nil {
+		execFunc := func() error {
+			return SMBMount(d.mounter, source, cifsMountPath, mountFsType, mountOptions, sensitiveMountOptions)
+		}
+		timeoutFunc := func() error { return fmt.Errorf("time out") }
+		if err := volumehelper.WaitUntilTimeout(2*time.Minute, execFunc, timeoutFunc); err != nil {
 			var helpLinkMsg string
 			if d.appendMountErrorHelpLink {
 				helpLinkMsg = "\nPlease refer to http://aka.ms/filemounterror for possible causes and solutions for mount errors."
diff --git a/pkg/util/util.go b/pkg/util/util.go
index 199daa6c1..651e7e30a 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -213,8 +213,8 @@ type ExecFunc func() (err error)
 // TimeoutFunc returns output and error if an ExecFunc timeout
 type TimeoutFunc func() (err error)
 
-// WaitForExecCompletion waits for the exec function to complete or return timeout error
-func WaitForExecCompletion(timeout time.Duration, execFunc ExecFunc, timeoutFunc TimeoutFunc) error {
+// WaitUntilTimeout waits for the exec function to complete or return timeout error
+func WaitUntilTimeout(timeout time.Duration, execFunc ExecFunc, timeoutFunc TimeoutFunc) error {
 	// Create a channel to receive the result of the azcopy exec function
 	done := make(chan bool)
 	var err error


Test patch:
diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go
index 1417ca678..07b437276 100644
--- a/pkg/util/util_test.go
+++ b/pkg/util/util_test.go
@@ -262,7 +262,7 @@ func TestParseAzcopyJobShow(t *testing.T) {
 	}
 }
 
-func TestWaitForExecCompletion(t *testing.T) {
+func TestWaitUntilTimeout(t *testing.T) {
 	tests := []struct {
 		desc        string
 		timeout     time.Duration
@@ -307,7 +307,7 @@ func TestWaitForExecCompletion(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		err := WaitForExecCompletion(test.timeout, test.execFunc, test.timeoutFunc)
+		err := WaitUntilTimeout(test.timeout, test.execFunc, test.timeoutFunc)
 		if err != nil && (err.Error() != test.expectedErr.Error()) {
 			t.Errorf("unexpected error: %v, expected error: %v", err, test.expectedErr)
 		}