Skip to content

Commit

Permalink
fix: remove SMBGlobalMapping on Windows node
Browse files Browse the repository at this point in the history
fix
  • Loading branch information
andyzhangx committed Apr 19, 2024
1 parent 55483a7 commit d42569b
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 19 deletions.
4 changes: 4 additions & 0 deletions pkg/azurefile/azure_common_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ func SMBMount(m *mount.SafeFormatAndMount, source, target, fsType string, option
return nil
}

func SMBUnmount(m *mount.SafeFormatAndMount, target string, _, _ bool) error {
return nil
}

func CleanupMountPoint(m *mount.SafeFormatAndMount, target string, extensiveMountCheck bool) error {
return nil
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/azurefile/azure_common_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ func SMBMount(m *mount.SafeFormatAndMount, source, target, fsType string, option
return m.MountSensitive(source, target, fsType, options, sensitiveMountOptions)
}

func SMBUnmount(m *mount.SafeFormatAndMount, target string, _, _ bool) error {
return mount.CleanupMountPoint(target, m.Interface, true /*extensiveMountPointCheck*/)
}

func CleanupMountPoint(m *mount.SafeFormatAndMount, target string, _ bool) error {
return mount.CleanupMountPoint(target, m.Interface, true /*extensiveMountPointCheck*/)
}
Expand Down
10 changes: 10 additions & 0 deletions pkg/azurefile/azure_common_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ func SMBMount(m *mount.SafeFormatAndMount, source, target, fsType string, mountO
return fmt.Errorf("could not cast to csi proxy class")
}

func SMBUnmount(m *mount.SafeFormatAndMount, target string, extensiveMountCheck, removeSMBMountOnWindows bool) error {
if proxy, ok := m.Interface.(mounter.CSIProxyMounter); ok {
if removeSMBMountOnWindows {
return proxy.Unmount(target)
}
return proxy.Rmdir(target)
}
return fmt.Errorf("could not cast to csi proxy class")
}

func CleanupMountPoint(m *mount.SafeFormatAndMount, target string, extensiveMountCheck bool) error {
if proxy, ok := m.Interface.(mounter.CSIProxyMounter); ok {
return proxy.Rmdir(target)
Expand Down
4 changes: 4 additions & 0 deletions pkg/azurefile/azurefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ type DriverOptions struct {
KubeAPIQPS float64
KubeAPIBurst int
EnableWindowsHostProcess bool
RemoveSMBMountOnWindows bool
AppendClosetimeoOption bool
AppendNoShareSockOption bool
AppendNoResvPortOption bool
Expand Down Expand Up @@ -250,6 +251,7 @@ type Driver struct {
kubeAPIQPS float64
kubeAPIBurst int
enableWindowsHostProcess bool
removeSMBMountOnWindows bool
appendClosetimeoOption bool
appendNoShareSockOption bool
appendNoResvPortOption bool
Expand Down Expand Up @@ -314,6 +316,8 @@ func NewDriver(options *DriverOptions) *Driver {
driver.kubeAPIQPS = options.KubeAPIQPS
driver.kubeAPIBurst = options.KubeAPIBurst
driver.enableWindowsHostProcess = options.EnableWindowsHostProcess
driver.removeSMBMountOnWindows = options.RemoveSMBMountOnWindows
driver.removeSMBMountOnWindows = options.RemoveSMBMountOnWindows
driver.appendClosetimeoOption = options.AppendClosetimeoOption
driver.appendNoShareSockOption = options.AppendNoShareSockOption
driver.appendNoResvPortOption = options.AppendNoResvPortOption
Expand Down
14 changes: 8 additions & 6 deletions pkg/azurefile/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,15 +421,17 @@ func (d *Driver) NodeUnstageVolume(_ context.Context, req *csi.NodeUnstageVolume
mc.ObserveOperationWithResult(isOperationSucceeded, VolumeID, volumeID)
}()

klog.V(2).Infof("NodeUnstageVolume: CleanupMountPoint volume %s on %s", volumeID, stagingTargetPath)
if err := CleanupMountPoint(d.mounter, stagingTargetPath, true /*extensiveMountPointCheck*/); err != nil {
klog.V(2).Infof("NodeUnstageVolume: unmount volume %s on %s", volumeID, stagingTargetPath)
if err := SMBUnmount(d.mounter, stagingTargetPath, true /*extensiveMountPointCheck*/, d.removeSMBMountOnWindows); err != nil {
return nil, status.Errorf(codes.Internal, "failed to unmount staging target %s: %v", stagingTargetPath, err)
}

targetPath := filepath.Join(filepath.Dir(stagingTargetPath), proxyMount)
klog.V(2).Infof("NodeUnstageVolume: CleanupMountPoint volume %s on %s", volumeID, targetPath)
if err := CleanupMountPoint(d.mounter, targetPath, false); err != nil {
return nil, status.Errorf(codes.Internal, "failed to unmount staging target %s: %v", targetPath, err)
if runtime.GOOS != "windows" {
targetPath := filepath.Join(filepath.Dir(stagingTargetPath), proxyMount)
klog.V(2).Infof("NodeUnstageVolume: CleanupMountPoint volume %s on %s", volumeID, targetPath)
if err := CleanupMountPoint(d.mounter, targetPath, false); err != nil {
return nil, status.Errorf(codes.Internal, "failed to unmount staging target %s: %v", targetPath, err)
}
}
klog.V(2).Infof("NodeUnstageVolume: unmount volume %s on %s successfully", volumeID, stagingTargetPath)

Expand Down
2 changes: 2 additions & 0 deletions pkg/azurefileplugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ var (
kubeAPIBurst = flag.Int("kube-api-burst", 50, "Burst to use while communicating with the kubernetes apiserver.")
appendMountErrorHelpLink = flag.Bool("append-mount-error-help-link", true, "Whether to include a link for help with mount errors when a mount error occurs.")
enableWindowsHostProcess = flag.Bool("enable-windows-host-process", false, "enable windows host process")
removeSMBMountOnWindows = flag.Bool("remove-smb-mount-on-windows", true, "remove smb global mapping on windows during unmount")
appendClosetimeoOption = flag.Bool("append-closetimeo-option", false, "Whether appending closetimeo=0 option to smb mount command")
appendNoShareSockOption = flag.Bool("append-nosharesock-option", true, "Whether appending nosharesock option to smb mount command")
appendNoResvPortOption = flag.Bool("append-noresvport-option", true, "Whether appending noresvport option to nfs mount command")
Expand Down Expand Up @@ -108,6 +109,7 @@ func handle() {
KubeAPIQPS: *kubeAPIQPS,
KubeAPIBurst: *kubeAPIBurst,
EnableWindowsHostProcess: *enableWindowsHostProcess,
RemoveSMBMountOnWindows: *removeSMBMountOnWindows,
AppendClosetimeoOption: *appendClosetimeoOption,
AppendNoShareSockOption: *appendNoShareSockOption,
AppendNoResvPortOption: *appendNoResvPortOption,
Expand Down
30 changes: 24 additions & 6 deletions pkg/mounter/safe_mounter_host_process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import (
"sigs.k8s.io/azurefile-csi-driver/pkg/os/smb"
)

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

var _ CSIProxyMounter = &winMounter{}

type winMounter struct{}
Expand Down Expand Up @@ -116,11 +118,6 @@ func (mounter *winMounter) SMBMount(source, target, fsType string, mountOptions,
return nil
}

func (mounter *winMounter) SMBUnmount(target string) error {
klog.V(2).Infof("SMBUnmount: local path: %s", target)
return mounter.Rmdir(target)
}

// Mount just creates a soft link at target pointing to source.
func (mounter *winMounter) Mount(source, target, fstype string, options []string) error {
return filesystem.LinkPath(normalizeWindowsPath(source), normalizeWindowsPath(target))
Expand All @@ -133,7 +130,28 @@ func (mounter *winMounter) Rmdir(path string) error {

// Unmount - Removes the directory - equivalent to unmount on Linux.
func (mounter *winMounter) Unmount(target string) error {
klog.V(2).Infof("Unmount: %s", target)
target = normalizeWindowsPath(target)
remoteServer, err := smb.GetRemoteServerFromTarget(target)
if err == nil {
klog.V(2).Infof("remote server path: %s, local path: %s", remoteServer, target)
hasDupSMBMount, err := smb.CheckForDuplicateSMBMounts(driverGlobalMountPath, target, remoteServer)
if err == nil {
if !hasDupSMBMount {
remoteServer = strings.Replace(remoteServer, "UNC\\", "\\\\", 1)
if err := smb.RemoveSmbGlobalMapping(remoteServer); err != nil {
klog.Errorf("RemoveSmbGlobalMapping(%s) failed with %v", target, err)
}
} else {
klog.V(2).Infof("skip unmount as there are other SMB mounts on the same remote server %s", remoteServer)
}
} else {
klog.Errorf("CheckForDuplicateSMBMounts(%s, %s) failed with %v", target, remoteServer, err)
}
} else {
klog.Errorf("GetRemoteServerFromTarget(%s) failed with %v", target, err)
}

klog.V(2).Infof("Unmount: remote path: %s local path: %s", remoteServer, target)
return mounter.Rmdir(target)
}

Expand Down
7 changes: 0 additions & 7 deletions pkg/mounter/safe_mounter_v1beta_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,6 @@ func (mounter *csiProxyMounterV1Beta) SMBMount(source, target, fsType string, mo
return nil
}

func (mounter *csiProxyMounterV1Beta) SMBUnmount(target string) error {
klog.V(4).Infof("SMBUnmount: local path: %s", target)
// TODO: We need to remove the SMB mapping. The change to remove the
// directory brings the CSI code in parity with the in-tree.
return mounter.Rmdir(target)
}

// Mount just creates a soft link at target pointing to source.
func (mounter *csiProxyMounterV1Beta) Mount(source string, target string, fstype string, options []string) error {
klog.V(4).Infof("Mount: old name: %s. new name: %s", source, target)
Expand Down
45 changes: 45 additions & 0 deletions pkg/os/smb/smb.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package smb

import (
"fmt"
"os"
"path/filepath"
"strings"

"k8s.io/klog/v2"
Expand Down Expand Up @@ -62,3 +64,46 @@ func RemoveSmbGlobalMapping(remotePath string) error {
}
return nil
}

// 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) {
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
}

// CheckForDuplicateSMBMounts checks if there is any other SMB mount exists on the same remote server
func CheckForDuplicateSMBMounts(dir, mount, remoteServer string) (bool, error) {
files, err := os.ReadDir(dir)
if err != nil {
return false, err
}

for _, file := range files {
klog.V(6).Infof("checking file %s", file.Name())
if file.IsDir() {
globalMountPath := filepath.Join(dir, file.Name(), "globalmount")
if strings.EqualFold(filepath.Clean(globalMountPath), filepath.Clean(mount)) {
klog.V(2).Infof("skip current mount path %s", mount)
} else {
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)
klog.V(2).Infof("checking remote server path %s on local path %s", remoteServerPath, globalMountPath)
if err == nil {
if remoteServerPath == remoteServer {
return true, nil
}
} else {
klog.Errorf("GetRemoteServerFromTarget(%s) failed with %v", globalMountPath, err)
}
}
}
}
}
return false, err
}

0 comments on commit d42569b

Please sign in to comment.