From e5e32be10a16d6d5007a3e3f617563c238d2aa2b Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Fri, 21 Jun 2024 09:11:31 +0000 Subject: [PATCH 1/2] fix: refine GetRemoteServerFromTarget on Windows with cache optimization fix gofmt --- .../safe_mounter_host_process_windows.go | 16 +++++++++----- pkg/mounter/safe_mounter_windows.go | 12 ++++++++++- pkg/os/smb/smb.go | 21 +++++++++++++++---- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/pkg/mounter/safe_mounter_host_process_windows.go b/pkg/mounter/safe_mounter_host_process_windows.go index e8d013f416..6bafb2d185 100644 --- a/pkg/mounter/safe_mounter_host_process_windows.go +++ b/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 { diff --git a/pkg/mounter/safe_mounter_windows.go b/pkg/mounter/safe_mounter_windows.go index 6c99c9a027..01bbedd009 100644 --- a/pkg/mounter/safe_mounter_windows.go +++ b/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 } diff --git a/pkg/os/smb/smb.go b/pkg/os/smb/smb.go index b2f0252998..ed094c02e7 100644 --- a/pkg/os/smb/smb.go +++ b/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 { From 5e4922584cb1ee7fd132d7977d59429d96d49090 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Fri, 21 Jun 2024 09:29:23 +0000 Subject: [PATCH 2/2] test: fix test --- pkg/os/smb/smb_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/os/smb/smb_test.go b/pkg/os/smb/smb_test.go index 98dc67406c..260477df19 100644 --- a/pkg/os/smb/smb_test.go +++ b/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) }