Merge pull request #1937 from k8s-infra-cherrypick-robot/cherry-pick-1934-to-release-1.29

[release-1.29] fix: refine GetRemoteServerFromTarget on Windows with cache optimization

Author: andyzhangx

pkg/mounter/safe_mounter_host_process_windows.go

@@ -31,16 +31,22 @@ import (
 
 	"sigs.k8s.io/azurefile-csi-driver/pkg/os/filesystem"
 	"sigs.k8s.io/azurefile-csi-driver/pkg/os/smb"
+
+	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 )
 
 var driverGlobalMountPath = "C:\\var\\lib\\kubelet\\plugins\\kubernetes.io\\csi\\file.csi.azure.com"
 
 var _ CSIProxyMounter = &winMounter{}
 
-type winMounter struct{}
+type winMounter struct {
+	volStatsCache azcache.Resource
+}
 
-func NewWinMounter() *winMounter {
-	return &winMounter{}
+func NewWinMounter(cache azcache.Resource) *winMounter {
+	return &winMounter{
+		volStatsCache: cache,
+	}
 }
 
 func (mounter *winMounter) SMBMount(source, target, fsType string, mountOptions, sensitiveMountOptions []string) error {
@@ -131,10 +137,10 @@ func (mounter *winMounter) Rmdir(path string) error {
 // Unmount - Removes the directory - equivalent to unmount on Linux.
 func (mounter *winMounter) Unmount(target string) error {
 	target = normalizeWindowsPath(target)
-	remoteServer, err := smb.GetRemoteServerFromTarget(target)
+	remoteServer, err := smb.GetRemoteServerFromTarget(target, mounter.volStatsCache)
 	if err == nil {
 		klog.V(2).Infof("remote server path: %s, local path: %s", remoteServer, target)
-		if hasDupSMBMount, err := smb.CheckForDuplicateSMBMounts(driverGlobalMountPath, target, remoteServer); err == nil {
+		if hasDupSMBMount, err := smb.CheckForDuplicateSMBMounts(driverGlobalMountPath, target, remoteServer, mounter.volStatsCache); err == nil {
 			if !hasDupSMBMount {
 				remoteServer = strings.Replace(remoteServer, "UNC\\", "\\\\", 1)
 				if err := smb.RemoveSmbGlobalMapping(remoteServer); err != nil {

pkg/mounter/safe_mounter_windows.go

@@ -25,6 +25,7 @@ import (
 	"os"
 	filepath "path/filepath"
 	"strings"
+	"time"
 
 	fs "github.com/kubernetes-csi/csi-proxy/client/api/filesystem/v1"
 	fsclient "github.com/kubernetes-csi/csi-proxy/client/groups/filesystem/v1"
@@ -35,6 +36,8 @@ import (
 	"k8s.io/klog/v2"
 	mount "k8s.io/mount-utils"
 	utilexec "k8s.io/utils/exec"
+
+	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 )
 
 // CSIProxyMounter extends the mount.Interface interface with CSI Proxy methods.
@@ -293,9 +296,16 @@ func NewCSIProxyMounter() (*csiProxyMounter, error) {
 
 func NewSafeMounter(enableWindowsHostProcess bool) (*mount.SafeFormatAndMount, error) {
 	if enableWindowsHostProcess {
+		// initialize the cache for volume stats
+		getter := func(key string) (interface{}, error) { return nil, nil }
+		volStatsCache, err := azcache.NewTimedCache(10*time.Minute, getter, false)
+		if err != nil {
+			return nil, err
+		}
+
 		klog.V(2).Infof("using windows host process mounter")
 		return &mount.SafeFormatAndMount{
-			Interface: NewWinMounter(),
+			Interface: NewWinMounter(volStatsCache),
 			Exec:      utilexec.New(),
 		}, nil
 	}

pkg/os/smb/smb.go

@@ -24,6 +24,7 @@ import (
 
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/azurefile-csi-driver/pkg/util"
+	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 )
 
 func IsSmbMapped(remotePath string) (bool, error) {
@@ -67,17 +68,29 @@ func RemoveSmbGlobalMapping(remotePath string) error {
 }
 
 // GetRemoteServerFromTarget- gets the remote server path given a mount point, the function is recursive until it find the remote server or errors out
-func GetRemoteServerFromTarget(mount string) (string, error) {
+func GetRemoteServerFromTarget(mount string, volStatsCache azcache.Resource) (string, error) {
+	cache, err := volStatsCache.Get(mount, azcache.CacheReadTypeDefault)
+	if err != nil {
+		return "", err
+	}
+	if cache != nil {
+		remoteServer := cache.(string)
+		klog.V(6).Infof("GetRemoteServerFromTarget(%s) cache hit: %s", mount, remoteServer)
+		return remoteServer, nil
+	}
 	cmd := "(Get-Item -Path $Env:mount).Target"
 	out, err := util.RunPowershellCmd(cmd, fmt.Sprintf("mount=%s", mount))
 	if err != nil || len(out) == 0 {
 		return "", fmt.Errorf("error getting volume from mount. cmd: %s, output: %s, error: %v", cmd, string(out), err)
 	}
-	return strings.TrimSpace(string(out)), nil
+	remoteServer := strings.TrimSpace(string(out))
+	// cache the remote server path
+	volStatsCache.Set(mount, remoteServer)
+	return remoteServer, nil
 }
 
 // CheckForDuplicateSMBMounts checks if there is any other SMB mount exists on the same remote server
-func CheckForDuplicateSMBMounts(dir, mount, remoteServer string) (bool, error) {
+func CheckForDuplicateSMBMounts(dir, mount, remoteServer string, volStatsCache azcache.Resource) (bool, error) {
 	files, err := os.ReadDir(dir)
 	if err != nil {
 		return false, err
@@ -93,7 +106,7 @@ func CheckForDuplicateSMBMounts(dir, mount, remoteServer string) (bool, error) {
 				fileInfo, err := os.Lstat(globalMountPath)
 				// check if the file is a symlink, if yes, check if it is pointing to the same remote server
 				if err == nil && fileInfo.Mode()&os.ModeSymlink != 0 {
-					remoteServerPath, err := GetRemoteServerFromTarget(globalMountPath)
+					remoteServerPath, err := GetRemoteServerFromTarget(globalMountPath, volStatsCache)
 					klog.V(2).Infof("checking remote server path %s on local path %s", remoteServerPath, globalMountPath)
 					if err == nil {
 						if remoteServerPath == remoteServer {

pkg/os/smb/smb_test.go

@@ -22,9 +22,15 @@ package smb
 import (
 	"fmt"
 	"testing"
+	"time"
+
+	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 )
 
 func TestCheckForDuplicateSMBMounts(t *testing.T) {
+	getter := func(key string) (interface{}, error) { return nil, nil }
+	volStatsCache, _ := azcache.NewTimedCache(10*time.Minute, getter, false)
+
 	tests := []struct {
 		name           string
 		dir            string
@@ -42,7 +48,7 @@ func TestCheckForDuplicateSMBMounts(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		result, err := CheckForDuplicateSMBMounts(test.dir, test.mount, test.remoteServer)
+		result, err := CheckForDuplicateSMBMounts(test.dir, test.mount, test.remoteServer, volStatsCache)
 		if result != test.expectedResult {
 			t.Errorf("Expected %v, got %v", test.expectedResult, result)
 		}