Merge pull request #2185 from kubernetes-sigs/revert-gettarget-cache

revert: GetRemoteServerFromTarget on Windows cache optimization

pkg/mounter/safe_mounter_host_process_windows.go

@@ -31,22 +31,16 @@ 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 {
-	volStatsCache azcache.Resource
-}
+type winMounter struct{}
 
-func NewWinMounter(cache azcache.Resource) *winMounter {
-	return &winMounter{
-		volStatsCache: cache,
-	}
+func NewWinMounter() *winMounter {
+	return &winMounter{}
 }
 
 func (mounter *winMounter) SMBMount(source, target, fsType string, mountOptions, sensitiveMountOptions []string) error {
@@ -137,10 +131,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, mounter.volStatsCache)
+	remoteServer, err := smb.GetRemoteServerFromTarget(target)
 	if err == nil {
 		klog.V(2).Infof("remote server path: %s, local path: %s", remoteServer, target)
-		if hasDupSMBMount, err := smb.CheckForDuplicateSMBMounts(driverGlobalMountPath, target, remoteServer, mounter.volStatsCache); err == nil {
+		if hasDupSMBMount, err := smb.CheckForDuplicateSMBMounts(driverGlobalMountPath, target, remoteServer); err == nil {
 			if !hasDupSMBMount {
 				if err := smb.RemoveSmbGlobalMapping(remoteServer); err != nil {
 					klog.Errorf("RemoveSmbGlobalMapping(%s) failed with %v", target, err)

pkg/mounter/safe_mounter_unix_test.go

@@ -23,7 +23,7 @@ import (
 )
 
 func TestNewSafeMounter(t *testing.T) {
-	resp, err := NewSafeMounter(false)
+	resp, err := NewSafeMounter(true)
 	assert.NotNil(t, resp)
 	assert.Nil(t, err)
 }

pkg/mounter/safe_mounter_windows.go

@@ -25,7 +25,6 @@ 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"
@@ -36,8 +35,6 @@ 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.
@@ -296,16 +293,9 @@ 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(volStatsCache),
+			Interface: NewWinMounter(),
 			Exec:      utilexec.New(),
 		}, nil
 	}

pkg/os/smb/smb.go

@@ -21,15 +21,11 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"sync"
 
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/azurefile-csi-driver/pkg/util"
-	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 )
 
-var getRemoteServerFromTargetMutex = &sync.Mutex{}
-
 func IsSmbMapped(remotePath string) (bool, error) {
 	cmdLine := `$(Get-SmbGlobalMapping -RemotePath $Env:smbremotepath -ErrorAction Stop).Status`
 	cmdEnv := fmt.Sprintf("smbremotepath=%s", remotePath)
@@ -71,33 +67,17 @@ 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, volStatsCache azcache.Resource) (string, error) {
-	// use mutex to allow more cache hit
-	getRemoteServerFromTargetMutex.Lock()
-	defer getRemoteServerFromTargetMutex.Unlock()
-
-	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
-	}
+func GetRemoteServerFromTarget(mount string) (string, error) {
 	target, err := os.Readlink(mount)
 	klog.V(2).Infof("read link for mount %s, target: %s", mount, target)
 	if err != nil || len(target) == 0 {
 		return "", fmt.Errorf("error reading link for mount %s. target %s err: %v", mount, target, err)
 	}
-	remoteServer := strings.TrimSpace(target)
-	// cache the remote server path
-	volStatsCache.Set(mount, remoteServer)
-	return remoteServer, nil
+	return strings.TrimSpace(target), nil
 }
 
 // CheckForDuplicateSMBMounts checks if there is any other SMB mount exists on the same remote server
-func CheckForDuplicateSMBMounts(dir, mount, remoteServer string, volStatsCache azcache.Resource) (bool, error) {
+func CheckForDuplicateSMBMounts(dir, mount, remoteServer string) (bool, error) {
 	files, err := os.ReadDir(dir)
 	if err != nil {
 		return false, err
@@ -113,7 +93,7 @@ func CheckForDuplicateSMBMounts(dir, mount, remoteServer string, volStatsCache a
 				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, volStatsCache)
+					remoteServerPath, err := GetRemoteServerFromTarget(globalMountPath)
 					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,15 +22,9 @@ 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
@@ -48,7 +42,7 @@ func TestCheckForDuplicateSMBMounts(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		result, err := CheckForDuplicateSMBMounts(test.dir, test.mount, test.remoteServer, volStatsCache)
+		result, err := CheckForDuplicateSMBMounts(test.dir, test.mount, test.remoteServer)
 		if result != test.expectedResult {
 			t.Errorf("Expected %v, got %v", test.expectedResult, result)
 		}