diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 34e66c09418..fd3d63a3482 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1383,9 +1383,6 @@ func validateLocalVolumeSource(ls *core.LocalVolumeSource, fldPath *field.Path) return allErrs } - if !path.IsAbs(ls.Path) { - allErrs = append(allErrs, field.Invalid(fldPath, ls.Path, "must be an absolute path")) - } allErrs = append(allErrs, validatePathNoBacksteps(ls.Path, fldPath.Child("path"))...) return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 2cbc8a58750..2fac66f74a7 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -521,8 +521,8 @@ func TestValidateLocalVolumes(t *testing.T) { volume: testVolume("foo", "", testLocalVolume("/foo/..", simpleVolumeNodeAffinity("foo", "bar"))), }, - "invalid-local-volume-relative-path": { - isExpectedFailure: true, + "valid-local-volume-relative-path": { + isExpectedFailure: false, volume: testVolume("foo", "", testLocalVolume("foo", simpleVolumeNodeAffinity("foo", "bar"))), }, diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index c1beb58c405..e204af1d4cc 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -90,23 +90,6 @@ func (kl *Kubelet) GetActivePods() []*v1.Pod { return activePods } -func makeAbsolutePath(goos, path string) string { - if goos != "windows" { - return "/" + path - } - // These are all for windows - // If there is a colon, give up. - if strings.Contains(path, ":") { - return path - } - // If there is a slash, but no drive, add 'c:' - if strings.HasPrefix(path, "/") || strings.HasPrefix(path, "\\") { - return "c:" + path - } - // Otherwise, add 'c:\' - return "c:\\" + path -} - // makeBlockVolumes maps the raw block devices specified in the path of the container // Experimental func (kl *Kubelet) makeBlockVolumes(pod *v1.Pod, container *v1.Container, podVolumes kubecontainer.VolumeMap, blkutil volumepathhandler.BlockVolumePathHandler) ([]kubecontainer.DeviceInfo, error) { @@ -235,7 +218,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h } } if !filepath.IsAbs(containerPath) { - containerPath = makeAbsolutePath(runtime.GOOS, containerPath) + containerPath = volumeutil.MakeAbsolutePath(runtime.GOOS, containerPath) } propagation, err := translateMountPropagation(mount.MountPropagation) diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 92552f5227b..036b2fe3240 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -51,52 +51,6 @@ import ( volumetest "k8s.io/kubernetes/pkg/volume/testing" ) -func TestMakeAbsolutePath(t *testing.T) { - tests := []struct { - goos string - path string - expectedPath string - name string - }{ - { - goos: "linux", - path: "non-absolute/path", - expectedPath: "/non-absolute/path", - name: "basic linux", - }, - { - goos: "windows", - path: "some\\path", - expectedPath: "c:\\some\\path", - name: "basic windows", - }, - { - goos: "windows", - path: "/some/path", - expectedPath: "c:/some/path", - name: "linux path on windows", - }, - { - goos: "windows", - path: "\\some\\path", - expectedPath: "c:\\some\\path", - name: "windows path no drive", - }, - { - goos: "windows", - path: "\\:\\some\\path", - expectedPath: "\\:\\some\\path", - name: "windows path with colon", - }, - } - for _, test := range tests { - path := makeAbsolutePath(test.goos, test.path) - if path != test.expectedPath { - t.Errorf("[%s] Expected %s saw %s", test.name, test.expectedPath, path) - } - } -} - func TestMakeMounts(t *testing.T) { bTrue := true propagationHostToContainer := v1.MountPropagationHostToContainer diff --git a/pkg/volume/local/BUILD b/pkg/volume/local/BUILD index 8710d93a670..c20da41347a 100644 --- a/pkg/volume/local/BUILD +++ b/pkg/volume/local/BUILD @@ -32,6 +32,10 @@ go_test( "local_test.go", ], "@io_bazel_rules_go//go/platform:linux": [ + "local_linux_test.go", + "local_test.go", + ], + "@io_bazel_rules_go//go/platform:windows": [ "local_test.go", ], "//conditions:default": [], @@ -54,6 +58,14 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/client-go/util/testing:go_default_library", ], + "@io_bazel_rules_go//go/platform:windows": [ + "//pkg/volume:go_default_library", + "//pkg/volume/testing:go_default_library", + "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", + "//vendor/k8s.io/client-go/util/testing:go_default_library", + ], "//conditions:default": [], }), ) diff --git a/pkg/volume/local/local.go b/pkg/volume/local/local.go index 5f6916eb485..aa84a329384 100644 --- a/pkg/volume/local/local.go +++ b/pkg/volume/local/local.go @@ -311,7 +311,8 @@ func (m *localVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { } glog.V(4).Infof("attempting to mount %s", dir) - err = m.mounter.Mount(m.globalPath, dir, "", options) + globalPath := util.MakeAbsolutePath(runtime.GOOS, m.globalPath) + err = m.mounter.Mount(globalPath, dir, "", options) if err != nil { glog.Errorf("Mount of volume %s failed: %v", dir, err) notMnt, mntErr := m.mounter.IsNotMountPoint(dir) @@ -374,8 +375,9 @@ var _ volume.BlockVolumeMapper = &localVolumeMapper{} // SetUpDevice provides physical device path for the local PV. func (m *localVolumeMapper) SetUpDevice() (string, error) { - glog.V(4).Infof("SetupDevice returning path %s", m.globalPath) - return m.globalPath, nil + globalPath := util.MakeAbsolutePath(runtime.GOOS, m.globalPath) + glog.V(4).Infof("SetupDevice returning path %s", globalPath) + return globalPath, nil } // localVolumeUnmapper implements the BlockVolumeUnmapper interface for local volumes. diff --git a/pkg/volume/local/local_linux_test.go b/pkg/volume/local/local_linux_test.go new file mode 100644 index 00000000000..1ec7a56d7d6 --- /dev/null +++ b/pkg/volume/local/local_linux_test.go @@ -0,0 +1,68 @@ +// +build linux darwin + +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package local + +import ( + "os" + "syscall" + "testing" + + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +func TestFSGroupMount(t *testing.T) { + tmpDir, plug := getPlugin(t) + defer os.RemoveAll(tmpDir) + info, err := os.Stat(tmpDir) + if err != nil { + t.Errorf("Error getting stats for %s (%v)", tmpDir, err) + } + s := info.Sys().(*syscall.Stat_t) + if s == nil { + t.Errorf("Error getting stats for %s (%v)", tmpDir, err) + } + fsGroup1 := int64(s.Gid) + fsGroup2 := fsGroup1 + 1 + pod1 := &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}} + pod1.Spec.SecurityContext = &v1.PodSecurityContext{ + FSGroup: &fsGroup1, + } + pod2 := &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}} + pod2.Spec.SecurityContext = &v1.PodSecurityContext{ + FSGroup: &fsGroup2, + } + err = testFSGroupMount(plug, pod1, tmpDir, fsGroup1) + if err != nil { + t.Errorf("Failed to make a new Mounter: %v", err) + } + err = testFSGroupMount(plug, pod2, tmpDir, fsGroup2) + if err != nil { + t.Errorf("Failed to make a new Mounter: %v", err) + } + //Checking if GID of tmpDir has not been changed by mounting it by second pod + s = info.Sys().(*syscall.Stat_t) + if s == nil { + t.Errorf("Error getting stats for %s (%v)", tmpDir, err) + } + if fsGroup1 != int64(s.Gid) { + t.Errorf("Old Gid %d for volume %s got overwritten by new Gid %d", fsGroup1, tmpDir, int64(s.Gid)) + } +} diff --git a/pkg/volume/local/local_test.go b/pkg/volume/local/local_test.go index 590f2de76f6..58ab8623bf7 100644 --- a/pkg/volume/local/local_test.go +++ b/pkg/volume/local/local_test.go @@ -1,4 +1,4 @@ -// +build linux darwin +// +build linux darwin windows /* Copyright 2017 The Kubernetes Authors. @@ -22,7 +22,7 @@ import ( "fmt" "os" "path" - "syscall" + "runtime" "testing" "k8s.io/api/core/v1" @@ -199,11 +199,15 @@ func TestMountUnmount(t *testing.T) { if err := mounter.SetUp(nil); 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) + + if runtime.GOOS != "windows" { + // skip this check in windows since the "bind mount" logic is implemented differently in mount_wiondows.go + 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) + } } } @@ -260,6 +264,7 @@ func TestMapUnmap(t *testing.T) { if err != nil { t.Errorf("Failed to SetUpDevice, err: %v", err) } + if _, err := os.Stat(devPath); err != nil { if os.IsNotExist(err) { t.Errorf("SetUpDevice() failed, volume path not created: %s", devPath) @@ -302,45 +307,6 @@ func testFSGroupMount(plug volume.VolumePlugin, pod *v1.Pod, tmpDir string, fsGr return nil } -func TestFSGroupMount(t *testing.T) { - tmpDir, plug := getPlugin(t) - defer os.RemoveAll(tmpDir) - info, err := os.Stat(tmpDir) - if err != nil { - t.Errorf("Error getting stats for %s (%v)", tmpDir, err) - } - s := info.Sys().(*syscall.Stat_t) - if s == nil { - t.Errorf("Error getting stats for %s (%v)", tmpDir, err) - } - fsGroup1 := int64(s.Gid) - fsGroup2 := fsGroup1 + 1 - pod1 := &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}} - pod1.Spec.SecurityContext = &v1.PodSecurityContext{ - FSGroup: &fsGroup1, - } - pod2 := &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}} - pod2.Spec.SecurityContext = &v1.PodSecurityContext{ - FSGroup: &fsGroup2, - } - err = testFSGroupMount(plug, pod1, tmpDir, fsGroup1) - if err != nil { - t.Errorf("Failed to make a new Mounter: %v", err) - } - err = testFSGroupMount(plug, pod2, tmpDir, fsGroup2) - if err != nil { - t.Errorf("Failed to make a new Mounter: %v", err) - } - //Checking if GID of tmpDir has not been changed by mounting it by second pod - s = info.Sys().(*syscall.Stat_t) - if s == nil { - t.Errorf("Error getting stats for %s (%v)", tmpDir, err) - } - if fsGroup1 != int64(s.Gid) { - t.Errorf("Old Gid %d for volume %s got overwritten by new Gid %d", fsGroup1, tmpDir, int64(s.Gid)) - } -} - func TestConstructVolumeSpec(t *testing.T) { tmpDir, plug := getPlugin(t) defer os.RemoveAll(tmpDir) diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 05959589b15..87a9b2f6782 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path" + "path/filepath" "strings" "syscall" @@ -721,3 +722,21 @@ func CheckVolumeModeFilesystem(volumeSpec *volume.Spec) (bool, error) { } return true, nil } + +// MakeAbsolutePath convert path to absolute path according to GOOS +func MakeAbsolutePath(goos, path string) string { + if goos != "windows" { + return "/" + filepath.Clean(path) + } + // These are all for windows + // If there is a colon, give up. + if strings.Contains(path, ":") { + return path + } + // If there is a slash, but no drive, add 'c:' + if strings.HasPrefix(path, "/") || strings.HasPrefix(path, "\\") { + return "c:" + path + } + // Otherwise, add 'c:\' + return "c:\\" + path +} diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index 5809211d5c0..2b9c39e6525 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -976,3 +976,49 @@ func TestGetWindowsPath(t *testing.T) { } } } + +func TestMakeAbsolutePath(t *testing.T) { + tests := []struct { + goos string + path string + expectedPath string + name string + }{ + { + goos: "linux", + path: "non-absolute/path", + expectedPath: "/non-absolute/path", + name: "basic linux", + }, + { + goos: "windows", + path: "some\\path", + expectedPath: "c:\\some\\path", + name: "basic windows", + }, + { + goos: "windows", + path: "/some/path", + expectedPath: "c:/some/path", + name: "linux path on windows", + }, + { + goos: "windows", + path: "\\some\\path", + expectedPath: "c:\\some\\path", + name: "windows path no drive", + }, + { + goos: "windows", + path: "\\:\\some\\path", + expectedPath: "\\:\\some\\path", + name: "windows path with colon", + }, + } + for _, test := range tests { + path := MakeAbsolutePath(test.goos, test.path) + if path != test.expectedPath { + t.Errorf("[%s] Expected %s saw %s", test.name, test.expectedPath, path) + } + } +}