Skip to content

revert: GetRemoteServerFromTarget on Windows cache optimization #2185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions pkg/mounter/safe_mounter_host_process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/mounter/safe_mounter_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
12 changes: 1 addition & 11 deletions pkg/mounter/safe_mounter_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
28 changes: 4 additions & 24 deletions pkg/os/smb/smb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
8 changes: 1 addition & 7 deletions pkg/os/smb/smb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
Loading