diff --git a/pkg/volume/fc/BUILD b/pkg/volume/fc/BUILD index b8d53706fb1..f6e513d34a3 100644 --- a/pkg/volume/fc/BUILD +++ b/pkg/volume/fc/BUILD @@ -43,6 +43,7 @@ go_test( "//pkg/util/mount:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/testing:go_default_library", + "//pkg/volume/util: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", diff --git a/pkg/volume/fc/attacher.go b/pkg/volume/fc/attacher.go index a0714dab36b..ff034c58e69 100644 --- a/pkg/volume/fc/attacher.go +++ b/pkg/volume/fc/attacher.go @@ -200,13 +200,15 @@ func volumeSpecToMounter(spec *volume.Spec, host volume.VolumeHost) (*fcDiskMoun volumeMode: volumeMode, readOnly: readOnly, mounter: volumehelper.NewSafeFormatAndMountFromHost(fcPluginName, host), + deviceUtil: volumeutil.NewDeviceHandler(volumeutil.NewIOHandler()), }, nil } return &fcDiskMounter{ - fcDisk: fcDisk, - fsType: fc.FSType, - readOnly: readOnly, - mounter: volumehelper.NewSafeFormatAndMountFromHost(fcPluginName, host), + fcDisk: fcDisk, + fsType: fc.FSType, + readOnly: readOnly, + mounter: volumehelper.NewSafeFormatAndMountFromHost(fcPluginName, host), + deviceUtil: volumeutil.NewDeviceHandler(volumeutil.NewIOHandler()), }, nil } @@ -215,6 +217,7 @@ func volumeSpecToUnmounter(mounter mount.Interface) *fcDiskUnmounter { fcDisk: &fcDisk{ io: &osIOHandler{}, }, - mounter: mounter, + mounter: mounter, + deviceUtil: volumeutil.NewDeviceHandler(volumeutil.NewIOHandler()), } } diff --git a/pkg/volume/fc/disk_manager.go b/pkg/volume/fc/disk_manager.go index efd1881808c..13cf923a923 100644 --- a/pkg/volume/fc/disk_manager.go +++ b/pkg/volume/fc/disk_manager.go @@ -31,7 +31,9 @@ type diskManager interface { // Attaches the disk to the kubelet's host machine. AttachDisk(b fcDiskMounter) (string, error) // Detaches the disk from the kubelet's host machine. - DetachDisk(disk fcDiskUnmounter, devName string) error + DetachDisk(disk fcDiskUnmounter, devicePath string) error + // Detaches the block disk from the kubelet's host machine. + DetachBlockFCDisk(disk fcDiskUnmapper, mntPath, devicePath string) error } // utility to mount a disk based filesystem diff --git a/pkg/volume/fc/fc.go b/pkg/volume/fc/fc.go index f6c2841bda5..8772ce91558 100644 --- a/pkg/volume/fc/fc.go +++ b/pkg/volume/fc/fc.go @@ -18,6 +18,7 @@ package fc import ( "fmt" + "os" "strconv" "strings" @@ -147,13 +148,15 @@ func (plugin *fcPlugin) newMounterInternal(spec *volume.Spec, podUID types.UID, volumeMode: volumeMode, readOnly: readOnly, mounter: &mount.SafeFormatAndMount{Interface: mounter, Exec: exec}, + deviceUtil: util.NewDeviceHandler(util.NewIOHandler()), }, nil } return &fcDiskMounter{ - fcDisk: fcDisk, - fsType: fc.FSType, - readOnly: readOnly, - mounter: &mount.SafeFormatAndMount{Interface: mounter, Exec: exec}, + fcDisk: fcDisk, + fsType: fc.FSType, + readOnly: readOnly, + mounter: &mount.SafeFormatAndMount{Interface: mounter, Exec: exec}, + deviceUtil: util.NewDeviceHandler(util.NewIOHandler()), }, nil } @@ -189,8 +192,9 @@ func (plugin *fcPlugin) newBlockVolumeMapperInternal(spec *volume.Spec, podUID t manager: manager, io: &osIOHandler{}, plugin: plugin}, - readOnly: readOnly, - mounter: &mount.SafeFormatAndMount{Interface: mounter, Exec: exec}, + readOnly: readOnly, + mounter: &mount.SafeFormatAndMount{Interface: mounter, Exec: exec}, + deviceUtil: util.NewDeviceHandler(util.NewIOHandler()), }, nil } @@ -208,7 +212,8 @@ func (plugin *fcPlugin) newUnmounterInternal(volName string, podUID types.UID, m plugin: plugin, io: &osIOHandler{}, }, - mounter: mounter, + mounter: mounter, + deviceUtil: util.NewDeviceHandler(util.NewIOHandler()), }, nil } @@ -225,6 +230,7 @@ func (plugin *fcPlugin) newUnmapperInternal(volName string, podUID types.UID, ma plugin: plugin, io: &osIOHandler{}, }, + deviceUtil: util.NewDeviceHandler(util.NewIOHandler()), }, nil } @@ -328,6 +334,7 @@ type fcDiskMounter struct { fsType string volumeMode v1.PersistentVolumeMode mounter *mount.SafeFormatAndMount + deviceUtil util.DeviceUtil } var _ volume.Mounter = &fcDiskMounter{} @@ -362,7 +369,8 @@ func (b *fcDiskMounter) SetUpAt(dir string, fsGroup *int64) error { type fcDiskUnmounter struct { *fcDisk - mounter mount.Interface + mounter mount.Interface + deviceUtil util.DeviceUtil } var _ volume.Unmounter = &fcDiskUnmounter{} @@ -380,8 +388,9 @@ func (c *fcDiskUnmounter) TearDownAt(dir string) error { // Block Volumes Support type fcDiskMapper struct { *fcDisk - readOnly bool - mounter mount.Interface + readOnly bool + mounter mount.Interface + deviceUtil util.DeviceUtil } var _ volume.BlockVolumeMapper = &fcDiskMapper{} @@ -392,18 +401,22 @@ func (b *fcDiskMapper) SetUpDevice() (string, error) { type fcDiskUnmapper struct { *fcDisk + deviceUtil util.DeviceUtil } var _ volume.BlockVolumeUnmapper = &fcDiskUnmapper{} -func (c *fcDiskUnmapper) TearDownDevice(_, devicePath string) error { - // Remove scsi device from the node. - if !strings.HasPrefix(devicePath, "/dev/") { - return fmt.Errorf("fc detach disk: invalid device name: %s", devicePath) +func (c *fcDiskUnmapper) TearDownDevice(mapPath, devicePath string) error { + err := c.manager.DetachBlockFCDisk(*c, mapPath, devicePath) + if err != nil { + return fmt.Errorf("fc: failed to detach disk: %s\nError: %v", mapPath, err) } - arr := strings.Split(devicePath, "/") - dev := arr[len(arr)-1] - removeFromScsiSubsystem(dev, c.io) + glog.V(4).Infof("fc: %q is unmounted, deleting the directory", mapPath) + err = os.RemoveAll(mapPath) + if err != nil { + return fmt.Errorf("fc: failed to delete the directory: %s\nError: %v", mapPath, err) + } + glog.V(4).Infof("fc: successfully detached disk: %s", mapPath) return nil } diff --git a/pkg/volume/fc/fc_test.go b/pkg/volume/fc/fc_test.go index 725f717bd67..42a530bc4a5 100644 --- a/pkg/volume/fc/fc_test.go +++ b/pkg/volume/fc/fc_test.go @@ -120,6 +120,15 @@ func (fake *fakeDiskManager) DetachDisk(c fcDiskUnmounter, mntPath string) error return nil } +func (fake *fakeDiskManager) DetachBlockFCDisk(c fcDiskUnmapper, mapPath, devicePath string) error { + err := os.RemoveAll(mapPath) + if err != nil { + return err + } + fake.detachCalled = true + return nil +} + func doTestPlugin(t *testing.T, spec *volume.Spec) { tmpDir, err := utiltesting.MkTmpdir("fc_test") if err != nil { diff --git a/pkg/volume/fc/fc_util.go b/pkg/volume/fc/fc_util.go index 83d930220b5..050989d353f 100644 --- a/pkg/volume/fc/fc_util.go +++ b/pkg/volume/fc/fc_util.go @@ -29,6 +29,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" + volumeutil "k8s.io/kubernetes/pkg/volume/util" ) type ioHandler interface { @@ -40,6 +41,11 @@ type ioHandler interface { type osIOHandler struct{} +const ( + byPath = "/dev/disk/by-path/" + byID = "/dev/disk/by-id/" +) + func (handler *osIOHandler) ReadDir(dirname string) ([]os.FileInfo, error) { return ioutil.ReadDir(dirname) } @@ -53,37 +59,17 @@ func (handler *osIOHandler) WriteFile(filename string, data []byte, perm os.File return ioutil.WriteFile(filename, data, perm) } -// given a disk path like /dev/sdx, find the devicemapper parent -// TODO #23192 Convert this code to use the generic code in ../util -// which is used by the iSCSI implementation -func findMultipathDeviceMapper(disk string, io ioHandler) string { - sys_path := "/sys/block/" - if dirs, err := io.ReadDir(sys_path); err == nil { - for _, f := range dirs { - name := f.Name() - if strings.HasPrefix(name, "dm-") { - if _, err1 := io.Lstat(sys_path + name + "/slaves/" + disk); err1 == nil { - return "/dev/" + name - } - } - } - } - return "" -} - // given a wwn and lun, find the device and associated devicemapper parent -func findDisk(wwn, lun string, io ioHandler) (string, string) { +func findDisk(wwn, lun string, io ioHandler, deviceUtil volumeutil.DeviceUtil) (string, string) { fc_path := "-fc-0x" + wwn + "-lun-" + lun - dev_path := "/dev/disk/by-path/" + dev_path := byPath if dirs, err := io.ReadDir(dev_path); err == nil { for _, f := range dirs { name := f.Name() if strings.Contains(name, fc_path) { if disk, err1 := io.EvalSymlinks(dev_path + name); err1 == nil { - arr := strings.Split(disk, "/") - l := len(arr) - 1 - dev := arr[l] - dm := findMultipathDeviceMapper(dev, io) + dm := deviceUtil.FindMultipathDeviceForDevice(disk) + glog.Infof("fc: find disk: %v, dm: %v", disk, dm) return disk, dm } } @@ -93,7 +79,7 @@ func findDisk(wwn, lun string, io ioHandler) (string, string) { } // given a wwid, find the device and associated devicemapper parent -func findDiskWWIDs(wwid string, io ioHandler) (string, string) { +func findDiskWWIDs(wwid string, io ioHandler, deviceUtil volumeutil.DeviceUtil) (string, string) { // Example wwid format: // 3600508b400105e210000900000490000 // @@ -104,7 +90,7 @@ func findDiskWWIDs(wwid string, io ioHandler) (string, string) { // underscore when wwid is exposed under /dev/by-id. fc_path := "scsi-" + wwid - dev_id := "/dev/disk/by-id/" + dev_id := byID if dirs, err := io.ReadDir(dev_id); err == nil { for _, f := range dirs { name := f.Name() @@ -114,10 +100,8 @@ func findDiskWWIDs(wwid string, io ioHandler) (string, string) { glog.V(2).Infof("fc: failed to find a corresponding disk from symlink[%s], error %v", dev_id+name, err) return "", "" } - arr := strings.Split(disk, "/") - l := len(arr) - 1 - dev := arr[l] - dm := findMultipathDeviceMapper(dev, io) + dm := deviceUtil.FindMultipathDeviceForDevice(disk) + glog.Infof("fc: find disk: %v, dm: %v", disk, dm) return disk, dm } } @@ -197,9 +181,9 @@ func searchDisk(b fcDiskMounter) (string, error) { for true { for _, diskId := range diskIds { if len(wwns) != 0 { - disk, dm = findDisk(diskId, lun, io) + disk, dm = findDisk(diskId, lun, io, b.deviceUtil) } else { - disk, dm = findDiskWWIDs(diskId, io) + disk, dm = findDiskWWIDs(diskId, io, b.deviceUtil) } // if multipath device is found, break if dm != "" { @@ -265,13 +249,153 @@ func (util *FCUtil) AttachDisk(b fcDiskMounter) (string, error) { return devicePath, err } -func (util *FCUtil) DetachDisk(c fcDiskUnmounter, devName string) error { - // Remove scsi device from the node. - if !strings.HasPrefix(devName, "/dev/") { - return fmt.Errorf("fc detach disk: invalid device name: %s", devName) +// DetachDisk removes scsi device file such as /dev/sdX from the node. +func (util *FCUtil) DetachDisk(c fcDiskUnmounter, devicePath string) error { + var devices []string + // devicePath might be like /dev/mapper/mpathX. Find destination. + dstPath, err := c.io.EvalSymlinks(devicePath) + if err != nil { + return err + } + // Find slave + if strings.HasPrefix(dstPath, "/dev/dm-") { + devices = c.deviceUtil.FindSlaveDevicesOnMultipath(dstPath) + } else { + // Add single devicepath to devices + devices = append(devices, dstPath) + } + glog.V(4).Infof("fc: DetachDisk devicePath: %v, dstPath: %v, devices: %v", devicePath, dstPath, devices) + var lastErr error + for _, device := range devices { + err := util.detachFCDisk(c.io, device) + if err != nil { + glog.Errorf("fc: detachFCDisk failed. device: %v err: %v", device, err) + lastErr = fmt.Errorf("fc: detachFCDisk failed. device: %v err: %v", device, err) + } + } + if lastErr != nil { + glog.Errorf("fc: last error occurred during detach disk:\n%v", lastErr) + return lastErr } - arr := strings.Split(devName, "/") - dev := arr[len(arr)-1] - removeFromScsiSubsystem(dev, c.io) return nil } + +// detachFCDisk removes scsi device file such as /dev/sdX from the node. +func (util *FCUtil) detachFCDisk(io ioHandler, devicePath string) error { + // Remove scsi device from the node. + if !strings.HasPrefix(devicePath, "/dev/") { + return fmt.Errorf("fc detach disk: invalid device name: %s", devicePath) + } + arr := strings.Split(devicePath, "/") + dev := arr[len(arr)-1] + removeFromScsiSubsystem(dev, io) + return nil +} + +// DetachBlockFCDisk detaches a volume from kubelet node, removes scsi device file +// such as /dev/sdX from the node, and then removes loopback for the scsi device. +func (util *FCUtil) DetachBlockFCDisk(c fcDiskUnmapper, mapPath, devicePath string) error { + // Check if devicePath is valid + if len(devicePath) != 0 { + if pathExists, pathErr := checkPathExists(devicePath); !pathExists || pathErr != nil { + return pathErr + } + } else { + // TODO: FC plugin can't obtain the devicePath from kubelet becuase devicePath + // in volume object isn't updated when volume is attached to kubelet node. + glog.Infof("fc: devicePath is empty. Try to retreive FC configuration from global map path: %v", mapPath) + } + + // Check if global map path is valid + // global map path examples: + // wwn+lun: plugins/kubernetes.io/fc/volumeDevices/50060e801049cfd1-lun-0/ + // wwid: plugins/kubernetes.io/fc/volumeDevices/3600508b400105e210000900000490000/ + if pathExists, pathErr := checkPathExists(mapPath); !pathExists || pathErr != nil { + return pathErr + } + + // Retreive volume plugin dependent path like '50060e801049cfd1-lun-0' from global map path + arr := strings.Split(mapPath, "/") + if len(arr) < 1 { + return fmt.Errorf("Fail to retreive volume plugin information from global map path: %v", mapPath) + } + volumeInfo := arr[len(arr)-1] + + // Search symbolick link which matches volumeInfo under /dev/disk/by-path or /dev/disk/by-id + // then find destination device path from the link + searchPath := byID + if strings.Contains(volumeInfo, "-lun-") { + searchPath = byPath + } + fis, err := ioutil.ReadDir(searchPath) + if err != nil { + return err + } + for _, fi := range fis { + if strings.Contains(fi.Name(), volumeInfo) { + devicePath = path.Join(searchPath, fi.Name()) + glog.V(5).Infof("fc: updated devicePath: %s", devicePath) + break + } + } + if len(devicePath) == 0 { + return fmt.Errorf("fc: failed to find corresponding device from searchPath: %v", searchPath) + } + dstPath, err := c.io.EvalSymlinks(devicePath) + if err != nil { + return err + } + glog.V(4).Infof("fc: find destination device path from symlink: %v", dstPath) + + // Get loopback device which takes fd lock for device beofore detaching a volume from node. + var devices []string + blkUtil := volumeutil.NewBlockVolumePathHandler() + dm := c.deviceUtil.FindMultipathDeviceForDevice(dstPath) + if len(dm) != 0 { + dstPath = dm + } + loop, err := volumeutil.BlockVolumePathHandler.GetLoopDevice(blkUtil, dstPath) + if err != nil { + glog.Warningf("fc: failed to get loopback for device: %v, err: %v", dstPath, err) + } else { + glog.V(4).Infof("fc: found loopback: %v", loop) + } + + // Detach volume from kubelet node + if len(dm) != 0 { + // Find all devices which are managed by multipath + devices = c.deviceUtil.FindSlaveDevicesOnMultipath(dm) + } else { + // Add single device path to devices + devices = append(devices, dstPath) + } + var lastErr error + for _, device := range devices { + err = util.detachFCDisk(c.io, device) + if err != nil { + glog.Errorf("fc: detachFCDisk failed. device: %v err: %v", device, err) + lastErr = fmt.Errorf("fc: detachFCDisk failed. device: %v err: %v", device, err) + } + } + if lastErr != nil { + glog.Errorf("fc: last error occurred during detach disk:\n%v", lastErr) + return lastErr + } + + // The volume was successfully detached from node. We can safely remove the loopback. + err = volumeutil.BlockVolumePathHandler.RemoveLoopDevice(blkUtil, loop) + if err != nil { + return fmt.Errorf("fc: failed to remove loopback :%v, err: %v", loop, err) + } + return nil +} + +func checkPathExists(path string) (bool, error) { + if pathExists, pathErr := volumeutil.PathExists(path); pathErr != nil { + return pathExists, fmt.Errorf("Error checking if path exists: %v", pathErr) + } else if !pathExists { + glog.Warningf("Warning: Unmap skipped because path does not exist: %v", path) + return pathExists, nil + } + return true, nil +} diff --git a/pkg/volume/fc/fc_util_test.go b/pkg/volume/fc/fc_util_test.go index 47e1c3b1dbf..50c5308b940 100644 --- a/pkg/volume/fc/fc_util_test.go +++ b/pkg/volume/fc/fc_util_test.go @@ -20,6 +20,8 @@ import ( "os" "testing" "time" + + "k8s.io/kubernetes/pkg/volume/util" ) type fakeFileInfo struct { @@ -91,6 +93,7 @@ func TestSearchDisk(t *testing.T) { lun: "0", io: &fakeIOHandler{}, }, + deviceUtil: util.NewDeviceHandler(util.NewIOHandler()), } devicePath, error := searchDisk(fakeMounter) // if no disk matches input wwn and lun, exit @@ -105,6 +108,7 @@ func TestSearchDiskWWID(t *testing.T) { wwids: []string{"3600508b400105e210000900000490000"}, io: &fakeIOHandler{}, }, + deviceUtil: util.NewDeviceHandler(util.NewIOHandler()), } devicePath, error := searchDisk(fakeMounter) // if no disk matches input wwid, exit diff --git a/pkg/volume/util/device_util.go b/pkg/volume/util/device_util.go index 9098d7b8597..9d504bc2e78 100644 --- a/pkg/volume/util/device_util.go +++ b/pkg/volume/util/device_util.go @@ -19,6 +19,7 @@ package util //DeviceUtil is a util for common device methods type DeviceUtil interface { FindMultipathDeviceForDevice(disk string) string + FindSlaveDevicesOnMultipath(disk string) []string } type deviceHandler struct { diff --git a/pkg/volume/util/device_util_linux.go b/pkg/volume/util/device_util_linux.go index 0d9851140f6..297004bf950 100644 --- a/pkg/volume/util/device_util_linux.go +++ b/pkg/volume/util/device_util_linux.go @@ -20,6 +20,7 @@ package util import ( "errors" + "path" "strings" ) @@ -59,3 +60,23 @@ func findDeviceForPath(path string, io IoUtil) (string, error) { } return "", errors.New("Illegal path for device " + devicePath) } + +// FindSlaveDevicesOnMultipath given a dm name like /dev/dm-1, find all devices +// which are managed by the devicemapper dm-1. +func (handler *deviceHandler) FindSlaveDevicesOnMultipath(dm string) []string { + var devices []string + io := handler.get_io + // Split path /dev/dm-1 into "", "dev", "dm-1" + parts := strings.Split(dm, "/") + if len(parts) != 3 || !strings.HasPrefix(parts[1], "dev") { + return devices + } + disk := parts[2] + slavesPath := path.Join("/sys/block/", disk, "/slaves/") + if files, err := io.ReadDir(slavesPath); err == nil { + for _, f := range files { + devices = append(devices, path.Join("/dev/", f.Name())) + } + } + return devices +} diff --git a/pkg/volume/util/device_util_linux_test.go b/pkg/volume/util/device_util_linux_test.go index 94ac9b5a47b..6ee7891a808 100644 --- a/pkg/volume/util/device_util_linux_test.go +++ b/pkg/volume/util/device_util_linux_test.go @@ -21,6 +21,7 @@ package util import ( "errors" "os" + "reflect" "testing" "time" ) @@ -29,11 +30,14 @@ type mockOsIOHandler struct{} func (handler *mockOsIOHandler) ReadDir(dirname string) ([]os.FileInfo, error) { switch dirname { - case "/sys/block/dm-2/slaves/": - f := &fakeFileInfo{ + case "/sys/block/dm-1/slaves": + f1 := &fakeFileInfo{ name: "sda", } - return []os.FileInfo{f}, nil + f2 := &fakeFileInfo{ + name: "sdb", + } + return []os.FileInfo{f1, f2}, nil case "/sys/block/": f1 := &fakeFileInfo{ name: "sda", @@ -62,8 +66,10 @@ func (handler *mockOsIOHandler) EvalSymlinks(path string) (string, error) { "/returns/a/dev": "/dev/sde", "/returns/non/dev": "/sys/block", "/dev/disk/by-path/127.0.0.1:3260-eui.02004567A425678D-lun-0": "/dev/sda", + "/dev/disk/by-path/127.0.0.3:3260-eui.03004567A425678D-lun-0": "/dev/sdb", "/dev/dm-2": "/dev/dm-2", "/dev/dm-3": "/dev/dm-3", + "/dev/sdc": "/dev/sdc", "/dev/sde": "/dev/sde", } return links[path], nil @@ -140,3 +146,15 @@ func TestFindDeviceForPath(t *testing.T) { } } + +func TestFindSlaveDevicesOnMultipath(t *testing.T) { + mockDeviceUtil := NewDeviceHandler(&mockOsIOHandler{}) + devices := mockDeviceUtil.FindSlaveDevicesOnMultipath("/dev/dm-1") + if !reflect.DeepEqual(devices, []string{"/dev/sda", "/dev/sdb"}) { + t.Fatalf("failed to find devices managed by mpio device. /dev/sda, /dev/sdb expected got [%s]", devices) + } + dev := mockDeviceUtil.FindSlaveDevicesOnMultipath("/dev/sdc") + if len(dev) != 0 { + t.Fatalf("mpio device not found '' expected got [%s]", dev) + } +} diff --git a/pkg/volume/util/device_util_unsupported.go b/pkg/volume/util/device_util_unsupported.go index 6afb1f13915..0b41eb3741c 100644 --- a/pkg/volume/util/device_util_unsupported.go +++ b/pkg/volume/util/device_util_unsupported.go @@ -22,3 +22,9 @@ package util func (handler *deviceHandler) FindMultipathDeviceForDevice(device string) string { return "" } + +// FindSlaveDevicesOnMultipath unsupported returns "" +func (handler *deviceHandler) FindSlaveDevicesOnMultipath(disk string) []string { + out := []string{} + return out +}