Merge pull request #114764 from claudiubelu/unittests-7

unittests: Fixes unit tests for Windows (part 7)

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")
+	var (
-	if err != nil {
+		source string
-		t.Fatalf("can't make a temp dir: %v", err)
+		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.
+	// Windows expects Disk Numbers. We start with rbdMapIndex 0, referring to the first Disk.
-	volIds, err := listVolumesOnDisk(strconv.Itoa(fake.rbdMapIndex))
+	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)
+	return mounter.MountSensitive(driverPath, target, fstype, options, sensitiveOptions)
-	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
 }
 
 // ListVolumesOnDisk - returns back list of volumes(volumeIDs) in the disk (requested in diskID).
-func listVolumesOnDisk(diskID string) (volumeIDs []string, err error) {
+func ListVolumesOnDisk(diskID string) (volumeIDs []string, err error) {
-	cmd := fmt.Sprintf("(Get-Disk -DeviceId %s | Get-Partition | Get-Volume).UniqueId", diskID)
+	// 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)
 	}