Skip to content

K
k3s
Commit eb7c36ee authored by andyzhangx's avatar andyzhangx
Browse files
Options

remove IsAbs validation on local volume

use MakeAbsolutePath to convert path in Windows fix test error: allow relative path for local volume fix comments fix comments and add windows unit tests
parent baab3992
Hide whitespace changes
Inline Side-by-side
Showing with 165 additions and 118 deletions
pkg/apis/core/validation/validation.go
View file @ eb7c36ee
......@@ -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
}
......
pkg/apis/core/validation/validation_test.go
View file @ eb7c36ee
......@@ -631,8 +631,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"))),
},
......
pkg/kubelet/kubelet_pods.go
View file @ eb7c36ee
......@@ -110,23 +110,6 @@ func (kl *Kubelet) makeGPUDevices(pod *v1.Pod, container *v1.Container) ([]kubec
return devices, nil
}
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) {
......@@ -255,7 +238,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)
......
pkg/kubelet/kubelet_pods_test.go
View file @ eb7c36ee
......@@ -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
......
pkg/volume/local/BUILD
View file @ eb7c36ee
......@@ -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": [],
}),
)
......
pkg/volume/local/local.go
View file @ eb7c36ee
......@@ -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.
......
pkg/volume/local/local_linux_test.go 0 → 100644
View file @ eb7c36ee
// +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))
}
}
pkg/volume/local/local_test.go
View file @ eb7c36ee
// +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)
......
pkg/volume/util/util.go
View file @ eb7c36ee
......@@ -21,6 +21,7 @@ import (
"io/ioutil"
"os"
"path"
"path/filepath"
"strings"
"syscall"
......@@ -749,3 +750,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
}
pkg/volume/util/util_test.go
View file @ eb7c36ee
......@@ -1089,3 +1089,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)
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment