Skip to content

Commit bde10b8

Browse files
committed
revert: GetRemoteServerFromTarget on Windows cache optimization
fix
1 parent 39e794e commit bde10b8

File tree

5 files changed

+12
-53
lines changed

5 files changed

+12
-53
lines changed

pkg/mounter/safe_mounter_host_process_windows.go

+5-11
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,16 @@ import (
3131

3232
"sigs.k8s.io/azurefile-csi-driver/pkg/os/filesystem"
3333
"sigs.k8s.io/azurefile-csi-driver/pkg/os/smb"
34-
35-
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
3634
)
3735

3836
var driverGlobalMountPath = "C:\\var\\lib\\kubelet\\plugins\\kubernetes.io\\csi\\file.csi.azure.com"
3937

4038
var _ CSIProxyMounter = &winMounter{}
4139

42-
type winMounter struct {
43-
volStatsCache azcache.Resource
44-
}
40+
type winMounter struct{}
4541

46-
func NewWinMounter(cache azcache.Resource) *winMounter {
47-
return &winMounter{
48-
volStatsCache: cache,
49-
}
42+
func NewWinMounter() *winMounter {
43+
return &winMounter{}
5044
}
5145

5246
func (mounter *winMounter) SMBMount(source, target, fsType string, mountOptions, sensitiveMountOptions []string) error {
@@ -137,10 +131,10 @@ func (mounter *winMounter) Rmdir(path string) error {
137131
// Unmount - Removes the directory - equivalent to unmount on Linux.
138132
func (mounter *winMounter) Unmount(target string) error {
139133
target = normalizeWindowsPath(target)
140-
remoteServer, err := smb.GetRemoteServerFromTarget(target, mounter.volStatsCache)
134+
remoteServer, err := smb.GetRemoteServerFromTarget(target)
141135
if err == nil {
142136
klog.V(2).Infof("remote server path: %s, local path: %s", remoteServer, target)
143-
if hasDupSMBMount, err := smb.CheckForDuplicateSMBMounts(driverGlobalMountPath, target, remoteServer, mounter.volStatsCache); err == nil {
137+
if hasDupSMBMount, err := smb.CheckForDuplicateSMBMounts(driverGlobalMountPath, target, remoteServer); err == nil {
144138
if !hasDupSMBMount {
145139
if err := smb.RemoveSmbGlobalMapping(remoteServer); err != nil {
146140
klog.Errorf("RemoveSmbGlobalMapping(%s) failed with %v", target, err)

pkg/mounter/safe_mounter_unix_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
)
2424

2525
func TestNewSafeMounter(t *testing.T) {
26-
resp, err := NewSafeMounter(false)
26+
resp, err := NewSafeMounter(true)
2727
assert.NotNil(t, resp)
2828
assert.Nil(t, err)
2929
}

pkg/mounter/safe_mounter_windows.go

+1-11
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"os"
2626
filepath "path/filepath"
2727
"strings"
28-
"time"
2928

3029
fs "github.com/kubernetes-csi/csi-proxy/client/api/filesystem/v1"
3130
fsclient "github.com/kubernetes-csi/csi-proxy/client/groups/filesystem/v1"
@@ -36,8 +35,6 @@ import (
3635
"k8s.io/klog/v2"
3736
mount "k8s.io/mount-utils"
3837
utilexec "k8s.io/utils/exec"
39-
40-
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
4138
)
4239

4340
// CSIProxyMounter extends the mount.Interface interface with CSI Proxy methods.
@@ -296,16 +293,9 @@ func NewCSIProxyMounter() (*csiProxyMounter, error) {
296293

297294
func NewSafeMounter(enableWindowsHostProcess bool) (*mount.SafeFormatAndMount, error) {
298295
if enableWindowsHostProcess {
299-
// initialize the cache for volume stats
300-
getter := func(key string) (interface{}, error) { return nil, nil }
301-
volStatsCache, err := azcache.NewTimedCache(10*time.Minute, getter, false)
302-
if err != nil {
303-
return nil, err
304-
}
305-
306296
klog.V(2).Infof("using windows host process mounter")
307297
return &mount.SafeFormatAndMount{
308-
Interface: NewWinMounter(volStatsCache),
298+
Interface: NewWinMounter(),
309299
Exec: utilexec.New(),
310300
}, nil
311301
}

pkg/os/smb/smb.go

+4-24
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,11 @@ import (
2121
"os"
2222
"path/filepath"
2323
"strings"
24-
"sync"
2524

2625
"k8s.io/klog/v2"
2726
"sigs.k8s.io/azurefile-csi-driver/pkg/util"
28-
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
2927
)
3028

31-
var getRemoteServerFromTargetMutex = &sync.Mutex{}
32-
3329
func IsSmbMapped(remotePath string) (bool, error) {
3430
cmdLine := `$(Get-SmbGlobalMapping -RemotePath $Env:smbremotepath -ErrorAction Stop).Status`
3531
cmdEnv := fmt.Sprintf("smbremotepath=%s", remotePath)
@@ -71,33 +67,17 @@ func RemoveSmbGlobalMapping(remotePath string) error {
7167
}
7268

7369
// GetRemoteServerFromTarget- gets the remote server path given a mount point, the function is recursive until it find the remote server or errors out
74-
func GetRemoteServerFromTarget(mount string, volStatsCache azcache.Resource) (string, error) {
75-
// use mutex to allow more cache hit
76-
getRemoteServerFromTargetMutex.Lock()
77-
defer getRemoteServerFromTargetMutex.Unlock()
78-
79-
cache, err := volStatsCache.Get(mount, azcache.CacheReadTypeDefault)
80-
if err != nil {
81-
return "", err
82-
}
83-
if cache != nil {
84-
remoteServer := cache.(string)
85-
klog.V(6).Infof("GetRemoteServerFromTarget(%s) cache hit: %s", mount, remoteServer)
86-
return remoteServer, nil
87-
}
70+
func GetRemoteServerFromTarget(mount string) (string, error) {
8871
target, err := os.Readlink(mount)
8972
klog.V(2).Infof("read link for mount %s, target: %s", mount, target)
9073
if err != nil || len(target) == 0 {
9174
return "", fmt.Errorf("error reading link for mount %s. target %s err: %v", mount, target, err)
9275
}
93-
remoteServer := strings.TrimSpace(target)
94-
// cache the remote server path
95-
volStatsCache.Set(mount, remoteServer)
96-
return remoteServer, nil
76+
return strings.TrimSpace(target), nil
9777
}
9878

9979
// CheckForDuplicateSMBMounts checks if there is any other SMB mount exists on the same remote server
100-
func CheckForDuplicateSMBMounts(dir, mount, remoteServer string, volStatsCache azcache.Resource) (bool, error) {
80+
func CheckForDuplicateSMBMounts(dir, mount, remoteServer string) (bool, error) {
10181
files, err := os.ReadDir(dir)
10282
if err != nil {
10383
return false, err
@@ -113,7 +93,7 @@ func CheckForDuplicateSMBMounts(dir, mount, remoteServer string, volStatsCache a
11393
fileInfo, err := os.Lstat(globalMountPath)
11494
// check if the file is a symlink, if yes, check if it is pointing to the same remote server
11595
if err == nil && fileInfo.Mode()&os.ModeSymlink != 0 {
116-
remoteServerPath, err := GetRemoteServerFromTarget(globalMountPath, volStatsCache)
96+
remoteServerPath, err := GetRemoteServerFromTarget(globalMountPath)
11797
klog.V(2).Infof("checking remote server path %s on local path %s", remoteServerPath, globalMountPath)
11898
if err == nil {
11999
if remoteServerPath == remoteServer {

pkg/os/smb/smb_test.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,9 @@ import (
2323
"fmt"
2424
"testing"
2525
"time"
26-
27-
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
2826
)
2927

3028
func TestCheckForDuplicateSMBMounts(t *testing.T) {
31-
getter := func(key string) (interface{}, error) { return nil, nil }
32-
volStatsCache, _ := azcache.NewTimedCache(10*time.Minute, getter, false)
33-
3429
tests := []struct {
3530
name string
3631
dir string
@@ -48,7 +43,7 @@ func TestCheckForDuplicateSMBMounts(t *testing.T) {
4843
}
4944

5045
for _, test := range tests {
51-
result, err := CheckForDuplicateSMBMounts(test.dir, test.mount, test.remoteServer, volStatsCache)
46+
result, err := CheckForDuplicateSMBMounts(test.dir, test.mount, test.remoteServer)
5247
if result != test.expectedResult {
5348
t.Errorf("Expected %v, got %v", test.expectedResult, result)
5449
}

0 commit comments

Comments
 (0)