Merge pull request #110399 from claudiubelu/unittests-2

unittests: Fixes unit tests for Windows (part 2)

pkg/volume/awsebs/attacher_test.go

@@ -21,6 +21,7 @@ package awsebs
 
 import (
 	"errors"
+	"os"
 	"testing"
 
 	"k8s.io/klog/v2"
@@ -145,7 +146,7 @@ func TestAttachDetach(t *testing.T) {
 // newPlugin creates a new gcePersistentDiskPlugin with fake cloud, NewAttacher
 // and NewDetacher won't work.
 func newPlugin(t *testing.T) *awsElasticBlockStorePlugin {
-	host := volumetest.NewFakeVolumeHost(t, "/tmp", nil, nil)
+	host := volumetest.NewFakeVolumeHost(t, os.TempDir(), nil, nil)
 	plugins := ProbeVolumePlugins()
 	plugin := plugins[0]
 	plugin.Init(host)

pkg/volume/awsebs/aws_ebs_block.go

@@ -22,6 +22,7 @@ package awsebs
 import (
 	"fmt"
 	"path/filepath"
+	goruntime "runtime"
 	"strconv"
 	"strings"
 
@@ -66,6 +67,10 @@ func (plugin *awsElasticBlockStorePlugin) getVolumeSpecFromGlobalMapPath(volumeN
 	}
 	fullVolumeID := strings.TrimPrefix(globalMapPath, pluginDir) // /vol-XXXXXX
 	fullVolumeID = strings.TrimLeft(fullVolumeID, "/")           // vol-XXXXXX
+	// Windows paths have \\ instead.
+	if goruntime.GOOS == "windows" {
+		fullVolumeID = strings.TrimLeft(fullVolumeID, "\\") // vol-XXXXXX
+	}
 	vID, err := formatVolumeID(fullVolumeID)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get AWS volume id from map path %q: %v", globalMapPath, err)

pkg/volume/awsebs/aws_ebs_test.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	goruntime "runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -145,11 +146,16 @@ func TestPlugin(t *testing.T) {
 	if err := mounter.SetUp(volume.MounterArgs{}); err != nil {
 		t.Errorf("Expected success, got: %v", err)
 	}
-	if _, err := os.Stat(path); err != nil {
+
-		if os.IsNotExist(err) {
+	// On Windows, Mount will create the parent of dir and mklink (create a symbolic link) at the volume path later,
-			t.Errorf("SetUp() failed, volume path not created: %s", path)
+	// so mounter.SetUp will not create the directory. Otherwise mklink will error: "Cannot create a file when that file already exists".
-		} else {
+	if goruntime.GOOS != "windows" {
-			t.Errorf("SetUp() failed: %v", err)
+		if _, err := os.Stat(path); err != nil {
+			if os.IsNotExist(err) {
+				t.Errorf("SetUp() failed, volume path not created: %s", path)
+			} else {
+				t.Errorf("SetUp() failed: %v", err)
+			}
 		}
 	}
 

pkg/volume/azure_file/azure_file_test.go

@@ -25,6 +25,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	goruntime "runtime"
 	"strings"
 	"testing"
 
@@ -158,11 +159,15 @@ func testPlugin(t *testing.T, tmpDir string, volumeHost volume.VolumeHost) {
 	if err := mounter.SetUp(volume.MounterArgs{}); err != nil {
 		t.Errorf("Expected success, got: %v", err)
 	}
-	if _, err := os.Stat(path); err != nil {
+	// On Windows, Mount will create the parent of dir and mklink (create a symbolic link) at the volume path later,
-		if os.IsNotExist(err) {
+	// so mounter.SetUp will not create the directory. Otherwise mklink will error: "Cannot create a file when that file already exists".
-			t.Errorf("SetUp() failed, volume path not created: %s", path)
+	if goruntime.GOOS != "windows" {
-		} else {
+		if _, err := os.Stat(path); err != nil {
-			t.Errorf("SetUp() failed: %v", err)
+			if os.IsNotExist(err) {
+				t.Errorf("SetUp() failed, volume path not created: %s", path)
+			} else {
+				t.Errorf("SetUp() failed: %v", err)
+			}
 		}
 	}
 
@@ -215,7 +220,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	client := fake.NewSimpleClientset(pv, claim)
 
 	plugMgr := volume.VolumePluginMgr{}
-	plugMgr.InitPlugins(ProbeVolumePlugins(), nil /* prober */, volumetest.NewFakeVolumeHost(t, "/tmp/fake", client, nil))
+	plugMgr.InitPlugins(ProbeVolumePlugins(), nil /* prober */, volumetest.NewFakeVolumeHost(t, filepath.Join(os.TempDir(), "fake"), client, nil))
 	plug, _ := plugMgr.FindPluginByName(azureFilePluginName)
 
 	// readOnly bool is supplied by persistent-claim volume source when its mounter creates other volumes

pkg/volume/cinder/attacher_test.go

@@ -22,6 +22,8 @@ package cinder
 import (
 	"context"
 	"errors"
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -89,7 +91,7 @@ func TestGetDeviceMountPath(t *testing.T) {
 	if err != nil {
 		t.Errorf("Get device mount path error")
 	}
-	expectedPath := rootDir + "plugins/kubernetes.io/cinder/mounts/" + name
+	expectedPath := filepath.Join(rootDir, "plugins/kubernetes.io/cinder/mounts", name)
 	if path != expectedPath {
 		t.Errorf("Device mount path error: expected %s, got %s ", expectedPath, path)
 	}
@@ -357,7 +359,7 @@ func serializeAttachments(attachments map[*volume.Spec]bool) string {
 // newPlugin creates a new gcePersistentDiskPlugin with fake cloud, NewAttacher
 // and NewDetacher won't work.
 func newPlugin(t *testing.T) *cinderPlugin {
-	host := volumetest.NewFakeVolumeHost(t, "/tmp", nil, nil)
+	host := volumetest.NewFakeVolumeHost(t, os.TempDir(), nil, nil)
 	plugins := ProbeVolumePlugins()
 	plugin := plugins[0]
 	plugin.Init(host)

pkg/volume/configmap/configmap_test.go

@@ -292,7 +292,7 @@ func TestMakePayload(t *testing.T) {
 }
 
 func newTestHost(t *testing.T, clientset clientset.Interface) (string, volume.VolumeHost) {
-	tempDir, err := ioutil.TempDir("/tmp", "configmap_volume_test.")
+	tempDir, err := ioutil.TempDir("", "configmap_volume_test.")
 	if err != nil {
 		t.Fatalf("can't make a temp rootdir: %v", err)
 	}
@@ -361,7 +361,7 @@ func TestPlugin(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -421,7 +421,7 @@ func TestPluginReboot(t *testing.T) {
 	podMetadataDir := fmt.Sprintf("%v/pods/test_pod_uid3/plugins/kubernetes.io~configmap/test_volume_name", rootDir)
 	util.SetReady(podMetadataDir)
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid3/volumes/kubernetes.io~configmap/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid3/volumes/kubernetes.io~configmap/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -485,7 +485,7 @@ func TestPluginOptional(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -584,7 +584,7 @@ func TestPluginKeysOptional(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -664,7 +664,7 @@ func TestInvalidConfigMapSetup(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~configmap/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -733,3 +733,7 @@ func doTestCleanAndTeardown(plugin volume.VolumePlugin, podUID types.UID, testVo
 		t.Errorf("TearDown() failed: %v", err)
 	}
 }
+
+func hasPathSuffix(s, suffix string) bool {
+	return strings.HasSuffix(s, filepath.FromSlash(suffix))
+}

pkg/volume/csi/csi_attacher_test.go

@@ -25,6 +25,7 @@ import (
 	"os/user"
 	"path/filepath"
 	"reflect"
+	goruntime "runtime"
 	"sync"
 	"testing"
 	"time"
@@ -1113,6 +1114,7 @@ func TestAttacherMountDevice(t *testing.T) {
 		delegateFSGroupFeatureGate     bool
 		driverSupportsVolumeMountGroup bool
 		shouldFail                     bool
+		skipOnWindows                  bool
 		createAttachment               bool
 		populateDeviceMountPath        bool
 		exitError                      error
@@ -1216,7 +1218,12 @@ func TestAttacherMountDevice(t *testing.T) {
 			createAttachment:        true,
 			populateDeviceMountPath: true,
 			shouldFail:              true,
-			spec:                    volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), true),
+			// NOTE: We're skipping this test on Windows because os.Chmod is not working as intended, which means that
+			// this test won't fail on Windows due to permission denied errors.
+			// TODO: Remove the skip once Windows file permissions support is added.
+			// https://github.com/kubernetes/kubernetes/pull/110921
+			skipOnWindows: true,
+			spec:          volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), true),
 		},
 		{
 			testName:                       "fsgroup provided, DelegateFSGroupToCSIDriver feature enabled, driver supports volume mount group; expect fsgroup to be passed to NodeStageVolume",
@@ -1281,6 +1288,9 @@ func TestAttacherMountDevice(t *testing.T) {
 			}
 		}
 		t.Run(tc.testName, func(t *testing.T) {
+			if tc.skipOnWindows && goruntime.GOOS == "windows" {
+				t.Skipf("Skipping test case on Windows: %s", tc.testName)
+			}
 			t.Logf("Running test case: %s", tc.testName)
 
 			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, tc.delegateFSGroupFeatureGate)()
@@ -1348,7 +1358,7 @@ func TestAttacherMountDevice(t *testing.T) {
 				if tc.populateDeviceMountPath {
 					// We're expecting saveVolumeData to fail, which is responsible
 					// for creating this file. It shouldn't exist.
-					_, err := os.Stat(parent + "/" + volDataFileName)
+					_, err := os.Stat(filepath.Join(parent, volDataFileName))
 					if !os.IsNotExist(err) {
 						t.Errorf("vol_data.json should not exist: %v", err)
 					}

pkg/volume/csi/csi_mounter_test.go

@@ -22,9 +22,9 @@ import (
 	"io/ioutil"
 	"math/rand"
 	"os"
-	"path"
 	"path/filepath"
 	"reflect"
+	goruntime "runtime"
 	"testing"
 	"time"
 
@@ -903,19 +903,25 @@ func TestUnmounterTeardown(t *testing.T) {
 	pv := makeTestPV("test-pv", 10, testDriver, testVol)
 
 	// save the data file prior to unmount
-	dir := filepath.Join(getTargetPath(testPodUID, pv.ObjectMeta.Name, plug.host), "/mount")
+	targetDir := getTargetPath(testPodUID, pv.ObjectMeta.Name, plug.host)
+	dir := filepath.Join(targetDir, "mount")
 	if err := os.MkdirAll(dir, 0755); err != nil && !os.IsNotExist(err) {
 		t.Errorf("failed to create dir [%s]: %v", dir, err)
 	}
 
 	// do a fake local mount
 	diskMounter := util.NewSafeFormatAndMountFromHost(plug.GetPluginName(), plug.host)
-	if err := diskMounter.FormatAndMount("/fake/device", dir, "testfs", nil); err != nil {
+	device := "/fake/device"
+	if goruntime.GOOS == "windows" {
+		// We need disk numbers on Windows.
+		device = "1"
+	}
+	if err := diskMounter.FormatAndMount(device, dir, "testfs", nil); err != nil {
 		t.Errorf("failed to mount dir [%s]: %v", dir, err)
 	}
 
 	if err := saveVolumeData(
-		path.Dir(dir),
+		targetDir,
 		volDataFileName,
 		map[string]string{
 			volDataKey.specVolID:  pv.ObjectMeta.Name,

pkg/volume/csi/csi_util_test.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path"
 	"path/filepath"
 	"testing"
 	"time"
@@ -116,12 +115,13 @@ func TestSaveVolumeData(t *testing.T) {
 	for i, tc := range testCases {
 		t.Logf("test case: %s", tc.name)
 		specVolID := fmt.Sprintf("spec-volid-%d", i)
-		mountDir := filepath.Join(getTargetPath(testPodUID, specVolID, plug.host), "/mount")
+		targetPath := getTargetPath(testPodUID, specVolID, plug.host)
+		mountDir := filepath.Join(targetPath, "mount")
 		if err := os.MkdirAll(mountDir, 0755); err != nil && !os.IsNotExist(err) {
 			t.Errorf("failed to create dir [%s]: %v", mountDir, err)
 		}
 
-		err := saveVolumeData(path.Dir(mountDir), volDataFileName, tc.data)
+		err := saveVolumeData(targetPath, volDataFileName, tc.data)
 
 		if !tc.shouldFail && err != nil {
 			t.Errorf("unexpected failure: %v", err)

pkg/volume/fc/fc_test.go

@@ -17,8 +17,8 @@ limitations under the License.
 package fc
 
 import (
-	"fmt"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
@@ -172,7 +172,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) {
 	}
 
 	path := mounter.GetPath()
-	expectedPath := fmt.Sprintf("%s/pods/poduid/volumes/kubernetes.io~fc/vol1", tmpDir)
+	expectedPath := filepath.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~fc/vol1")
 	if path != expectedPath {
 		t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, path)
 	}

pkg/volume/flexvolume/common_test.go

@@ -18,6 +18,7 @@ package flexvolume
 
 import (
 	"encoding/json"
+	goruntime "runtime"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/kubernetes/pkg/volume"
@@ -41,7 +42,11 @@ func testPlugin(h *harness.Harness) (*flexVolumeAttachablePlugin, string) {
 
 func assertDriverCall(t *harness.Harness, output exectesting.FakeAction, expectedCommand string, expectedArgs ...string) exectesting.FakeCommandAction {
 	return func(cmd string, args ...string) exec.Cmd {
-		if cmd != "/plugin/test" {
+		executable := "/plugin/test"
+		if goruntime.GOOS == "windows" {
+			executable = "c:\\plugin\\test"
+		}
+		if cmd != executable {
 			t.Errorf("Wrong executable called: got %v, expected %v", cmd, "/plugin/test")
 		}
 		if args[0] != expectedCommand {

pkg/volume/flexvolume/flexvolume_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	goruntime "runtime"
 	"testing"
 	"text/template"
 
@@ -75,6 +76,45 @@ exit 1
 echo -n $@ &> {{.OutputFile}}
 `
 
+// NOTE: Typically, Windows requires file extensions for executable files. If a file does not
+// have a file extension, Windows will check if there is a file with the given name + one of the
+// extensions from $env:PATHEXT (in order) and run that file with that extension.
+// For example, if we have the file C:\\foo.bat, we can run C:\\foo.
+// For these tests, .bat was chosen since it's one of the default values in $env.PATHEXT. .ps1 is
+// not in that list, but it might be useful for flexvolumes to be able to handle powershell scripts.
+// There's no argument count variable in batch. Instead, we can check that the n-th argument
+// is an empty string.
+const execScriptTemplBat = `
+@echo off
+
+if "%1"=="init" if "%2"=="" (
+    echo {"status": "Success"}
+    exit 0
+)
+if "%1"=="attach" if "%3"=="" (
+    echo {"device": "{{.DevicePath}}", "status": "Success"}
+    exit 0
+)
+
+if "%1"=="detach" if "%3"=="" (
+    echo {"status": "Success"}
+    exit 0
+)
+
+if "%1"=="getvolumename" if "%5"=="" (
+    echo {"status": "Success", "volume": "fakevolume"}
+    exit 0
+)
+
+if "%1"=="isattached" if "%3"=="" (
+    echo {"status": "Success", "attached": true}
+    exit 0
+)
+
+echo {"status": "Not supported"}
+exit 1
+`
+
 func installPluginUnderTest(t *testing.T, vendorName, plugName, tmpDir string, execScriptTempl string, execTemplateData *map[string]interface{}) {
 	vendoredName := plugName
 	if vendorName != "" {
@@ -86,6 +126,9 @@ func installPluginUnderTest(t *testing.T, vendorName, plugName, tmpDir string, e
 		t.Errorf("Failed to create plugin: %v", err)
 	}
 	pluginExec := filepath.Join(pluginDir, plugName)
+	if goruntime.GOOS == "windows" {
+		pluginExec = pluginExec + ".bat"
+	}
 	f, err := os.Create(pluginExec)
 	if err != nil {
 		t.Errorf("Failed to install plugin")
@@ -123,7 +166,11 @@ func TestCanSupport(t *testing.T) {
 
 	plugMgr := volume.VolumePluginMgr{}
 	runner := exec.New()
-	installPluginUnderTest(t, "kubernetes.io", "fakeAttacher", tmpDir, execScriptTempl1, nil)
+	execScriptTempl := execScriptTempl1
+	if goruntime.GOOS == "windows" {
+		execScriptTempl = execScriptTemplBat
+	}
+	installPluginUnderTest(t, "kubernetes.io", "fakeAttacher", tmpDir, execScriptTempl, nil)
 	if err := plugMgr.InitPlugins(nil, GetDynamicPluginProberWithoutWatcher(tmpDir, runner), volumetest.NewFakeVolumeHost(t, "fake", nil, nil)); err != nil {
 		t.Fatalf("Could not initialize plugins: %v", err)
 	}
@@ -154,7 +201,11 @@ func TestGetAccessModes(t *testing.T) {
 
 	plugMgr := volume.VolumePluginMgr{}
 	runner := exec.New()
-	installPluginUnderTest(t, "kubernetes.io", "fakeAttacher", tmpDir, execScriptTempl1, nil)
+	execScriptTempl := execScriptTempl1
+	if goruntime.GOOS == "windows" {
+		execScriptTempl = execScriptTemplBat
+	}
+	installPluginUnderTest(t, "kubernetes.io", "fakeAttacher", tmpDir, execScriptTempl, nil)
 	if err := plugMgr.InitPlugins(nil, GetDynamicPluginProberWithoutWatcher(tmpDir, runner), volumetest.NewFakeVolumeHost(t, tmpDir, nil, nil)); err != nil {
 		t.Fatalf("Could not initialize plugins: %v", err)
 	}

pkg/volume/flexvolume/probe_test.go

@@ -19,6 +19,7 @@ package flexvolume
 import (
 	"fmt"
 	"path/filepath"
+	goruntime "runtime"
 	"strings"
 	"testing"
 
@@ -52,7 +53,11 @@ func TestProberExistingDriverBeforeInit(t *testing.T) {
 	// current subdirectories) registered.
 	assert.Equal(t, 1, len(events))
 	assert.Equal(t, volume.ProbeAddOrUpdate, events[0].Op)
-	assertPathSuffix(t, pluginDir, watcher.watches[0])
+	plugDir := pluginDir
+	if goruntime.GOOS == "windows" {
+		plugDir = "\\flexvolume"
+	}
+	assertPathSuffix(t, plugDir, watcher.watches[0])
 	assertPathSuffix(t, driverPath, watcher.watches[1])
 	assert.NoError(t, err)
 
@@ -202,7 +207,8 @@ func TestEmptyPluginDir(t *testing.T) {
 func TestRemovePluginDir(t *testing.T) {
 	// Arrange
 	driverPath, fs, watcher, _ := initTestEnvironment(t)
-	fs.RemoveAll(pluginDir)
+	err := fs.RemoveAll(pluginDir)
+	assert.NoError(t, err)
 	watcher.TriggerEvent(fsnotify.Remove, filepath.Join(driverPath, driverName))
 	watcher.TriggerEvent(fsnotify.Remove, driverPath)
 	watcher.TriggerEvent(fsnotify.Remove, pluginDir)
@@ -211,7 +217,11 @@ func TestRemovePluginDir(t *testing.T) {
 
 	// Assert
 	assert.Equal(t, 3, len(watcher.watches)) // 2 from initial setup, 1 from new watch.
-	assertPathSuffix(t, pluginDir, watcher.watches[len(watcher.watches)-1])
+	plugDir := pluginDir
+	if goruntime.GOOS == "windows" {
+		plugDir = "\\flexvolume"
+	}
+	assertPathSuffix(t, plugDir, watcher.watches[len(watcher.watches)-1])
 }
 
 // Issue an event to remove plugindir. New directory should still be watched.
@@ -321,7 +331,10 @@ func TestProberSuccessAndError(t *testing.T) {
 func installDriver(driverName string, fs utilfs.Filesystem) {
 	driverPath := filepath.Join(pluginDir, driverName)
 	fs.MkdirAll(driverPath, 0777)
-	fs.Create(filepath.Join(driverPath, driverName))
+
+	// We need to close the file, otherwise we won't be able to remove it.
+	f, _ := fs.Create(filepath.Join(driverPath, driverName))
+	f.Close()
 }
 
 // Initializes mocks, installs a single driver in the mock fs, then initializes prober.

pkg/volume/gcepd/attacher_test.go

@@ -22,6 +22,8 @@ package gcepd
 import (
 	"errors"
 	"fmt"
+	"os"
+	"path/filepath"
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
@@ -96,8 +98,9 @@ func TestAttachDetachRegional(t *testing.T) {
 		test: func(testcase *testcase) error {
 			attacher := newAttacher(testcase)
 			devicePath, err := attacher.Attach(spec, nodeName)
-			if devicePath != "/dev/disk/by-id/google-disk" {
+			expectedDevicePath := filepath.FromSlash("/dev/disk/by-id/google-disk")
-				return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath)
+			if devicePath != expectedDevicePath {
+				return fmt.Errorf("devicePath incorrect. Expected<\"%s\"> Actual: <%q>", expectedDevicePath, devicePath)
 			}
 			return err
 		},
@@ -118,34 +121,30 @@ func TestAttachDetach(t *testing.T) {
 	attachError := errors.New("fake attach error")
 	detachError := errors.New("fake detach error")
 	diskCheckError := errors.New("fake DiskIsAttached error")
+
+	attachTestFunc := func(testcase *testcase) error {
+		attacher := newAttacher(testcase)
+		devicePath, err := attacher.Attach(spec, nodeName)
+		expectedDevicePath := filepath.FromSlash("/dev/disk/by-id/google-disk")
+		if devicePath != expectedDevicePath {
+			return fmt.Errorf("devicePath incorrect. Expected<\"%s\"> Actual: <%q>", expectedDevicePath, devicePath)
+		}
+		return err
+	}
 	tests := []testcase{
 		// Successful Attach call
 		{
 			name:           "Attach_Positive",
 			diskIsAttached: diskIsAttachedCall{disksAttachedMap{nodeName: {}}, nil},
 			attach:         attachCall{diskName, nodeName, readOnly, regional, nil},
-			test: func(testcase *testcase) error {
+			test:           attachTestFunc,
-				attacher := newAttacher(testcase)
-				devicePath, err := attacher.Attach(spec, nodeName)
-				if devicePath != "/dev/disk/by-id/google-disk" {
-					return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath)
-				}
-				return err
-			},
 		},
 
 		// Disk is already attached
 		{
 			name:           "Attach_Positive_AlreadyAttached",
 			diskIsAttached: diskIsAttachedCall{disksAttachedMap{nodeName: {diskName}}, nil},
-			test: func(testcase *testcase) error {
+			test:           attachTestFunc,
-				attacher := newAttacher(testcase)
-				devicePath, err := attacher.Attach(spec, nodeName)
-				if devicePath != "/dev/disk/by-id/google-disk" {
-					return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath)
-				}
-				return err
-			},
 		},
 
 		// DiskIsAttached fails and Attach succeeds
@@ -153,14 +152,7 @@ func TestAttachDetach(t *testing.T) {
 			name:           "Attach_Positive_CheckFails",
 			diskIsAttached: diskIsAttachedCall{disksAttachedMap{nodeName: {}}, diskCheckError},
 			attach:         attachCall{diskName, nodeName, readOnly, regional, nil},
-			test: func(testcase *testcase) error {
+			test:           attachTestFunc,
-				attacher := newAttacher(testcase)
-				devicePath, err := attacher.Attach(spec, nodeName)
-				if devicePath != "/dev/disk/by-id/google-disk" {
-					return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath)
-				}
-				return err
-			},
 		},
 
 		// Attach call fails
@@ -406,9 +398,9 @@ func TestVerifyVolumesAttached(t *testing.T) {
 // and NewDetacher won't work.
 func newPlugin(t *testing.T) *gcePersistentDiskPlugin {
 	host := volumetest.NewFakeVolumeHost(t,
-		"/tmp", /* rootDir */
+		os.TempDir(), /* rootDir */
-		nil,    /* kubeClient */
+		nil,          /* kubeClient */
-		nil,    /* plugins */
+		nil,          /* plugins */
 	)
 	plugins := ProbeVolumePlugins()
 	plugin := plugins[0]

pkg/volume/gcepd/gce_pd_test.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	goruntime "runtime"
 	"sort"
 	"testing"
 
@@ -148,11 +149,15 @@ func TestPlugin(t *testing.T) {
 	if err := mounter.SetUp(volume.MounterArgs{}); err != nil {
 		t.Errorf("Expected success, got: %v", err)
 	}
-	if _, err := os.Stat(path); err != nil {
+	// On Windows, Mount will create the parent of dir and mklink (create a symbolic link) at the volume path later,
-		if os.IsNotExist(err) {
+	// so mounter.SetUp will not create the directory. Otherwise mklink will error: "Cannot create a file when that file already exists".
-			t.Errorf("SetUp() failed, volume path not created: %s", path)
+	if goruntime.GOOS != "windows" {
-		} else {
+		if _, err := os.Stat(path); err != nil {
-			t.Errorf("SetUp() failed: %v", err)
+			if os.IsNotExist(err) {
+				t.Errorf("SetUp() failed, volume path not created: %s", path)
+			} else {
+				t.Errorf("SetUp() failed: %v", err)
+			}
 		}
 	}
 

pkg/volume/git_repo/git_repo_test.go

@@ -36,7 +36,7 @@ import (
 )
 
 func newTestHost(t *testing.T) (string, volume.VolumeHost) {
-	tempDir, err := ioutil.TempDir("/tmp", "git_repo_test.")
+	tempDir, err := ioutil.TempDir("", "git_repo_test.")
 	if err != nil {
 		t.Fatalf("can't make a temp rootdir: %v", err)
 	}
@@ -314,7 +314,7 @@ func doTestPlugin(scenario struct {
 	}
 
 	path := mounter.GetPath()
-	suffix := fmt.Sprintf("pods/poduid/volumes/kubernetes.io~git-repo/%v", scenario.vol.Name)
+	suffix := filepath.Join("pods/poduid/volumes/kubernetes.io~git-repo", scenario.vol.Name)
 	if !strings.HasSuffix(path, suffix) {
 		allErrs = append(allErrs,
 			fmt.Errorf("got unexpected path: %s", path))
@@ -439,7 +439,7 @@ func doTestSetUp(scenario struct {
 
 	var expectedPaths []string
 	for _, expected := range expecteds {
-		expectedPaths = append(expectedPaths, g.GetPath()+expected.dir)
+		expectedPaths = append(expectedPaths, filepath.Join(g.GetPath(), expected.dir))
 	}
 	if len(fcmd.Dirs) != len(expectedPaths) || !reflect.DeepEqual(expectedPaths, fcmd.Dirs) {
 		allErrs = append(allErrs,

pkg/volume/glusterfs/glusterfs_test.go

@@ -19,6 +19,7 @@ package glusterfs
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -115,7 +116,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) {
 	if mounter == nil {
 		t.Error("Got a nil Mounter")
 	}
-	expectedPath := fmt.Sprintf("%s/pods/poduid/volumes/kubernetes.io~glusterfs/vol1", tmpDir)
+	expectedPath := filepath.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~glusterfs/vol1")
 	if volumePath != expectedPath {
 		t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, volumePath)
 	}

pkg/volume/iscsi/iscsi_test.go

@@ -19,6 +19,7 @@ package iscsi
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -170,7 +171,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) {
 	}
 
 	path := mounter.GetPath()
-	expectedPath := fmt.Sprintf("%s/pods/poduid/volumes/kubernetes.io~iscsi/vol1", tmpDir)
+	expectedPath := filepath.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~iscsi/vol1")
 	if path != expectedPath {
 		t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, path)
 	}

pkg/volume/nfs/nfs_test.go

@@ -17,8 +17,8 @@ limitations under the License.
 package nfs
 
 import (
-	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"k8s.io/mount-utils"
@@ -119,7 +119,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec, expectedDevice string) {
 		t.Errorf("Got a nil Mounter")
 	}
 	volumePath := mounter.GetPath()
-	expectedPath := fmt.Sprintf("%s/pods/poduid/volumes/kubernetes.io~nfs/vol1", tmpDir)
+	expectedPath := filepath.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~nfs/vol1")
 	if volumePath != expectedPath {
 		t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, volumePath)
 	}

pkg/volume/projected/projected_test.go

@@ -874,7 +874,7 @@ func TestCollectDataWithServiceAccountToken(t *testing.T) {
 }
 
 func newTestHost(t *testing.T, clientset clientset.Interface) (string, volume.VolumeHost) {
-	tempDir, err := ioutil.TempDir("/tmp", "projected_volume_test.")
+	tempDir, err := ioutil.TempDir("", "projected_volume_test.")
 	if err != nil {
 		t.Fatalf("can't make a temp rootdir: %v", err)
 	}
@@ -934,7 +934,7 @@ func TestPlugin(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~projected/%s", testVolumeName)) {
+	if !strings.HasSuffix(volumePath, filepath.Join("pods/test_pod_uid/volumes/kubernetes.io~projected", testVolumeName)) {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -999,7 +999,7 @@ func TestInvalidPathProjected(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~projected/%s", testVolumeName)) {
+	if !strings.HasSuffix(volumePath, filepath.Join("pods/test_pod_uid/volumes/kubernetes.io~projected", testVolumeName)) {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -1051,7 +1051,7 @@ func TestPluginReboot(t *testing.T) {
 	podMetadataDir := fmt.Sprintf("%v/pods/test_pod_uid3/plugins/kubernetes.io~projected/test_volume_name", rootDir)
 	util.SetReady(podMetadataDir)
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid3/volumes/kubernetes.io~projected/test_volume_name")) {
+	if !strings.HasSuffix(volumePath, filepath.FromSlash("pods/test_pod_uid3/volumes/kubernetes.io~projected/test_volume_name")) {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -1103,7 +1103,7 @@ func TestPluginOptional(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~projected/test_volume_name")) {
+	if !strings.HasSuffix(volumePath, filepath.FromSlash("pods/test_pod_uid/volumes/kubernetes.io~projected/test_volume_name")) {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -1201,7 +1201,7 @@ func TestPluginOptionalKeys(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~projected/test_volume_name")) {
+	if !strings.HasSuffix(volumePath, filepath.FromSlash("pods/test_pod_uid/volumes/kubernetes.io~projected/test_volume_name")) {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 

pkg/volume/quobyte/quobyte_test.go

@@ -19,6 +19,7 @@ package quobyte
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"k8s.io/mount-utils"
@@ -99,7 +100,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) {
 	}
 
 	volumePath := mounter.GetPath()
-	if volumePath != fmt.Sprintf("%s/plugins/kubernetes.io~quobyte/root#root@vol", tmpDir) {
+	if volumePath != filepath.Join(tmpDir, "plugins/kubernetes.io~quobyte/root#root@vol") {
 		t.Errorf("Got unexpected path: %s expected: %s", volumePath, fmt.Sprintf("%s/plugins/kubernetes.io~quobyte/root#root@vol", tmpDir))
 	}
 	if err := mounter.SetUp(volume.MounterArgs{}); err != nil {

pkg/volume/rbd/rbd.go

@@ -44,6 +44,7 @@ import (
 
 var (
 	supportedFeatures = sets.NewString("layering")
+	pathSeparator     = string(os.PathSeparator)
 )
 
 // ProbeVolumePlugins is the primary entrypoint for volume plugins.
@@ -846,6 +847,7 @@ func (b *rbdMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error {
 	err := diskSetUp(b.manager, *b, dir, b.mounter, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy)
 	if err != nil {
 		klog.Errorf("rbd: failed to setup at %s %v", dir, err)
+		return err
 	}
 	klog.V(3).Infof("rbd: successfully setup at %s", dir)
 	return err
@@ -948,7 +950,7 @@ type rbdDiskUnmapper struct {
 
 func getPoolAndImageFromMapPath(mapPath string) (string, string, error) {
 
-	pathParts := dstrings.Split(mapPath, "/")
+	pathParts := dstrings.Split(mapPath, pathSeparator)
 	if len(pathParts) < 2 {
 		return "", "", fmt.Errorf("corrupted mapPath")
 	}

pkg/volume/rbd/rbd_test.go

@@ -397,8 +397,8 @@ func TestPlugin(t *testing.T) {
 			},
 		},
 		expectedDevicePath:      "/dev/rbd1",
-		expectedDeviceMountPath: fmt.Sprintf("%s/plugins/kubernetes.io/rbd/mounts/pool1-image-image1", tmpDir),
+		expectedDeviceMountPath: filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/mounts/pool1-image-image1"),
-		expectedPodMountPath:    fmt.Sprintf("%s/pods/%s/volumes/kubernetes.io~rbd/vol1", tmpDir, podUID),
+		expectedPodMountPath:    filepath.Join(tmpDir, "pods", string(podUID), "volumes/kubernetes.io~rbd/vol1"),
 	})
 	cases = append(cases, &testcase{
 		spec: volume.NewSpecFromPersistentVolume(&v1.PersistentVolume{
@@ -426,8 +426,8 @@ func TestPlugin(t *testing.T) {
 			},
 		},
 		expectedDevicePath:      "/dev/rbd1",
-		expectedDeviceMountPath: fmt.Sprintf("%s/plugins/kubernetes.io/rbd/mounts/pool2-image-image2", tmpDir),
+		expectedDeviceMountPath: filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/mounts/pool2-image-image2"),
-		expectedPodMountPath:    fmt.Sprintf("%s/pods/%s/volumes/kubernetes.io~rbd/vol2", tmpDir, podUID),
+		expectedPodMountPath:    filepath.Join(tmpDir, "pods", string(podUID), "volumes/kubernetes.io~rbd/vol2"),
 	})
 
 	for i := 0; i < len(cases); i++ {
@@ -560,8 +560,8 @@ func TestGetDeviceMountPath(t *testing.T) {
 		},
 	})
 
-	deprecatedDir := fmt.Sprintf("%s/plugins/kubernetes.io/rbd/rbd/%s-image-%s", tmpDir, pool, image)
+	deprecatedDir := filepath.Join(tmpDir, fmt.Sprintf("plugins/kubernetes.io/rbd/rbd/%s-image-%s", pool, image))
-	canonicalDir := fmt.Sprintf("%s/plugins/kubernetes.io/rbd/mounts/%s-image-%s", tmpDir, pool, image)
+	canonicalDir := filepath.Join(tmpDir, fmt.Sprintf("plugins/kubernetes.io/rbd/mounts/%s-image-%s", pool, image))
 
 	type testCase struct {
 		deprecated bool
@@ -609,9 +609,9 @@ func TestConstructVolumeSpec(t *testing.T) {
 	fakeMounter := fakeVolumeHost.GetMounter(plug.GetPluginName()).(*mount.FakeMounter)
 
 	pool, image, volumeName := "pool", "image", "vol"
-	podMountPath := fmt.Sprintf("%s/pods/pod123/volumes/kubernetes.io~rbd/%s", tmpDir, volumeName)
+	podMountPath := filepath.Join(tmpDir, "pods/pod123/volumes/kubernetes.io~rbd", volumeName)
-	deprecatedDir := fmt.Sprintf("%s/plugins/kubernetes.io/rbd/rbd/%s-image-%s", tmpDir, pool, image)
+	deprecatedDir := filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/rbd", fmt.Sprintf("%s-image-%s", pool, image))
-	canonicalDir := fmt.Sprintf("%s/plugins/kubernetes.io/rbd/mounts/%s-image-%s", tmpDir, pool, image)
+	canonicalDir := filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/mounts", fmt.Sprintf("%s-image-%s", pool, image))
 
 	type testCase struct {
 		volumeName string

pkg/volume/secret/secret_test.go

@@ -263,7 +263,7 @@ func TestMakePayload(t *testing.T) {
 }
 
 func newTestHost(t *testing.T, clientset clientset.Interface) (string, volume.VolumeHost) {
-	tempDir, err := ioutil.TempDir("/tmp", "secret_volume_test.")
+	tempDir, err := ioutil.TempDir("", "secret_volume_test.")
 	if err != nil {
 		t.Fatalf("can't make a temp rootdir: %v", err)
 	}
@@ -323,7 +323,7 @@ func TestPlugin(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -397,7 +397,7 @@ func TestInvalidPathSecret(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -449,7 +449,7 @@ func TestPluginReboot(t *testing.T) {
 	podMetadataDir := fmt.Sprintf("%v/pods/test_pod_uid3/plugins/kubernetes.io~secret/test_volume_name", rootDir)
 	util.SetReady(podMetadataDir)
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid3/volumes/kubernetes.io~secret/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid3/volumes/kubernetes.io~secret/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -501,7 +501,7 @@ func TestPluginOptional(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -599,7 +599,7 @@ func TestPluginOptionalKeys(t *testing.T) {
 	}
 
 	volumePath := mounter.GetPath()
-	if !strings.HasSuffix(volumePath, fmt.Sprintf("pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name")) {
+	if !hasPathSuffix(volumePath, "pods/test_pod_uid/volumes/kubernetes.io~secret/test_volume_name") {
 		t.Errorf("Got unexpected path: %s", volumePath)
 	}
 
@@ -701,3 +701,7 @@ func doTestCleanAndTeardown(plugin volume.VolumePlugin, podUID types.UID, testVo
 		t.Errorf("TearDown() failed: %v", err)
 	}
 }
+
+func hasPathSuffix(s, suffix string) bool {
+	return strings.HasSuffix(s, filepath.FromSlash(suffix))
+}

pkg/volume/testing/testing.go

@@ -1146,7 +1146,7 @@ func (fc *FakeProvisioner) Provision(selectedNode *v1.Node, allowedTopologies []
 			return nil, fmt.Errorf("expected error")
 		}
 	}
-	fullpath := fmt.Sprintf("/tmp/hostpath_pv/%s", uuid.NewUUID())
+	fullpath := fmt.Sprintf("/%s/hostpath_pv/%s", os.TempDir(), uuid.NewUUID())
 
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{

pkg/volume/vsphere_volume/attacher_test.go

@@ -22,6 +22,8 @@ package vsphere_volume
 import (
 	"errors"
 	"fmt"
+	"os"
+	"path/filepath"
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
@@ -89,6 +91,7 @@ func TestAttachDetach(t *testing.T) {
 	diskName := "[local] volumes/test"
 	nodeName := types.NodeName("host")
 	spec := createVolSpec(diskName)
+	expectedDevice := filepath.FromSlash("/dev/disk/by-id/wwn-0x" + uuid)
 	attachError := errors.New("fake attach error")
 	detachError := errors.New("fake detach error")
 	diskCheckError := errors.New("fake DiskIsAttached error")
@@ -101,7 +104,7 @@ func TestAttachDetach(t *testing.T) {
 				attacher := newAttacher(testcase)
 				return attacher.Attach(spec, nodeName)
 			},
-			expectedDevice: "/dev/disk/by-id/wwn-0x" + uuid,
+			expectedDevice: expectedDevice,
 		},
 
 		// Attach call fails
@@ -176,7 +179,7 @@ func TestAttachDetach(t *testing.T) {
 // newPlugin creates a new vsphereVolumePlugin with fake cloud, NewAttacher
 // and NewDetacher won't work.
 func newPlugin(t *testing.T) *vsphereVolumePlugin {
-	host := volumetest.NewFakeVolumeHost(t, "/tmp", nil, nil)
+	host := volumetest.NewFakeVolumeHost(t, os.TempDir(), nil, nil)
 	plugins := ProbeVolumePlugins()
 	plugin := plugins[0]
 	plugin.Init(host)