Skip to content
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

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

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: 11 additions & 5 deletions pkg/mounter/safe_mounter_host_process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 11 additions & 1 deletion pkg/mounter/safe_mounter_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
21 changes: 17 additions & 4 deletions pkg/os/smb/smb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion pkg/os/smb/smb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
Loading