fix smb remount issue on Windows

add comments for doSMBMount func

fix comments about smb mount

pkg/util/mount/mount_windows.go

@@ -49,12 +49,13 @@ func New(mounterPath string) Interface {
 	}
 }
 
-// Mount : mounts source to target as NTFS with given options.
+// Mount : mounts source to target with given options.
+// currently only supports cifs(smb), bind mount(for disk)
 func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error {
 	target = normalizeWindowsPath(target)
 
 	if source == "tmpfs" {
-		klog.V(3).Infof("azureMount: mounting source (%q), target (%q), with options (%q)", source, target, options)
+		klog.V(3).Infof("mounting source (%q), target (%q), with options (%q)", source, target, options)
 		return os.MkdirAll(target, 0755)
 	}
 
@@ -63,9 +64,9 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio
 		return err
 	}
 
-	klog.V(4).Infof("azureMount: mount options(%q) source:%q, target:%q, fstype:%q, begin to mount",
+	klog.V(4).Infof("mount options(%q) source:%q, target:%q, fstype:%q, begin to mount",
 		options, source, target, fstype)
-	bindSource := ""
+	bindSource := source
 
 	// tell it's going to mount azure disk or azure file according to options
 	if bind, _, _ := isBind(options); bind {
@@ -73,31 +74,28 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio
 		bindSource = normalizeWindowsPath(source)
 	} else {
 		if len(options) < 2 {
-			klog.Warningf("azureMount: mount options(%q) command number(%d) less than 2, source:%q, target:%q, skip mounting",
+			klog.Warningf("mount options(%q) command number(%d) less than 2, source:%q, target:%q, skip mounting",
 				options, len(options), source, target)
 			return nil
 		}
 
 		// currently only cifs mount is supported
 		if strings.ToLower(fstype) != "cifs" {
-			return fmt.Errorf("azureMount: only cifs mount is supported now, fstype: %q, mounting source (%q), target (%q), with options (%q)", fstype, source, target, options)
+			return fmt.Errorf("only cifs mount is supported now, fstype: %q, mounting source (%q), target (%q), with options (%q)", fstype, source, target, options)
 		}
 
-		bindSource = source
+		if output, err := newSMBMapping(options[0], options[1], source); err != nil {
-
+			if isSMBMappingExist(source) {
-		// use PowerShell Environment Variables to store user input string to prevent command line injection
+				klog.V(2).Infof("SMB Mapping(%s) already exists, now begin to remove and remount", source)
-		// https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables?view=powershell-5.1
+				if output, err := removeSMBMapping(source); err != nil {
-		cmdLine := fmt.Sprintf(`$PWord = ConvertTo-SecureString -String $Env:smbpassword -AsPlainText -Force` +
+					return fmt.Errorf("Remove-SmbGlobalMapping failed: %v, output: %q", err, output)
-			`;$Credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $Env:smbuser, $PWord` +
+				}
-			`;New-SmbGlobalMapping -RemotePath $Env:smbremotepath -Credential $Credential`)
+				if output, err := newSMBMapping(options[0], options[1], source); err != nil {
-
+					return fmt.Errorf("New-SmbGlobalMapping remount failed: %v, output: %q", err, output)
-		cmd := exec.Command("powershell", "/c", cmdLine)
+				}
-		cmd.Env = append(os.Environ(),
+			} else {
-			fmt.Sprintf("smbuser=%s", options[0]),
+				return fmt.Errorf("New-SmbGlobalMapping failed: %v, output: %q", err, output)
-			fmt.Sprintf("smbpassword=%s", options[1]),
+			}
-			fmt.Sprintf("smbremotepath=%s", source))
-		if output, err := cmd.CombinedOutput(); err != nil {
-			return fmt.Errorf("azureMount: SmbGlobalMapping failed: %v, only SMB mount is supported now, output: %q", err, string(output))
 		}
 	}
 
@@ -109,6 +107,44 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio
 	return nil
 }
 
+// do the SMB mount with username, password, remotepath
+// return (output, error)
+func newSMBMapping(username, password, remotepath string) (string, error) {
+	if username == "" || password == "" || remotepath == "" {
+		return "", fmt.Errorf("invalid parameter(username: %s, password: %s, remoteapth: %s)", username, password, remotepath)
+	}
+
+	// use PowerShell Environment Variables to store user input string to prevent command line injection
+	// https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables?view=powershell-5.1
+	cmdLine := `$PWord = ConvertTo-SecureString -String $Env:smbpassword -AsPlainText -Force` +
+		`;$Credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $Env:smbuser, $PWord` +
+		`;New-SmbGlobalMapping -RemotePath $Env:smbremotepath -Credential $Credential`
+	cmd := exec.Command("powershell", "/c", cmdLine)
+	cmd.Env = append(os.Environ(),
+		fmt.Sprintf("smbuser=%s", username),
+		fmt.Sprintf("smbpassword=%s", password),
+		fmt.Sprintf("smbremotepath=%s", remotepath))
+
+	output, err := cmd.CombinedOutput()
+	return string(output), err
+}
+
+// check whether remotepath is already mounted
+func isSMBMappingExist(remotepath string) bool {
+	cmd := exec.Command("powershell", "/c", `Get-SmbGlobalMapping -RemotePath $Env:smbremotepath`)
+	cmd.Env = append(os.Environ(), fmt.Sprintf("smbremotepath=%s", remotepath))
+	_, err := cmd.CombinedOutput()
+	return err == nil
+}
+
+// remove SMB mapping
+func removeSMBMapping(remotepath string) (string, error) {
+	cmd := exec.Command("powershell", "/c", `Remove-SmbGlobalMapping -RemotePath $Env:smbremotepath -Force`)
+	cmd.Env = append(os.Environ(), fmt.Sprintf("smbremotepath=%s", remotepath))
+	output, err := cmd.CombinedOutput()
+	return string(output), err
+}
+
 // Unmount unmounts the target.
 func (mounter *Mounter) Unmount(target string) error {
 	klog.V(4).Infof("azureMount: Unmount target (%q)", target)

pkg/util/mount/mount_windows_test.go

@@ -788,3 +788,40 @@ func TestFormatAndMount(t *testing.T) {
 		}
 	}
 }
+
+func TestNewSMBMapping(t *testing.T) {
+	tests := []struct {
+		username    string
+		password    string
+		remotepath  string
+		expectError bool
+	}{
+		{
+			"",
+			"password",
+			`\\remotepath`,
+			true,
+		},
+		{
+			"username",
+			"",
+			`\\remotepath`,
+			true,
+		},
+		{
+			"username",
+			"password",
+			"",
+			true,
+		},
+	}
+
+	for _, test := range tests {
+		_, err := newSMBMapping(test.username, test.password, test.remotepath)
+		if test.expectError {
+			assert.NotNil(t, err, "Expect error during newSMBMapping(%s, %s, %s, %v)", test.username, test.password, test.remotepath)
+		} else {
+			assert.Nil(t, err, "Expect error is nil during newSMBMapping(%s, %s, %s, %v)", test.username, test.password, test.remotepath)
+		}
+	}
+}