diff --git a/pkg/volume/awsebs/attacher_test.go b/pkg/volume/awsebs/attacher_test.go index 72414f9a793..9dd44e09799 100644 --- a/pkg/volume/awsebs/attacher_test.go +++ b/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) diff --git a/pkg/volume/awsebs/aws_ebs_block.go b/pkg/volume/awsebs/aws_ebs_block.go index d924aedcc2f..2eaab4e46f2 100644 --- a/pkg/volume/awsebs/aws_ebs_block.go +++ b/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) diff --git a/pkg/volume/awsebs/aws_ebs_test.go b/pkg/volume/awsebs/aws_ebs_test.go index 1ac8fcacd65..9ffb080a61c 100644 --- a/pkg/volume/awsebs/aws_ebs_test.go +++ b/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) { - t.Errorf("SetUp() failed, volume path not created: %s", path) - } else { - t.Errorf("SetUp() failed: %v", err) + + // On Windows, Mount will create the parent of dir and mklink (create a symbolic link) at the volume path later, + // so mounter.SetUp will not create the directory. Otherwise mklink will error: "Cannot create a file when that file already exists". + if goruntime.GOOS != "windows" { + 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) + } } } diff --git a/pkg/volume/azure_file/azure_file_test.go b/pkg/volume/azure_file/azure_file_test.go index a1546621ca8..770965ffc9c 100644 --- a/pkg/volume/azure_file/azure_file_test.go +++ b/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 { - if os.IsNotExist(err) { - t.Errorf("SetUp() failed, volume path not created: %s", path) - } else { - t.Errorf("SetUp() failed: %v", err) + // On Windows, Mount will create the parent of dir and mklink (create a symbolic link) at the volume path later, + // so mounter.SetUp will not create the directory. Otherwise mklink will error: "Cannot create a file when that file already exists". + if goruntime.GOOS != "windows" { + 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) + } } } @@ -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 diff --git a/pkg/volume/cinder/attacher_test.go b/pkg/volume/cinder/attacher_test.go index dd884d72b68..f5b3ab0ec5e 100644 --- a/pkg/volume/cinder/attacher_test.go +++ b/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) diff --git a/pkg/volume/configmap/configmap_test.go b/pkg/volume/configmap/configmap_test.go index 89c6ec01e38..7f793e13a40 100644 --- a/pkg/volume/configmap/configmap_test.go +++ b/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)) +} diff --git a/pkg/volume/csi/csi_attacher_test.go b/pkg/volume/csi/csi_attacher_test.go index 074535cf870..26270946a9f 100644 --- a/pkg/volume/csi/csi_attacher_test.go +++ b/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) } diff --git a/pkg/volume/csi/csi_mounter_test.go b/pkg/volume/csi/csi_mounter_test.go index e30697c6117..e8f5c5fea38 100644 --- a/pkg/volume/csi/csi_mounter_test.go +++ b/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, diff --git a/pkg/volume/csi/csi_util_test.go b/pkg/volume/csi/csi_util_test.go index 04d66408f5a..f287ce19623 100644 --- a/pkg/volume/csi/csi_util_test.go +++ b/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) diff --git a/pkg/volume/fc/fc_test.go b/pkg/volume/fc/fc_test.go index e95af37fbf5..4f352a73b87 100644 --- a/pkg/volume/fc/fc_test.go +++ b/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) } diff --git a/pkg/volume/flexvolume/common_test.go b/pkg/volume/flexvolume/common_test.go index 60a339ba6a2..7fd196a0f74 100644 --- a/pkg/volume/flexvolume/common_test.go +++ b/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 { diff --git a/pkg/volume/flexvolume/flexvolume_test.go b/pkg/volume/flexvolume/flexvolume_test.go index d6faa9adc22..7c64fb0ac9b 100644 --- a/pkg/volume/flexvolume/flexvolume_test.go +++ b/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) } diff --git a/pkg/volume/flexvolume/probe_test.go b/pkg/volume/flexvolume/probe_test.go index 0462a16c322..e79c4aaa70a 100644 --- a/pkg/volume/flexvolume/probe_test.go +++ b/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. diff --git a/pkg/volume/gcepd/attacher_test.go b/pkg/volume/gcepd/attacher_test.go index 3e7af5b7ee9..6dd54a68d68 100644 --- a/pkg/volume/gcepd/attacher_test.go +++ b/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" { - return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath) + 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 }, @@ -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 { - 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 - }, + test: attachTestFunc, }, // Disk is already attached { name: "Attach_Positive_AlreadyAttached", diskIsAttached: diskIsAttachedCall{disksAttachedMap{nodeName: {diskName}}, nil}, - test: func(testcase *testcase) error { - 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 - }, + test: attachTestFunc, }, // 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 { - 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 - }, + test: attachTestFunc, }, // 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 */ - nil, /* kubeClient */ - nil, /* plugins */ + os.TempDir(), /* rootDir */ + nil, /* kubeClient */ + nil, /* plugins */ ) plugins := ProbeVolumePlugins() plugin := plugins[0] diff --git a/pkg/volume/gcepd/gce_pd_test.go b/pkg/volume/gcepd/gce_pd_test.go index 4ad7aec7b2d..723ba55c7fd 100644 --- a/pkg/volume/gcepd/gce_pd_test.go +++ b/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 { - if os.IsNotExist(err) { - t.Errorf("SetUp() failed, volume path not created: %s", path) - } else { - t.Errorf("SetUp() failed: %v", err) + // On Windows, Mount will create the parent of dir and mklink (create a symbolic link) at the volume path later, + // so mounter.SetUp will not create the directory. Otherwise mklink will error: "Cannot create a file when that file already exists". + if goruntime.GOOS != "windows" { + 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) + } } } diff --git a/pkg/volume/git_repo/git_repo_test.go b/pkg/volume/git_repo/git_repo_test.go index 3b855239dc3..b9104c9aecd 100644 --- a/pkg/volume/git_repo/git_repo_test.go +++ b/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, diff --git a/pkg/volume/glusterfs/glusterfs_test.go b/pkg/volume/glusterfs/glusterfs_test.go index 43ca2b14002..4b440636077 100644 --- a/pkg/volume/glusterfs/glusterfs_test.go +++ b/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) } diff --git a/pkg/volume/iscsi/iscsi_test.go b/pkg/volume/iscsi/iscsi_test.go index f4b34454b0b..7fd675e55f3 100644 --- a/pkg/volume/iscsi/iscsi_test.go +++ b/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) } diff --git a/pkg/volume/nfs/nfs_test.go b/pkg/volume/nfs/nfs_test.go index b07a00589bc..47e87fd543a 100644 --- a/pkg/volume/nfs/nfs_test.go +++ b/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) } diff --git a/pkg/volume/projected/projected_test.go b/pkg/volume/projected/projected_test.go index f88f3456cca..ac24859d167 100644 --- a/pkg/volume/projected/projected_test.go +++ b/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) } diff --git a/pkg/volume/quobyte/quobyte_test.go b/pkg/volume/quobyte/quobyte_test.go index 3842373ce34..a822232263e 100644 --- a/pkg/volume/quobyte/quobyte_test.go +++ b/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 { diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index dcfb0880442..85f949a5b55 100644 --- a/pkg/volume/rbd/rbd.go +++ b/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") } diff --git a/pkg/volume/rbd/rbd_test.go b/pkg/volume/rbd/rbd_test.go index ff9b7c308ce..91cceaba3a3 100644 --- a/pkg/volume/rbd/rbd_test.go +++ b/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), - expectedPodMountPath: fmt.Sprintf("%s/pods/%s/volumes/kubernetes.io~rbd/vol1", tmpDir, podUID), + expectedDeviceMountPath: filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/mounts/pool1-image-image1"), + 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), - expectedPodMountPath: fmt.Sprintf("%s/pods/%s/volumes/kubernetes.io~rbd/vol2", tmpDir, podUID), + expectedDeviceMountPath: filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/mounts/pool2-image-image2"), + 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) - canonicalDir := fmt.Sprintf("%s/plugins/kubernetes.io/rbd/mounts/%s-image-%s", tmpDir, pool, image) + deprecatedDir := filepath.Join(tmpDir, fmt.Sprintf("plugins/kubernetes.io/rbd/rbd/%s-image-%s", 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) - deprecatedDir := fmt.Sprintf("%s/plugins/kubernetes.io/rbd/rbd/%s-image-%s", tmpDir, pool, image) - canonicalDir := fmt.Sprintf("%s/plugins/kubernetes.io/rbd/mounts/%s-image-%s", tmpDir, pool, image) + podMountPath := filepath.Join(tmpDir, "pods/pod123/volumes/kubernetes.io~rbd", volumeName) + deprecatedDir := filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/rbd", fmt.Sprintf("%s-image-%s", pool, image)) + canonicalDir := filepath.Join(tmpDir, "plugins/kubernetes.io/rbd/mounts", fmt.Sprintf("%s-image-%s", pool, image)) type testCase struct { volumeName string diff --git a/pkg/volume/secret/secret_test.go b/pkg/volume/secret/secret_test.go index be7b65f6816..808e5751283 100644 --- a/pkg/volume/secret/secret_test.go +++ b/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)) +} diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 4f5d1c2f0c1..12882a16f0d 100644 --- a/pkg/volume/testing/testing.go +++ b/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{ diff --git a/pkg/volume/vsphere_volume/attacher_test.go b/pkg/volume/vsphere_volume/attacher_test.go index b89034acc1e..fc3f0c2b0c0 100644 --- a/pkg/volume/vsphere_volume/attacher_test.go +++ b/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)