Address CR comments and add more tests

pkg/kubelet/kubelet_pods.go

@@ -128,7 +128,6 @@ func (kl *Kubelet) makeBlockVolumes(pod *v1.Pod, container *v1.Container, podVol
 
 // makeMounts determines the mount points for the given container.
 func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap, mounter mountutil.Interface, expandEnvs []kubecontainer.EnvVar) ([]kubecontainer.Mount, func(), error) {
-
 	// Kubernetes only mounts on /etc/hosts if:
 	// - container is not an infrastructure (pause) container
 	// - container is not already mounting on /etc/hosts
@@ -216,15 +215,11 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
 		}
 
 		// Docker Volume Mounts fail on Windows if it is not of the form C:/
-		containerPath := mount.MountPath
-		if runtime.GOOS == "windows" {
-			// Append C: only if it looks like a local path. Do not process UNC path/SMB shares/named pipes
-			// NOTE: strings.HasPrefix(hostPath, "\\") checks for prefix "\" as in "\foo\bar" and not "\\foo\bar"
-			// IsWindowsUNCPath checks for prefix "\\" as in "\\foo\bar" and not "\foo\bar"
-			if (strings.HasPrefix(hostPath, "/") || strings.HasPrefix(hostPath, "\\")) && !strings.Contains(hostPath, ":") && !volumeutil.IsWindowsUNCPath(runtime.GOOS, hostPath) {
-				hostPath = "c:" + hostPath
-			}
+		if volumeutil.IsWindowsLocalPath(runtime.GOOS, hostPath) {
+			hostPath = "c:" + hostPath
 		}
+
+		containerPath := mount.MountPath
 		// IsAbs returns false for UNC path/SMB shares/named pipes in Windows. So check for those specifically and skip MakeAbsolutePath
 		if !volumeutil.IsWindowsUNCPath(runtime.GOOS, containerPath) && !filepath.IsAbs(containerPath) {
 			containerPath = volumeutil.MakeAbsolutePath(runtime.GOOS, containerPath)

pkg/kubelet/kubelet_pods_windows_test.go

-// +build windows
-
 /*
 Copyright 2017 The Kubernetes Authors.
 
@@ -50,6 +48,21 @@ func TestMakeMountsWindows(t *testing.T) {
 				Name:      "disk5",
 				ReadOnly:  false,
 			},
+			{
+				MountPath: `\mnt\path6`,
+				Name:      "disk6",
+				ReadOnly:  false,
+			},
+			{
+				MountPath: `/mnt/path7`,
+				Name:      "disk7",
+				ReadOnly:  false,
+			},
+			{
+				MountPath: `\\.\pipe\pipe1`,
+				Name:      "pipe1",
+				ReadOnly:  false,
+			},
 		},
 	}
 
@@ -57,6 +70,9 @@ func TestMakeMountsWindows(t *testing.T) {
 		"disk":  kubecontainer.VolumeInfo{Mounter: &stubVolume{path: "c:/mnt/disk"}},
 		"disk4": kubecontainer.VolumeInfo{Mounter: &stubVolume{path: "c:/mnt/host"}},
 		"disk5": kubecontainer.VolumeInfo{Mounter: &stubVolume{path: "c:/var/lib/kubelet/podID/volumes/empty/disk5"}},
+		"disk6": kubecontainer.VolumeInfo{Mounter: &stubVolume{path: `/mnt/disk6`}},
+		"disk7": kubecontainer.VolumeInfo{Mounter: &stubVolume{path: `\mnt\disk7`}},
+		"pipe1": kubecontainer.VolumeInfo{Mounter: &stubVolume{path: `\\.\pipe\pipe1`}},
 	}
 
 	pod := v1.Pod{
@@ -97,6 +113,27 @@ func TestMakeMountsWindows(t *testing.T) {
 			ReadOnly:       false,
 			SELinuxRelabel: false,
 		},
+		{
+			Name:           "disk6",
+			ContainerPath:  `c:\mnt\path6`,
+			HostPath:       `c:/mnt/disk6`,
+			ReadOnly:       false,
+			SELinuxRelabel: false,
+		},
+		{
+			Name:           "disk7",
+			ContainerPath:  `c:/mnt/path7`,
+			HostPath:       `c:\mnt\disk7`,
+			ReadOnly:       false,
+			SELinuxRelabel: false,
+		},
+		{
+			Name:           "pipe1",
+			ContainerPath:  `\\.\pipe\pipe1`,
+			HostPath:       `\\.\pipe\pipe1`,
+			ReadOnly:       false,
+			SELinuxRelabel: false,
+		},
 	}
 	assert.Equal(t, expectedMounts, mounts, "mounts of container %+v", container)
 }

pkg/volume/util/util.go

@@ -959,6 +959,24 @@ func IsWindowsUNCPath(goos, path string) bool {
 	return false
 }
 
+// IsWindowsLocalPath checks if path is a local path
+// prefixed with "/" or "\" like "/foo/bar" or "\foo\bar"
+func IsWindowsLocalPath(goos, path string) bool {
+	if goos != "windows" {
+		return false
+	}
+	if IsWindowsUNCPath(goos, path) {
+		return false
+	}
+	if strings.Contains(path, ":") {
+		return false
+	}
+	if !(strings.HasPrefix(path, `/`) || strings.HasPrefix(path, `\`)) {
+		return false
+	}
+	return true
+}
+
 // MakeAbsolutePath convert path to absolute path according to GOOS
 func MakeAbsolutePath(goos, path string) string {
 	if goos != "windows" {

pkg/volume/util/util_test.go

@@ -2424,6 +2424,97 @@ func TestIsWindowsUNCPath(t *testing.T) {
 	}
 }
 
+func TestIsWindowsLocalPath(t *testing.T) {
+	tests := []struct {
+		goos               string
+		path               string
+		isWindowsLocalPath bool
+	}{
+		{
+			goos:               "linux",
+			path:               `/usr/bin`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "linux",
+			path:               `\\.\pipe\foo`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `C:\foo`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `:\foo`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `X:\foo`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `\\server\share\foo`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `\\?\server\share`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `\\?\c:\`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `\\.\pipe\valid_pipe`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `foo`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `:foo`,
+			isWindowsLocalPath: false,
+		},
+		{
+			goos:               "windows",
+			path:               `\foo`,
+			isWindowsLocalPath: true,
+		},
+		{
+			goos:               "windows",
+			path:               `\foo\bar`,
+			isWindowsLocalPath: true,
+		},
+		{
+			goos:               "windows",
+			path:               `/foo`,
+			isWindowsLocalPath: true,
+		},
+		{
+			goos:               "windows",
+			path:               `/foo/bar`,
+			isWindowsLocalPath: true,
+		},
+	}
+
+	for _, test := range tests {
+		result := IsWindowsLocalPath(test.goos, test.path)
+		if result != test.isWindowsLocalPath {
+			t.Errorf("isWindowsLocalPath(%v) returned (%v), expected (%v)", test.path, result, test.isWindowsLocalPath)
+		}
+	}
+}
+
 func TestMakeAbsolutePath(t *testing.T) {
 	tests := []struct {
 		goos         string