github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 19:56:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #57155 from mtanino/issue/57153

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 55475, 57155, 57260, 57222). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

FC plugin: Update detach logic for block volume if devicePath is empty

**What this PR does / why we need it**:
Currently, FC plugin doesn't have a chance to update attached volume's devicePath into volume object.
Therefore, at the volume teardown path, FC plugin gets empty devicePath from kubelet then fails volume detach operations especially volumeMode is Block.

This PR adds logic to obtain devicePath from global map path if passed devicePath from kubelet is empty.

I think this PR isn't complete solution but is a workaround to avoid teardown failures. In order to solve the root cause, we need to discuss the way to update devicePath at kubelet side again.(https://github.com/kubernetes/kubernetes/pull/54264)

If volume is managed by multipath, FC plugin fails to detach volume from kubelet node on both Filesystem and Block cases. This PR also fix the problem.

**Which issue(s) this PR fixes** : Fixes #57153

**Special notes for your reviewer**:
@rootfs @jsafrane 

**Release note**:

```
NONE
```
This commit is contained in:
Kubernetes Submit Queue 2017-12-18 19:45:41 -08:00 committed by GitHub
commit 199b110f72
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 267 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/volume/fc/BUILD
View File

@ -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",

13
pkg/volume/fc/attacher.go
View File

@ -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()),
}
}

4
pkg/volume/fc/disk_manager.go
View File

@ -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

47
pkg/volume/fc/fc.go
View File

@ -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
}

9
pkg/volume/fc/fc_test.go
View File

@ -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 {

202
pkg/volume/fc/fc_util.go
View File

@ -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
// <VENDOR NAME> <IDENTIFIER NUMBER>
@ -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
}

4
pkg/volume/fc/fc_util_test.go
View File

@ -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

1
pkg/volume/util/device_util.go
View File

@ -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 {

21
pkg/volume/util/device_util_linux.go
View File

@ -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
}

24
pkg/volume/util/device_util_linux_test.go
View File

@ -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)
}
}

6
pkg/volume/util/device_util_unsupported.go
View File

@ -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
}