From 03e28476c4abaf540ae9828351c8bf0d307f36ab Mon Sep 17 00:00:00 2001 From: mtanino Date: Mon, 10 Jul 2017 19:51:24 -0400 Subject: [PATCH] FC plugin: Support WWID for volume identifier This PR adds World Wide Identifier (WWID) parameter to FCVolumeSource as an unique volume identifier. fixes #48639 --- pkg/api/types.go | 10 ++- pkg/api/validation/validation.go | 20 ++++-- pkg/api/validation/validation_test.go | 66 +++++++++++++++++-- pkg/volume/fc/attacher.go | 22 +++++-- pkg/volume/fc/fc.go | 32 ++++++--- pkg/volume/fc/fc_test.go | 88 ++++++++++++++++++++++++- pkg/volume/fc/fc_util.go | 76 +++++++++++++++++---- pkg/volume/fc/fc_util_test.go | 33 ++++++++-- staging/src/k8s.io/api/core/v1/types.go | 14 ++-- 9 files changed, 308 insertions(+), 53 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index e6d637f0c82..5b60d37f569 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -715,9 +715,11 @@ type ISCSIVolumeSource struct { // Fibre Channel volumes can only be mounted as read/write once. // Fibre Channel volumes support ownership management and SELinux relabeling. type FCVolumeSource struct { - // Required: FC target worldwide names (WWNs) + // Optional: FC target worldwide names (WWNs) + // +optional TargetWWNs []string - // Required: FC target lun number + // Optional: FC target lun number + // +optional Lun *int32 // Filesystem type to mount. // Must be a filesystem type supported by the host operating system. @@ -729,6 +731,10 @@ type FCVolumeSource struct { // the ReadOnly setting in VolumeMounts. // +optional ReadOnly bool + // Optional: FC volume World Wide Identifiers (WWIDs) + // Either WWIDs or TargetWWNs and Lun must be set, but not both simultaneously. + // +optional + WWIDs []string } // FlexVolume represents a generic volume resource that is diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 0bfe1091076..b04fd7c09cb 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -648,15 +648,21 @@ func validateISCSIVolumeSource(iscsi *api.ISCSIVolumeSource, fldPath *field.Path func validateFCVolumeSource(fc *api.FCVolumeSource, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if len(fc.TargetWWNs) < 1 { - allErrs = append(allErrs, field.Required(fldPath.Child("targetWWNs"), "")) + if len(fc.TargetWWNs) < 1 && len(fc.WWIDs) < 1 { + allErrs = append(allErrs, field.Required(fldPath.Child("targetWWNs"), "must specify either targetWWNs or wwids, but not both")) } - if fc.Lun == nil { - allErrs = append(allErrs, field.Required(fldPath.Child("lun"), "")) - } else { - if *fc.Lun < 0 || *fc.Lun > 255 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("lun"), fc.Lun, validation.InclusiveRangeError(0, 255))) + if len(fc.TargetWWNs) != 0 && len(fc.WWIDs) != 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("targetWWNs"), fc.TargetWWNs, "targetWWNs and wwids can not be specified simultaneously")) + } + + if len(fc.TargetWWNs) != 0 { + if fc.Lun == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("lun"), "lun is required if targetWWNs is specified")) + } else { + if *fc.Lun < 0 || *fc.Lun > 255 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("lun"), fc.Lun, validation.InclusiveRangeError(0, 255))) + } } } return allErrs diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 95f89958f7b..6adca43ec87 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api" @@ -2099,7 +2100,7 @@ func TestValidateVolumes(t *testing.T) { }, // FC { - name: "valid FC", + name: "FC valid targetWWNs and lun", vol: api.Volume{ Name: "fc", VolumeSource: api.VolumeSource{ @@ -2113,23 +2114,56 @@ func TestValidateVolumes(t *testing.T) { }, }, { - name: "fc empty wwn", + name: "FC valid wwids", + vol: api.Volume{ + Name: "fc", + VolumeSource: api.VolumeSource{ + FC: &api.FCVolumeSource{ + WWIDs: []string{"some_wwid"}, + FSType: "ext4", + ReadOnly: false, + }, + }, + }, + }, + { + name: "FC empty targetWWNs and wwids", vol: api.Volume{ Name: "fc", VolumeSource: api.VolumeSource{ FC: &api.FCVolumeSource{ TargetWWNs: []string{}, Lun: newInt32(1), + WWIDs: []string{}, FSType: "ext4", ReadOnly: false, }, }, }, - errtype: field.ErrorTypeRequired, - errfield: "fc.targetWWNs", + errtype: field.ErrorTypeRequired, + errfield: "fc.targetWWNs", + errdetail: "must specify either targetWWNs or wwids", }, { - name: "fc empty lun", + name: "FC invalid: both targetWWNs and wwids simultaneously", + vol: api.Volume{ + Name: "fc", + VolumeSource: api.VolumeSource{ + FC: &api.FCVolumeSource{ + TargetWWNs: []string{"some_wwn"}, + Lun: newInt32(1), + WWIDs: []string{"some_wwid"}, + FSType: "ext4", + ReadOnly: false, + }, + }, + }, + errtype: field.ErrorTypeInvalid, + errfield: "fc.targetWWNs", + errdetail: "targetWWNs and wwids can not be specified simultaneously", + }, + { + name: "FC valid targetWWNs and empty lun", vol: api.Volume{ Name: "fc", VolumeSource: api.VolumeSource{ @@ -2141,8 +2175,26 @@ func TestValidateVolumes(t *testing.T) { }, }, }, - errtype: field.ErrorTypeRequired, - errfield: "fc.lun", + errtype: field.ErrorTypeRequired, + errfield: "fc.lun", + errdetail: "lun is required if targetWWNs is specified", + }, + { + name: "FC valid targetWWNs and invalid lun", + vol: api.Volume{ + Name: "fc", + VolumeSource: api.VolumeSource{ + FC: &api.FCVolumeSource{ + TargetWWNs: []string{"wwn"}, + Lun: newInt32(256), + FSType: "ext4", + ReadOnly: false, + }, + }, + }, + errtype: field.ErrorTypeInvalid, + errfield: "fc.lun", + errdetail: validation.InclusiveRangeError(0, 255), }, // FlexVolume { diff --git a/pkg/volume/fc/attacher.go b/pkg/volume/fc/attacher.go index c582a03ea42..c0e469c35bf 100644 --- a/pkg/volume/fc/attacher.go +++ b/pkg/volume/fc/attacher.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "strconv" + "strings" "time" "github.com/golang/glog" @@ -167,18 +168,27 @@ func volumeSpecToMounter(spec *volume.Spec, host volume.VolumeHost) (*fcDiskMoun if err != nil { return nil, err } - if fc.Lun == nil { - return nil, fmt.Errorf("empty lun") + var lun string + var wwids []string + if fc.Lun != nil && len(fc.TargetWWNs) != 0 { + lun = strconv.Itoa(int(*fc.Lun)) + } else if len(fc.WWIDs) != 0 { + for _, wwid := range fc.WWIDs { + wwids = append(wwids, strings.Replace(wwid, " ", "_", -1)) + } + } else { + return nil, fmt.Errorf("fc: no fc disk information found. failed to make a new mounter") } - lun := strconv.Itoa(int(*fc.Lun)) + return &fcDiskMounter{ fcDisk: &fcDisk{ plugin: &fcPlugin{ host: host, }, - wwns: fc.TargetWWNs, - lun: lun, - io: &osIOHandler{}, + wwns: fc.TargetWWNs, + lun: lun, + wwids: wwids, + io: &osIOHandler{}, }, fsType: fc.FSType, readOnly: readOnly, diff --git a/pkg/volume/fc/fc.go b/pkg/volume/fc/fc.go index 634cdb4ca63..26e69bd709f 100644 --- a/pkg/volume/fc/fc.go +++ b/pkg/volume/fc/fc.go @@ -19,12 +19,13 @@ package fc import ( "fmt" "strconv" + "strings" "github.com/golang/glog" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/util/mount" - "k8s.io/kubernetes/pkg/util/strings" + utilstrings "k8s.io/kubernetes/pkg/util/strings" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" "k8s.io/utils/exec" @@ -62,8 +63,15 @@ func (plugin *fcPlugin) GetVolumeName(spec *volume.Spec) (string, error) { return "", err } - // TargetWWNs are the FibreChannel target worldwide names - return fmt.Sprintf("%v", volumeSource.TargetWWNs), nil + if len(volumeSource.TargetWWNs) != 0 { + // TargetWWNs are the FibreChannel target worldwide names + return fmt.Sprintf("%v", volumeSource.TargetWWNs), nil + } else if len(volumeSource.WWIDs) != 0 { + // WWIDs are the FibreChannel World Wide Identifiers + return fmt.Sprintf("%v", volumeSource.WWIDs), nil + } + + return "", err } func (plugin *fcPlugin) CanSupport(spec *volume.Spec) bool { @@ -106,18 +114,25 @@ func (plugin *fcPlugin) newMounterInternal(spec *volume.Spec, podUID types.UID, return nil, err } - if fc.Lun == nil { - return nil, fmt.Errorf("empty lun") + var lun string + var wwids []string + if fc.Lun != nil && len(fc.TargetWWNs) != 0 { + lun = strconv.Itoa(int(*fc.Lun)) + } else if len(fc.WWIDs) != 0 { + for _, wwid := range fc.WWIDs { + wwids = append(wwids, strings.Replace(wwid, " ", "_", -1)) + } + } else { + return nil, fmt.Errorf("fc: no fc disk information found. failed to make a new mounter") } - lun := strconv.Itoa(int(*fc.Lun)) - return &fcDiskMounter{ fcDisk: &fcDisk{ podUID: podUID, volName: spec.Name(), wwns: fc.TargetWWNs, lun: lun, + wwids: wwids, manager: manager, io: &osIOHandler{}, plugin: plugin}, @@ -166,6 +181,7 @@ type fcDisk struct { portal string wwns []string lun string + wwids []string plugin *fcPlugin // Utility interface that provides API calls to the provider to attach/detach disks. manager diskManager @@ -177,7 +193,7 @@ type fcDisk struct { func (fc *fcDisk) GetPath() string { name := fcPluginName // safe to use PodVolumeDir now: volume teardown occurs before pod is cleaned up - return fc.plugin.host.GetPodVolumeDir(fc.podUID, strings.EscapeQualifiedNameForDisk(name), fc.volName) + return fc.plugin.host.GetPodVolumeDir(fc.podUID, utilstrings.EscapeQualifiedNameForDisk(name), fc.volName) } type fcDiskMounter struct { diff --git a/pkg/volume/fc/fc_test.go b/pkg/volume/fc/fc_test.go index 2675f923ac1..e2662d2c909 100644 --- a/pkg/volume/fc/fc_test.go +++ b/pkg/volume/fc/fc_test.go @@ -193,13 +193,39 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) { } } +func doTestPluginNilMounter(t *testing.T, spec *volume.Spec) { + tmpDir, err := utiltesting.MkTmpdir("fc_test") + if err != nil { + t.Fatalf("error creating temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + plugMgr := volume.VolumePluginMgr{} + plugMgr.InitPlugins(ProbeVolumePlugins(), volumetest.NewFakeVolumeHost(tmpDir, nil, nil)) + + plug, err := plugMgr.FindPluginByName("kubernetes.io/fc") + if err != nil { + t.Errorf("Can't find the plugin by name") + } + fakeManager := NewFakeDiskManager() + defer fakeManager.Cleanup() + fakeMounter := &mount.FakeMounter{} + mounter, err := plug.(*fcPlugin).newMounterInternal(spec, types.UID("poduid"), fakeManager, fakeMounter) + if err == nil { + t.Errorf("Error failed to make a new Mounter is expected: %v", err) + } + if mounter != nil { + t.Errorf("A nil Mounter is expected: %v", err) + } +} + func TestPluginVolume(t *testing.T) { lun := int32(0) vol := &v1.Volume{ Name: "vol1", VolumeSource: v1.VolumeSource{ FC: &v1.FCVolumeSource{ - TargetWWNs: []string{"some_wwn"}, + TargetWWNs: []string{"500a0981891b8dc5"}, FSType: "ext4", Lun: &lun, }, @@ -217,7 +243,7 @@ func TestPluginPersistentVolume(t *testing.T) { Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ FC: &v1.FCVolumeSource{ - TargetWWNs: []string{"some_wwn"}, + TargetWWNs: []string{"500a0981891b8dc5"}, FSType: "ext4", Lun: &lun, }, @@ -227,6 +253,64 @@ func TestPluginPersistentVolume(t *testing.T) { doTestPlugin(t, volume.NewSpecFromPersistentVolume(vol, false)) } +func TestPluginVolumeWWIDs(t *testing.T) { + vol := &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + FC: &v1.FCVolumeSource{ + WWIDs: []string{"3600508b400105e210000900000490000"}, + FSType: "ext4", + }, + }, + } + doTestPlugin(t, volume.NewSpecFromVolume(vol)) +} + +func TestPluginPersistentVolumeWWIDs(t *testing.T) { + vol := &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vol1", + }, + Spec: v1.PersistentVolumeSpec{ + PersistentVolumeSource: v1.PersistentVolumeSource{ + FC: &v1.FCVolumeSource{ + WWIDs: []string{"3600508b400105e21 000900000490000"}, + FSType: "ext4", + }, + }, + }, + } + doTestPlugin(t, volume.NewSpecFromPersistentVolume(vol, false)) +} + +func TestPluginVolumeNoDiskInfo(t *testing.T) { + vol := &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + FC: &v1.FCVolumeSource{ + FSType: "ext4", + }, + }, + } + doTestPluginNilMounter(t, volume.NewSpecFromVolume(vol)) +} + +func TestPluginPersistentVolumeNoDiskInfo(t *testing.T) { + vol := &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vol1", + }, + Spec: v1.PersistentVolumeSpec{ + PersistentVolumeSource: v1.PersistentVolumeSource{ + FC: &v1.FCVolumeSource{ + FSType: "ext4", + }, + }, + }, + } + doTestPluginNilMounter(t, volume.NewSpecFromPersistentVolume(vol, false)) +} + func TestPersistentClaimReadOnlyFlag(t *testing.T) { 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 feded0c2723..5ad268c4b2f 100644 --- a/pkg/volume/fc/fc_util.go +++ b/pkg/volume/fc/fc_util.go @@ -89,6 +89,40 @@ func findDisk(wwn, lun string, io ioHandler) (string, string) { return "", "" } +// given a wwid, find the device and associated devicemapper parent +func findDiskWWIDs(wwid string, io ioHandler) (string, string) { + // Example wwid format: + // 3600508b400105e210000900000490000 + // + // Example of symlink under by-id: + // /dev/by-id/scsi-3600508b400105e210000900000490000 + // /dev/by-id/scsi-_ + // The wwid could contain white space and it will be replaced + // underscore when wwid is exposed under /dev/by-id. + + fc_path := "scsi-" + wwid + dev_id := "/dev/disk/by-id/" + if dirs, err := io.ReadDir(dev_id); err == nil { + for _, f := range dirs { + name := f.Name() + if name == fc_path { + disk, err := io.EvalSymlinks(dev_id + name) + if err != nil { + 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) + return disk, dm + } + } + } + glog.V(2).Infof("fc: failed to find a disk [%s]", dev_id+fc_path) + return "", "" +} + // Removes a scsi device based upon /dev/sdX name func removeFromScsiSubsystem(deviceName string, io ioHandler) { fileName := "/sys/block/" + deviceName + "/device/delete" @@ -110,27 +144,46 @@ func scsiHostRescan(io ioHandler) { } // make a directory like /var/lib/kubelet/plugins/kubernetes.io/pod/fc/target-lun-0 -func makePDNameInternal(host volume.VolumeHost, wwns []string, lun string) string { - return path.Join(host.GetPluginDir(fcPluginName), wwns[0]+"-lun-"+lun) +func makePDNameInternal(host volume.VolumeHost, wwns []string, lun string, wwids []string) string { + if len(wwns) != 0 { + return path.Join(host.GetPluginDir(fcPluginName), wwns[0]+"-lun-"+lun) + } else { + return path.Join(host.GetPluginDir(fcPluginName), wwids[0]) + } } type FCUtil struct{} func (util *FCUtil) MakeGlobalPDName(fc fcDisk) string { - return makePDNameInternal(fc.plugin.host, fc.wwns, fc.lun) + return makePDNameInternal(fc.plugin.host, fc.wwns, fc.lun, fc.wwids) } -func searchDisk(wwns []string, lun string, io ioHandler) (string, string) { - disk := "" - dm := "" +func searchDisk(b fcDiskMounter) (string, string) { + var diskIds []string + var disk string + var dm string + io := b.io + wwids := b.wwids + wwns := b.wwns + lun := b.lun + + if len(wwns) != 0 { + diskIds = wwns + } else { + diskIds = wwids + } rescaned := false // two-phase search: // first phase, search existing device path, if a multipath dm is found, exit loop // otherwise, in second phase, rescan scsi bus and search again, return with any findings for true { - for _, wwn := range wwns { - disk, dm = findDisk(wwn, lun, io) + for _, diskId := range diskIds { + if len(wwns) != 0 { + disk, dm = findDisk(diskId, lun, io) + } else { + disk, dm = findDiskWWIDs(diskId, io) + } // if multipath device is found, break if dm != "" { break @@ -150,10 +203,9 @@ func searchDisk(wwns []string, lun string, io ioHandler) (string, string) { func (util *FCUtil) AttachDisk(b fcDiskMounter) (string, error) { devicePath := "" - wwns := b.wwns - lun := b.lun - io := b.io - disk, dm := searchDisk(wwns, lun, io) + var disk, dm string + + disk, dm = searchDisk(b) // if no disk matches input wwn and lun, exit if disk == "" && dm == "" { return "", fmt.Errorf("no fc disk found") diff --git a/pkg/volume/fc/fc_util_test.go b/pkg/volume/fc/fc_util_test.go index e6ae7a6f2c2..d9f609592b4 100644 --- a/pkg/volume/fc/fc_util_test.go +++ b/pkg/volume/fc/fc_util_test.go @@ -63,6 +63,11 @@ func (handler *fakeIOHandler) ReadDir(dirname string) ([]os.FileInfo, error) { name: "dm-1", } return []os.FileInfo{f}, nil + case "/dev/disk/by-id/": + f := &fakeFileInfo{ + name: "scsi-3600508b400105e210000900000490000", + } + return []os.FileInfo{f}, nil } return nil, nil } @@ -79,13 +84,31 @@ func (handler *fakeIOHandler) WriteFile(filename string, data []byte, perm os.Fi return nil } -func TestIoHandler(t *testing.T) { - io := &fakeIOHandler{} - wwns := []string{"500a0981891b8dc5"} - lun := "0" - disk, dm := searchDisk(wwns, lun, io) +func TestSearchDisk(t *testing.T) { + fakeMounter := fcDiskMounter{ + fcDisk: &fcDisk{ + wwns: []string{"500a0981891b8dc5"}, + lun: "0", + io: &fakeIOHandler{}, + }, + } + disk, dm := searchDisk(fakeMounter) // if no disk matches input wwn and lun, exit if disk == "" && dm == "" { t.Errorf("no fc disk found") } } + +func TestSearchDiskWWID(t *testing.T) { + fakeMounter := fcDiskMounter{ + fcDisk: &fcDisk{ + wwids: []string{"3600508b400105e210000900000490000"}, + io: &fakeIOHandler{}, + }, + } + disk, dm := searchDisk(fakeMounter) + // if no disk matches input wwid, exit + if disk == "" && dm == "" { + t.Errorf("no fc disk found") + } +} diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 6b79e441e9d..6c56bcf39aa 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -1102,10 +1102,12 @@ type ISCSIVolumeSource struct { // Fibre Channel volumes can only be mounted as read/write once. // Fibre Channel volumes support ownership management and SELinux relabeling. type FCVolumeSource struct { - // Required: FC target worldwide names (WWNs) - TargetWWNs []string `json:"targetWWNs" protobuf:"bytes,1,rep,name=targetWWNs"` - // Required: FC target lun number - Lun *int32 `json:"lun" protobuf:"varint,2,opt,name=lun"` + // Optional: FC target worldwide names (WWNs) + // +optional + TargetWWNs []string `json:"targetWWNs,omitempty" protobuf:"bytes,1,rep,name=targetWWNs"` + // Optional: FC target lun number + // +optional + Lun *int32 `json:"lun,omitempty" protobuf:"varint,2,opt,name=lun"` // Filesystem type to mount. // Must be a filesystem type supported by the host operating system. // Ex. "ext4", "xfs", "ntfs". Implicitly inferred to be "ext4" if unspecified. @@ -1116,6 +1118,10 @@ type FCVolumeSource struct { // the ReadOnly setting in VolumeMounts. // +optional ReadOnly bool `json:"readOnly,omitempty" protobuf:"varint,4,opt,name=readOnly"` + // Optional: FC volume world wide identifiers (wwids) + // Either wwids or combination of targetWWNs and lun must be set, but not both simultaneously. + // +optional + WWIDs []string `json:"wwids,omitempty" protobuf:"bytes,5,rep,name=wwids"` } // AzureFile represents an Azure File Service mount on the host and bind mount to the pod.