diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index b34d977da0c..cc57be3184d 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -1138,12 +1138,12 @@ type FakeVolumePathHandler struct { sync.RWMutex } -func (fv *FakeVolumePathHandler) MapDevice(devicePath string, mapDir string, linkName string) error { +func (fv *FakeVolumePathHandler) MapDevice(devicePath string, mapDir string, linkName string, bindMount bool) error { // nil is success, else error return nil } -func (fv *FakeVolumePathHandler) UnmapDevice(mapDir string, linkName string) error { +func (fv *FakeVolumePathHandler) UnmapDevice(mapDir string, linkName string, bindMount bool) error { // nil is success, else error return nil } @@ -1158,7 +1158,12 @@ func (fv *FakeVolumePathHandler) IsSymlinkExist(mapPath string) (bool, error) { return true, nil } -func (fv *FakeVolumePathHandler) GetDeviceSymlinkRefs(devPath string, mapPath string) ([]string, error) { +func (fv *FakeVolumePathHandler) IsBindMountExist(mapPath string) (bool, error) { + // nil is success, else error + return true, nil +} + +func (fv *FakeVolumePathHandler) GetDeviceBindMountRefs(devPath string, mapPath string) ([]string, error) { // nil is success, else error return []string{}, nil } diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 54f81649bbe..9d69b683161 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -1210,7 +1210,7 @@ func (og *operationGenerator) GenerateUnmapVolumeFunc( // Try to unmap volumeName symlink under pod device map path dir // pods/{podUid}/volumeDevices/{escapeQualifiedPluginName}/{volumeName} podDeviceUnmapPath, volName := blockVolumeUnmapper.GetPodDeviceMapPath() - unmapDeviceErr := og.blkUtil.UnmapDevice(podDeviceUnmapPath, volName) + unmapDeviceErr := og.blkUtil.UnmapDevice(podDeviceUnmapPath, volName, false /* bindMount */) if unmapDeviceErr != nil { // On failure, return error. Caller will log and retry. return volumeToUnmount.GenerateError("UnmapVolume.UnmapDevice on pod device map path failed", unmapDeviceErr) @@ -1218,7 +1218,7 @@ func (og *operationGenerator) GenerateUnmapVolumeFunc( // Try to unmap podUID symlink under global map path dir // plugins/kubernetes.io/{PluginName}/volumeDevices/{volumePluginDependentPath}/{podUID} globalUnmapPath := volumeToUnmount.DeviceMountPath - unmapDeviceErr = og.blkUtil.UnmapDevice(globalUnmapPath, string(volumeToUnmount.PodUID)) + unmapDeviceErr = og.blkUtil.UnmapDevice(globalUnmapPath, string(volumeToUnmount.PodUID), true /* bindMount */) if unmapDeviceErr != nil { // On failure, return error. Caller will log and retry. return volumeToUnmount.GenerateError("UnmapVolume.UnmapDevice on global map path failed", unmapDeviceErr) @@ -1306,9 +1306,9 @@ func (og *operationGenerator) GenerateUnmapDeviceFunc( // Search under globalMapPath dir if all symbolic links from pods have been removed already. // If symbolic links are there, pods may still refer the volume. globalMapPath := deviceToDetach.DeviceMountPath - refs, err := og.blkUtil.GetDeviceSymlinkRefs(deviceToDetach.DevicePath, globalMapPath) + refs, err := og.blkUtil.GetDeviceBindMountRefs(deviceToDetach.DevicePath, globalMapPath) if err != nil { - return deviceToDetach.GenerateError("UnmapDevice.GetDeviceSymlinkRefs check failed", err) + return deviceToDetach.GenerateError("UnmapDevice.GetDeviceBindMountRefs check failed", err) } if len(refs) > 0 { err = fmt.Errorf("The device %q is still referenced from other Pods %v", globalMapPath, refs) diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 42fea454baf..f097edaa58f 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -515,16 +515,18 @@ func MapBlockVolume( ) error { blkUtil := volumepathhandler.NewBlockVolumePathHandler() - // map devicePath to global node path - mapErr := blkUtil.MapDevice(devicePath, globalMapPath, string(podUID)) + // map devicePath to global node path as bind mount + mapErr := blkUtil.MapDevice(devicePath, globalMapPath, string(podUID), true /* bindMount */) if mapErr != nil { - return mapErr + return fmt.Errorf("blkUtil.MapDevice failed. devicePath: %s, globalMapPath:%s, podUID: %s, bindMount: %v: %v", + devicePath, globalMapPath, string(podUID), true, mapErr) } // map devicePath to pod volume path - mapErr = blkUtil.MapDevice(devicePath, podVolumeMapPath, volumeMapName) + mapErr = blkUtil.MapDevice(devicePath, podVolumeMapPath, volumeMapName, false /* bindMount */) if mapErr != nil { - return mapErr + return fmt.Errorf("blkUtil.MapDevice failed. devicePath: %s, podVolumeMapPath:%s, volumeMapName: %s, bindMount: %v: %v", + devicePath, podVolumeMapPath, volumeMapName, false, mapErr) } return nil diff --git a/pkg/volume/util/volumepathhandler/BUILD b/pkg/volume/util/volumepathhandler/BUILD index e344849a517..d1d562a8108 100644 --- a/pkg/volume/util/volumepathhandler/BUILD +++ b/pkg/volume/util/volumepathhandler/BUILD @@ -10,6 +10,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/volume/util/volumepathhandler", visibility = ["//visibility:public"], deps = [ + "//pkg/util/mount:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], diff --git a/pkg/volume/util/volumepathhandler/volume_path_handler.go b/pkg/volume/util/volumepathhandler/volume_path_handler.go index a05db3e25b5..d245e558925 100644 --- a/pkg/volume/util/volumepathhandler/volume_path_handler.go +++ b/pkg/volume/util/volumepathhandler/volume_path_handler.go @@ -23,12 +23,14 @@ import ( "path/filepath" "k8s.io/klog" + "k8s.io/kubernetes/pkg/util/mount" "k8s.io/apimachinery/pkg/types" ) const ( losetupPath = "losetup" + statPath = "stat" ErrDeviceNotFound = "device not found" ErrDeviceNotSupported = "device not supported" ) @@ -36,15 +38,17 @@ const ( // BlockVolumePathHandler defines a set of operations for handling block volume-related operations type BlockVolumePathHandler interface { // MapDevice creates a symbolic link to block device under specified map path - MapDevice(devicePath string, mapPath string, linkName string) error + MapDevice(devicePath string, mapPath string, linkName string, bindMount bool) error // UnmapDevice removes a symbolic link to block device under specified map path - UnmapDevice(mapPath string, linkName string) error + UnmapDevice(mapPath string, linkName string, bindMount bool) error // RemovePath removes a file or directory on specified map path RemoveMapPath(mapPath string) error // IsSymlinkExist retruns true if specified symbolic link exists IsSymlinkExist(mapPath string) (bool, error) - // GetDeviceSymlinkRefs searches symbolic links under global map path - GetDeviceSymlinkRefs(devPath string, mapPath string) ([]string, error) + // IsBindMountExist retruns true if specified bind mount exists + IsBindMountExist(mapPath string) (bool, error) + // GetDeviceBindMountRefs searches bind mounts under global map path + GetDeviceBindMountRefs(devPath string, mapPath string) ([]string, error) // FindGlobalMapPathUUIDFromPod finds {pod uuid} symbolic link under globalMapPath // corresponding to map path symlink, and then return global map path with pod uuid. FindGlobalMapPathUUIDFromPod(pluginDir, mapPath string, podUID types.UID) (string, error) @@ -68,7 +72,7 @@ type VolumePathHandler struct { } // MapDevice creates a symbolic link to block device under specified map path -func (v VolumePathHandler) MapDevice(devicePath string, mapPath string, linkName string) error { +func (v VolumePathHandler) MapDevice(devicePath string, mapPath string, linkName string, bindMount bool) error { // Example of global map path: // globalMapPath/linkName: plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumePluginDependentPath}/{podUid} // linkName: {podUid} @@ -98,25 +102,105 @@ func (v VolumePathHandler) MapDevice(devicePath string, mapPath string, linkName if err = os.MkdirAll(mapPath, 0750); err != nil { return fmt.Errorf("Failed to mkdir %s, error %v", mapPath, err) } + + if bindMount { + return mapBindMountDevice(v, devicePath, mapPath, linkName) + } + return mapSymlinkDevice(v, devicePath, mapPath, linkName) +} + +func mapBindMountDevice(v VolumePathHandler, devicePath string, mapPath string, linkName string) error { + // Check bind mount exists + linkPath := filepath.Join(mapPath, string(linkName)) + + file, err := os.Stat(linkPath) + if err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("Failed to stat file %s: %v", linkPath, err) + } + + klog.Warningf("Warning: Path to bind mount %v has not yet been created", linkPath) + // Create file + newFile, err := os.OpenFile(linkPath, os.O_CREATE|os.O_RDWR, 0750) + if err != nil { + return err + } + if err := newFile.Close(); err != nil { + return err + } + } else { + // Check if device file + // TODO: Need to check if this device file is actually the expected bind mount + if file.Mode()&os.ModeDevice == os.ModeDevice { + klog.Warningf("Warning: Map skipped because bind mount already exist on the path: %v", linkPath) + return nil + } + + klog.Warningf("Warning: file %s is already exist but not mounted, skip creating file", linkPath) + } + + // Bind mount file + mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: mount.NewOSExec()} + if err := mounter.Mount(devicePath, linkPath, "" /* fsType */, []string{"bind"}); err != nil { + klog.Errorf("Failed to bind mount devicePath: %s to linkPath %s: %v", devicePath, linkPath, err) + return err + } + + return nil +} + +func mapSymlinkDevice(v VolumePathHandler, devicePath string, mapPath string, linkName string) error { // Remove old symbolic link(or file) then create new one. // This should be done because current symbolic link is // stale across node reboot. linkPath := filepath.Join(mapPath, string(linkName)) - if err = os.Remove(linkPath); err != nil && !os.IsNotExist(err) { + if err := os.Remove(linkPath); err != nil && !os.IsNotExist(err) { return err } - err = os.Symlink(devicePath, linkPath) - return err + return os.Symlink(devicePath, linkPath) } // UnmapDevice removes a symbolic link associated to block device under specified map path -func (v VolumePathHandler) UnmapDevice(mapPath string, linkName string) error { +func (v VolumePathHandler) UnmapDevice(mapPath string, linkName string, bindMount bool) error { if len(mapPath) == 0 { return fmt.Errorf("Failed to unmap device from map path. mapPath is empty") } klog.V(5).Infof("UnmapDevice: mapPath %s", mapPath) klog.V(5).Infof("UnmapDevice: linkName %s", linkName) + if bindMount { + return unmapBindMountDevice(v, mapPath, linkName) + } + return unmapSymlinkDevice(v, mapPath, linkName) +} + +func unmapBindMountDevice(v VolumePathHandler, mapPath string, linkName string) error { + // Check bind mount exists + linkPath := filepath.Join(mapPath, string(linkName)) + if isMountExist, checkErr := v.IsBindMountExist(linkPath); checkErr != nil { + return checkErr + } else if !isMountExist { + klog.Warningf("Warning: Unmap skipped because bind mount does not exist on the path: %v", linkPath) + // TODO: consider deleting empty file if it exists + return nil + } + + // Unmount file + mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: mount.NewOSExec()} + if err := mounter.Unmount(linkPath); err != nil { + klog.Errorf("Failed to unmount linkPath %s: %v", linkPath, err) + return err + } + + // Remove file + if err := os.Remove(linkPath); err != nil && !os.IsNotExist(err) { + return err + } + + return nil +} + +func unmapSymlinkDevice(v VolumePathHandler, mapPath string, linkName string) error { // Check symbolic link exists linkPath := filepath.Join(mapPath, string(linkName)) if islinkExist, checkErr := v.IsSymlinkExist(linkPath); checkErr != nil { @@ -125,8 +209,7 @@ func (v VolumePathHandler) UnmapDevice(mapPath string, linkName string) error { klog.Warningf("Warning: Unmap skipped because symlink does not exist on the path: %v", linkPath) return nil } - err := os.Remove(linkPath) - return err + return os.Remove(linkPath) } // RemoveMapPath removes a file or directory on specified map path @@ -163,70 +246,42 @@ func (v VolumePathHandler) IsSymlinkExist(mapPath string) (bool, error) { return false, err } -// GetDeviceSymlinkRefs searches symbolic links under global map path -func (v VolumePathHandler) GetDeviceSymlinkRefs(devPath string, mapPath string) ([]string, error) { +// IsBindMountExist returns true if specified file exists and the type is device. +// If file doesn't exist, or file exists but not device, return false with no error. +// On other cases, return false with error from Lstat(). +func (v VolumePathHandler) IsBindMountExist(mapPath string) (bool, error) { + fi, err := os.Lstat(mapPath) + if err == nil { + // If file exits and it's device, return true and no error + if fi.Mode()&os.ModeDevice == os.ModeDevice { + return true, nil + } + // If file exits but it's not device, return fale and no error + return false, nil + } + // If file doesn't exist, return false and no error + if os.IsNotExist(err) { + return false, nil + } + // Return error from Lstat() + return false, err +} + +// GetDeviceBindMountRefs searches bind mounts under global map path +func (v VolumePathHandler) GetDeviceBindMountRefs(devPath string, mapPath string) ([]string, error) { var refs []string files, err := ioutil.ReadDir(mapPath) if err != nil { return nil, fmt.Errorf("Directory cannot read %v", err) } for _, file := range files { - if file.Mode()&os.ModeSymlink != os.ModeSymlink { + if file.Mode()&os.ModeDevice != os.ModeDevice { continue } filename := file.Name() - fp, err := os.Readlink(filepath.Join(mapPath, filename)) - if err != nil { - return nil, fmt.Errorf("Symbolic link cannot be retrieved %v", err) - } - klog.V(5).Infof("GetDeviceSymlinkRefs: filepath: %v, devPath: %v", fp, devPath) - if fp == devPath { - refs = append(refs, filepath.Join(mapPath, filename)) - } + // TODO: Might need to check if the file is actually linked to devPath + refs = append(refs, filepath.Join(mapPath, filename)) } - klog.V(5).Infof("GetDeviceSymlinkRefs: refs %v", refs) + klog.V(5).Infof("GetDeviceBindMountRefs: refs %v", refs) return refs, nil } - -// FindGlobalMapPathUUIDFromPod finds {pod uuid} symbolic link under globalMapPath -// corresponding to map path symlink, and then return global map path with pod uuid. -// ex. mapPath symlink: pods/{podUid}}/{DefaultKubeletVolumeDevicesDirName}/{escapeQualifiedPluginName}/{volumeName} -> /dev/sdX -// globalMapPath/{pod uuid}: plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumePluginDependentPath}/{pod uuid} -> /dev/sdX -func (v VolumePathHandler) FindGlobalMapPathUUIDFromPod(pluginDir, mapPath string, podUID types.UID) (string, error) { - var globalMapPathUUID string - // Find symbolic link named pod uuid under plugin dir - err := filepath.Walk(pluginDir, func(path string, fi os.FileInfo, err error) error { - if err != nil { - return err - } - if (fi.Mode()&os.ModeSymlink == os.ModeSymlink) && (fi.Name() == string(podUID)) { - klog.V(5).Infof("FindGlobalMapPathFromPod: path %s, mapPath %s", path, mapPath) - if res, err := compareSymlinks(path, mapPath); err == nil && res { - globalMapPathUUID = path - } - } - return nil - }) - if err != nil { - return "", err - } - klog.V(5).Infof("FindGlobalMapPathFromPod: globalMapPathUUID %s", globalMapPathUUID) - // Return path contains global map path + {pod uuid} - return globalMapPathUUID, nil -} - -func compareSymlinks(global, pod string) (bool, error) { - devGlobal, err := os.Readlink(global) - if err != nil { - return false, err - } - devPod, err := os.Readlink(pod) - if err != nil { - return false, err - } - klog.V(5).Infof("CompareSymlinks: devGloBal %s, devPod %s", devGlobal, devPod) - if devGlobal == devPod { - return true, nil - } - return false, nil -} diff --git a/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go b/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go index 7170edc7de0..db9ecfad301 100644 --- a/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go +++ b/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go @@ -19,12 +19,15 @@ limitations under the License. package volumepathhandler import ( + "bufio" "errors" "fmt" "os" "os/exec" + "path/filepath" "strings" + "k8s.io/apimachinery/pkg/types" "k8s.io/klog" ) @@ -64,7 +67,7 @@ func (v VolumePathHandler) GetLoopDevice(path string) (string, error) { klog.V(2).Infof("Failed device discover command for path %s: %v %s", path, err, out) return "", err } - return parseLosetupOutputForDevice(out) + return parseLosetupOutputForDevice(out, path) } func makeLoopDevice(path string) (string, error) { @@ -75,7 +78,14 @@ func makeLoopDevice(path string) (string, error) { klog.V(2).Infof("Failed device create command for path: %s %v %s ", path, err, out) return "", err } - return parseLosetupOutputForDevice(out) + + // losetup -f --show {path} returns device in the format: + // /dev/loop1 + if len(out) == 0 { + return "", errors.New(ErrDeviceNotFound) + } + + return strings.TrimSpace(string(out)), nil } // RemoveLoopDevice removes specified loopback device @@ -93,16 +103,97 @@ func (v VolumePathHandler) RemoveLoopDevice(device string) error { return nil } -func parseLosetupOutputForDevice(output []byte) (string, error) { +func parseLosetupOutputForDevice(output []byte, path string) (string, error) { if len(output) == 0 { return "", errors.New(ErrDeviceNotFound) } - // losetup returns device in the format: - // /dev/loop1: [0073]:148662 (/dev/sda) - device := strings.TrimSpace(strings.SplitN(string(output), ":", 2)[0]) + // losetup -j {path} returns device in the format: + // /dev/loop1: [0073]:148662 (/dev/{globalMapPath}/{pod1-UUID}) + // /dev/loop2: [0073]:148662 (/dev/{globalMapPath}/{pod2-UUID}) + s := string(output) + // Find line that exact matches the path + var matched string + scanner := bufio.NewScanner(strings.NewReader(s)) + for scanner.Scan() { + if strings.HasSuffix(scanner.Text(), "("+path+")") { + matched = scanner.Text() + break + } + } + if len(matched) == 0 { + return "", errors.New(ErrDeviceNotFound) + } + s = matched + + device := strings.TrimSpace(strings.SplitN(s, ":", 2)[0]) if len(device) == 0 { return "", errors.New(ErrDeviceNotFound) } return device, nil } + +// FindGlobalMapPathUUIDFromPod finds {pod uuid} bind mount under globalMapPath +// corresponding to map path symlink, and then return global map path with pod uuid. +// ex. mapPath symlink: pods/{podUid}}/{DefaultKubeletVolumeDevicesDirName}/{escapeQualifiedPluginName}/{volumeName} -> /dev/sdX +// globalMapPath/{pod uuid} bind mount: plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumePluginDependentPath}/{pod uuid} -> /dev/sdX +func (v VolumePathHandler) FindGlobalMapPathUUIDFromPod(pluginDir, mapPath string, podUID types.UID) (string, error) { + var globalMapPathUUID string + // Find symbolic link named pod uuid under plugin dir + err := filepath.Walk(pluginDir, func(path string, fi os.FileInfo, err error) error { + if err != nil { + return err + } + if (fi.Mode()&os.ModeDevice == os.ModeDevice) && (fi.Name() == string(podUID)) { + klog.V(5).Infof("FindGlobalMapPathFromPod: path %s, mapPath %s", path, mapPath) + if res, err := compareBindMountAndSymlinks(path, mapPath); err == nil && res { + globalMapPathUUID = path + } + } + return nil + }) + if err != nil { + return "", err + } + klog.V(5).Infof("FindGlobalMapPathFromPod: globalMapPathUUID %s", globalMapPathUUID) + // Return path contains global map path + {pod uuid} + return globalMapPathUUID, nil +} + +func compareBindMountAndSymlinks(global, pod string) (bool, error) { + devNumGlobal, err := getDeviceMajorMinor(global) + if err != nil { + return false, err + } + devPod, err := os.Readlink(pod) + if err != nil { + return false, err + } + devNumPod, err := getDeviceMajorMinor(devPod) + if err != nil { + return false, err + } + klog.V(5).Infof("CompareBindMountAndSymlinks: devNumGlobal %s, devNumPod %s", devNumGlobal, devNumPod) + if devNumGlobal == devNumPod { + return true, nil + } + return false, nil +} + +func getDeviceMajorMinor(path string) (string, error) { + args := []string{"-c", "%t:%T", path} + cmd := exec.Command(statPath, args...) + out, err := cmd.CombinedOutput() + if err != nil { + klog.V(2).Infof("Failed to stat path: %s %v %s ", path, err, out) + return "", err + } + + // stat -c "%t:%T" {path} outputs following format(major:minor in hex): + // fc:10 + if len(out) == 0 { + return "", errors.New(ErrDeviceNotFound) + } + + return strings.TrimSpace(string(out)), nil +} diff --git a/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go b/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go index 266398b1da8..03d4c1dba2b 100644 --- a/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go +++ b/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go @@ -20,6 +20,8 @@ package volumepathhandler import ( "fmt" + + "k8s.io/apimachinery/pkg/types" ) // AttachFileDevice takes a path to a regular file and makes it available as an @@ -37,3 +39,9 @@ func (v VolumePathHandler) GetLoopDevice(path string) (string, error) { func (v VolumePathHandler) RemoveLoopDevice(device string) error { return fmt.Errorf("RemoveLoopDevice not supported for this build.") } + +// FindGlobalMapPathUUIDFromPod finds {pod uuid} bind mount under globalMapPath +// corresponding to map path symlink, and then return global map path with pod uuid. +func (v VolumePathHandler) FindGlobalMapPathUUIDFromPod(pluginDir, mapPath string, podUID types.UID) (string, error) { + return "", fmt.Errorf("FindGlobalMapPathUUIDFromPod not supported for this build.") +}