diff --git a/pkg/volume/local/local_test.go b/pkg/volume/local/local_test.go index 0a956aaf654..8d549ae7c15 100644 --- a/pkg/volume/local/local_test.go +++ b/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) diff --git a/pkg/volume/rbd/rbd_test.go b/pkg/volume/rbd/rbd_test.go index 2e0b608ddad..b11246666ce 100644 --- a/pkg/volume/rbd/rbd_test.go +++ b/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() diff --git a/pkg/volume/rbd/rbd_unix_test.go b/pkg/volume/rbd/rbd_unix_test.go index 4c7877be013..1c935be80f0 100644 --- a/pkg/volume/rbd/rbd_unix_test.go +++ b/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 } diff --git a/pkg/volume/rbd/rbd_windows_test.go b/pkg/volume/rbd/rbd_windows_test.go index 208b332493c..a915dc30de8 100644 --- a/pkg/volume/rbd/rbd_windows_test.go +++ b/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 } diff --git a/pkg/volume/vsphere_volume/vsphere_volume_util_windows.go b/pkg/volume/vsphere_volume/vsphere_volume_util_windows.go index b9f2b6c590b..ae85c73f431 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume_util_windows.go +++ b/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 { diff --git a/staging/src/k8s.io/mount-utils/mount_windows.go b/staging/src/k8s.io/mount-utils/mount_windows.go index 7b2a2d1a15e..4f2b7aee873 100644 --- a/staging/src/k8s.io/mount-utils/mount_windows.go +++ b/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) }