unittests: Fixes unit tests for Windows (part 7)

Currently, there are some unit tests that are failing on Windows due to
various reasons:

- if a powershell command that could return an array (e.g.: Get-Disk) would return an array of
  only one element, powershell will in fact return that object directly, and **not** an array
  containing that element. In a few cases, these commands are used and their output is converted
  to json, after which they're unmarshalled in golang, with the expectation that the unmarshalled
  data to be an array. If it's not an array, we get an error.
- when mounting Block Devices, Windows expects the given source to be a Disk Number, not a path.
- for rbd_windows_test.go, we should start with Disk Number 0, which exists on all hosts.
- if a Disk has multiple volumes, Get-Volume doesn't return the volumes in the same order. This
  can result in various assertions failing.
- the pkg/volume/rbd/rdb_test.TestPlugin test expects that mounter.MountSensitive is called when
  attacher.MountDevice is called. The Windows attacher doesn't currently make that call.

pkg/volume/local/local_test.go

@@ -136,30 +136,40 @@ func getPersistentPlugin(t *testing.T) (string, volume.PersistentVolumePlugin) {
 }
 
 func getDeviceMountablePluginWithBlockPath(t *testing.T, isBlockDevice bool) (string, volume.DeviceMountableVolumePlugin) {
-	tmpDir, err := utiltesting.MkTmpdir("localVolumeTest")
-	if err != nil {
-		t.Fatalf("can't make a temp dir: %v", err)
+	var (
+		source string
+		err    error
+	)
+
+	if isBlockDevice && runtime.GOOS == "windows" {
+		// On Windows, block devices are referenced by the disk number, which is validated by the mounter,
+		source = "0"
+	} else {
+		source, err = utiltesting.MkTmpdir("localVolumeTest")
+		if err != nil {
+			t.Fatalf("can't make a temp dir: %v", err)
+		}
 	}
 
 	plugMgr := volume.VolumePluginMgr{}
 	var pathToFSType map[string]hostutil.FileType
 	if isBlockDevice {
 		pathToFSType = map[string]hostutil.FileType{
-			tmpDir: hostutil.FileTypeBlockDev,
+			source: hostutil.FileTypeBlockDev,
 		}
 	}
 
-	plugMgr.InitPlugins(ProbeVolumePlugins(), nil /* prober */, volumetest.NewFakeKubeletVolumeHostWithMounterFSType(t, tmpDir, nil, nil, pathToFSType))
+	plugMgr.InitPlugins(ProbeVolumePlugins(), nil /* prober */, volumetest.NewFakeKubeletVolumeHostWithMounterFSType(t, source, nil, nil, pathToFSType))
 
 	plug, err := plugMgr.FindDeviceMountablePluginByName(localVolumePluginName)
 	if err != nil {
-		os.RemoveAll(tmpDir)
+		os.RemoveAll(source)
 		t.Fatalf("Can't find the plugin by name")
 	}
 	if plug.GetPluginName() != localVolumePluginName {
 		t.Errorf("Wrong name: %s", plug.GetPluginName())
 	}
-	return tmpDir, plug
+	return source, plug
 }
 
 func getTestVolume(readOnly bool, path string, isBlock bool, mountOptions []string) *volume.Spec {
@@ -244,11 +254,6 @@ func TestInvalidLocalPath(t *testing.T) {
 }
 
 func TestBlockDeviceGlobalPathAndMountDevice(t *testing.T) {
-	// Skip tests that fail on Windows, as discussed during the SIG Testing meeting from January 10, 2023
-	if runtime.GOOS == "windows" {
-		t.Skip("Skipping test that fails on Windows")
-	}
-
 	// Block device global mount path testing
 	tmpBlockDir, plug := getDeviceMountablePluginWithBlockPath(t, true)
 	defer os.RemoveAll(tmpBlockDir)

pkg/volume/rbd/rbd_test.go

@@ -359,11 +359,6 @@ type testcase struct {
 }
 
 func TestPlugin(t *testing.T) {
-	// Skip tests that fail on Windows, as discussed during the SIG Testing meeting from January 10, 2023
-	if runtime.GOOS == "windows" {
-		t.Skip("Skipping test that fails on Windows")
-	}
-
 	tmpDir, err := utiltesting.MkTmpdir("rbd_test")
 	if err != nil {
 		t.Fatalf("error creating temp dir: %v", err)
@@ -374,10 +369,10 @@ func TestPlugin(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	expectedDevicePath := "/dev/rbd1"
+	expectedDevicePath := "/dev/rbd0"
 	if runtime.GOOS == "windows" {
 		// Windows expects Disk Numbers.
-		expectedDevicePath = "1"
+		expectedDevicePath = "0"
 	}
 
 	podUID := uuid.NewUUID()

pkg/volume/rbd/rbd_unix_test.go

@@ -26,9 +26,10 @@ import (
 func (fake *fakeDiskManager) AttachDisk(b rbdMounter) (string, error) {
 	fake.mutex.Lock()
 	defer fake.mutex.Unlock()
-	fake.rbdMapIndex++
 	devicePath := fmt.Sprintf("/dev/rbd%d", fake.rbdMapIndex)
 	fake.rbdDevices[devicePath] = true
+	// Increment rbdMapIndex afterwards, so we can start from rbd0.
+	fake.rbdMapIndex++
 	return devicePath, nil
 }
 

pkg/volume/rbd/rbd_windows_test.go

@@ -20,42 +20,29 @@ limitations under the License.
 package rbd
 
 import (
-	"fmt"
-	"os/exec"
 	"strconv"
-	"strings"
+
+	"k8s.io/mount-utils"
 )
 
 func (fake *fakeDiskManager) AttachDisk(b rbdMounter) (string, error) {
 	fake.mutex.Lock()
 	defer fake.mutex.Unlock()
-	fake.rbdMapIndex++
 
-	// Windows expects Disk Numbers.
-	volIds, err := listVolumesOnDisk(strconv.Itoa(fake.rbdMapIndex))
+	// Windows expects Disk Numbers. We start with rbdMapIndex 0, referring to the first Disk.
+	volIds, err := mount.ListVolumesOnDisk(strconv.Itoa(fake.rbdMapIndex))
 	if err != nil {
 		return "", err
 	}
 	fake.rbdDevices[volIds[0]] = true
 	devicePath := strconv.Itoa(fake.rbdMapIndex)
+	fake.rbdMapIndex++
 	return devicePath, nil
 }
 
-// listVolumesOnDisk - returns back list of volumes(volumeIDs) in the disk (requested in diskID) on Windows.
-func listVolumesOnDisk(diskID string) (volumeIDs []string, err error) {
-	cmd := fmt.Sprintf("(Get-Disk -DeviceId %s | Get-Partition | Get-Volume).UniqueId", diskID)
-	output, err := exec.Command("powershell", "/c", cmd).CombinedOutput()
-	if err != nil {
-		return []string{}, fmt.Errorf("error list volumes on disk. cmd: %s, output: %s, error: %v", cmd, string(output), err)
-	}
-
-	volumeIds := strings.Split(strings.TrimSpace(string(output)), "\r\n")
-	return volumeIds, nil
-}
-
 func getLoggedSource(devicePath string) (string, error) {
 	// Windows mounter is mounting based on the Disk's Unique ID.
-	volIds, err := listVolumesOnDisk(devicePath)
+	volIds, err := mount.ListVolumesOnDisk(devicePath)
 	if err != nil {
 		return "", err
 	}

pkg/volume/vsphere_volume/vsphere_volume_util_windows.go

@@ -40,6 +40,10 @@ func verifyDevicePath(path string) (string, error) {
 		klog.V(4).Infof("Found vSphere disk attached with disk number %v", path)
 		return path, nil
 	}
+	// NOTE: If a powershell command that would return an array (e.g.: Get-Disk) would return an array of
+	// one element, powershell will in fact return that object directly, and **not an array containing
+	// that elemenent, which means piping it to ConvertTo-Json would not result in array as expected below.
+	// The following syntax forces it to always be an array.
 	cmd := exec.Command("powershell", "/c", "Get-Disk | Select Number, SerialNumber | ConvertTo-JSON")
 	output, err := cmd.Output()
 	if err != nil {

staging/src/k8s.io/mount-utils/mount_windows.go

@@ -299,26 +299,20 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target
 	}
 	klog.V(4).Infof("diskMount: Disk successfully formatted, disk: %q, fstype: %q", source, fstype)
 
-	volumeIds, err := listVolumesOnDisk(source)
+	volumeIds, err := ListVolumesOnDisk(source)
 	if err != nil {
 		return err
 	}
 	driverPath := volumeIds[0]
-	target = NormalizeWindowsPath(target)
-	output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, driverPath).CombinedOutput()
-	if err != nil {
-		klog.Errorf("mklink(%s, %s) failed: %v, output: %q", target, driverPath, err, string(output))
-		return err
-	}
-	klog.V(2).Infof("formatAndMount disk(%s) fstype(%s) on(%s) with output(%s) successfully", driverPath, fstype, target, string(output))
-	return nil
+	return mounter.MountSensitive(driverPath, target, fstype, options, sensitiveOptions)
 }
 
 // ListVolumesOnDisk - returns back list of volumes(volumeIDs) in the disk (requested in diskID).
-func listVolumesOnDisk(diskID string) (volumeIDs []string, err error) {
-	cmd := fmt.Sprintf("(Get-Disk -DeviceId %s | Get-Partition | Get-Volume).UniqueId", diskID)
+func ListVolumesOnDisk(diskID string) (volumeIDs []string, err error) {
+	// If a Disk has multiple volumes, Get-Volume may not return items in the same order.
+	cmd := fmt.Sprintf("(Get-Disk -DeviceId %s | Get-Partition | Get-Volume | Sort-Object -Property UniqueId).UniqueId", diskID)
 	output, err := exec.Command("powershell", "/c", cmd).CombinedOutput()
-	klog.V(4).Infof("listVolumesOnDisk id from %s: %s", diskID, string(output))
+	klog.V(4).Infof("ListVolumesOnDisk id from %s: %s", diskID, string(output))
 	if err != nil {
 		return []string{}, fmt.Errorf("error list volumes on disk. cmd: %s, output: %s, error: %v", cmd, string(output), err)
 	}