diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index cc57be3184d..f93aaf62bd8 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -1178,16 +1178,16 @@ func (fv *FakeVolumePathHandler) AttachFileDevice(path string) (string, error) { return "", nil } +func (fv *FakeVolumePathHandler) DetachFileDevice(path string) error { + // nil is success, else error + return nil +} + func (fv *FakeVolumePathHandler) GetLoopDevice(path string) (string, error) { // nil is success, else error return "/dev/loop1", nil } -func (fv *FakeVolumePathHandler) RemoveLoopDevice(device string) error { - // nil is success, else error - return nil -} - // FindEmptyDirectoryUsageOnTmpfs finds the expected usage of an empty directory existing on // a tmpfs filesystem on this system. func FindEmptyDirectoryUsageOnTmpfs() (*resource.Quantity, error) { diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 9d69b683161..4b665ee2a67 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -1096,7 +1096,7 @@ func (og *operationGenerator) GenerateMapVolumeFunc( // that the block device is silently removed and attached another device with same name. // Container runtime can't handler this problem. To avoid unexpected condition fd lock // for the block device is required. - _, err = og.blkUtil.AttachFileDevice(devicePath) + _, err = og.blkUtil.AttachFileDevice(filepath.Join(globalMapPath, string(volumeToMount.Pod.UID))) if err != nil { return volumeToMount.GenerateError("MapVolume.AttachFileDevice failed", err) } @@ -1207,6 +1207,14 @@ func (og *operationGenerator) GenerateUnmapVolumeFunc( } unmapVolumeFunc := func() (error, error) { + globalUnmapPath := volumeToUnmount.DeviceMountPath + + // Release file descriptor lock. + err := og.blkUtil.DetachFileDevice(filepath.Join(globalUnmapPath, string(volumeToUnmount.PodUID))) + if err != nil { + return volumeToUnmount.GenerateError("UnmapVolume.UnmapDevice Detaching descriptor lock failed", err) + } + // Try to unmap volumeName symlink under pod device map path dir // pods/{podUid}/volumeDevices/{escapeQualifiedPluginName}/{volumeName} podDeviceUnmapPath, volName := blockVolumeUnmapper.GetPodDeviceMapPath() @@ -1217,7 +1225,6 @@ 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), true /* bindMount */) if unmapDeviceErr != nil { // On failure, return error. Caller will log and retry. @@ -1315,27 +1322,6 @@ func (og *operationGenerator) GenerateUnmapDeviceFunc( return deviceToDetach.GenerateError("UnmapDevice failed", err) } - // The block volume is not referenced from Pods. Release file descriptor lock. - // This should be done before calling TearDownDevice, because some plugins that do local detach - // in TearDownDevice will fail in detaching device due to the refcnt on the loopback device. - klog.V(4).Infof("UnmapDevice: deviceToDetach.DevicePath: %v", deviceToDetach.DevicePath) - loopPath, err := og.blkUtil.GetLoopDevice(deviceToDetach.DevicePath) - if err != nil { - if err.Error() == volumepathhandler.ErrDeviceNotFound { - klog.Warningf(deviceToDetach.GenerateMsgDetailed("UnmapDevice: Couldn't find loopback device which takes file descriptor lock", fmt.Sprintf("device path: %q", deviceToDetach.DevicePath))) - } else { - errInfo := "UnmapDevice.GetLoopDevice failed to get loopback device, " + fmt.Sprintf("device path: %q", deviceToDetach.DevicePath) - return deviceToDetach.GenerateError(errInfo, err) - } - } else { - if len(loopPath) != 0 { - err = og.blkUtil.RemoveLoopDevice(loopPath) - if err != nil { - return deviceToDetach.GenerateError("UnmapDevice.RemoveLoopDevice failed", err) - } - } - } - // Execute tear down device unmapErr := blockVolumeUnmapper.TearDownDevice(globalMapPath, deviceToDetach.DevicePath) if unmapErr != nil { diff --git a/pkg/volume/util/volumepathhandler/volume_path_handler.go b/pkg/volume/util/volumepathhandler/volume_path_handler.go index d245e558925..cf9c23beb68 100644 --- a/pkg/volume/util/volumepathhandler/volume_path_handler.go +++ b/pkg/volume/util/volumepathhandler/volume_path_handler.go @@ -55,10 +55,11 @@ type BlockVolumePathHandler interface { // AttachFileDevice takes a path to a regular file and makes it available as an // attached block device. AttachFileDevice(path string) (string, error) + // DetachFileDevice takes a path to the attached block device and + // detach it from block device. + DetachFileDevice(path string) error // GetLoopDevice returns the full path to the loop device associated with the given path. GetLoopDevice(path string) (string, error) - // RemoveLoopDevice removes specified loopback device - RemoveLoopDevice(device string) error } // NewBlockVolumePathHandler returns a new instance of BlockVolumeHandler. diff --git a/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go b/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go index db9ecfad301..272f074d11e 100644 --- a/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go +++ b/pkg/volume/util/volumepathhandler/volume_path_handler_linux.go @@ -50,6 +50,27 @@ func (v VolumePathHandler) AttachFileDevice(path string) (string, error) { return blockDevicePath, nil } +// DetachFileDevice takes a path to the attached block device and +// detach it from block device. +func (v VolumePathHandler) DetachFileDevice(path string) error { + loopPath, err := v.GetLoopDevice(path) + if err != nil { + if err.Error() == ErrDeviceNotFound { + klog.Warningf("DetachFileDevice: Couldn't find loopback device which takes file descriptor lock. device path: %q", path) + } else { + return err + } + } else { + if len(loopPath) != 0 { + err = removeLoopDevice(loopPath) + if err != nil { + return err + } + } + } + return nil +} + // GetLoopDevice returns the full path to the loop device associated with the given path. func (v VolumePathHandler) GetLoopDevice(path string) (string, error) { _, err := os.Stat(path) @@ -88,8 +109,8 @@ func makeLoopDevice(path string) (string, error) { return strings.TrimSpace(string(out)), nil } -// RemoveLoopDevice removes specified loopback device -func (v VolumePathHandler) RemoveLoopDevice(device string) error { +// removeLoopDevice removes specified loopback device +func removeLoopDevice(device string) error { args := []string{"-d", device} cmd := exec.Command(losetupPath, args...) out, err := cmd.CombinedOutput() diff --git a/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go b/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go index 03d4c1dba2b..f4411c6cdb7 100644 --- a/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go +++ b/pkg/volume/util/volumepathhandler/volume_path_handler_unsupported.go @@ -30,16 +30,17 @@ func (v VolumePathHandler) AttachFileDevice(path string) (string, error) { return "", fmt.Errorf("AttachFileDevice not supported for this build.") } +// DetachFileDevice takes a path to the attached block device and +// detach it from block device. +func (v VolumePathHandler) DetachFileDevice(path string) error { + return fmt.Errorf("DetachFileDevice not supported for this build.") +} + // GetLoopDevice returns the full path to the loop device associated with the given path. func (v VolumePathHandler) GetLoopDevice(path string) (string, error) { return "", fmt.Errorf("GetLoopDevice not supported for this build.") } -// RemoveLoopDevice removes specified loopback device -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) {