diff --git a/pkg/volume/fc/fc.go b/pkg/volume/fc/fc.go index 6b2afafc6f..3086743d7a 100644 --- a/pkg/volume/fc/fc.go +++ b/pkg/volume/fc/fc.go @@ -19,6 +19,7 @@ package fc import ( "fmt" "os" + "path/filepath" "strconv" "strings" @@ -257,40 +258,20 @@ func (plugin *fcPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volu if len(globalPDPath) == 0 { return nil, fmt.Errorf("couldn't fetch globalPDPath. failed to obtain volume spec") } - arr := strings.Split(globalPDPath, "/") - if len(arr) < 1 { - return nil, fmt.Errorf("failed to retrieve volume plugin information from globalPDPath: %v", globalPDPath) + + wwns, lun, wwids, err := parsePDName(globalPDPath) + if err != nil { + return nil, fmt.Errorf("failed to retrieve volume plugin information from globalPDPath: %s", err) } - volumeInfo := arr[len(arr)-1] // Create volume from wwn+lun or wwid - var fcVolume *v1.Volume - if strings.Contains(volumeInfo, "-lun-") { - wwnLun := strings.Split(volumeInfo, "-lun-") - if len(wwnLun) < 2 { - return nil, fmt.Errorf("failed to retrieve TargetWWN and Lun. volumeInfo is invalid: %v", volumeInfo) - } - lun, err := strconv.Atoi(wwnLun[1]) - if err != nil { - return nil, err - } - lun32 := int32(lun) - fcVolume = &v1.Volume{ - Name: volumeName, - VolumeSource: v1.VolumeSource{ - FC: &v1.FCVolumeSource{TargetWWNs: []string{wwnLun[0]}, Lun: &lun32}, - }, - } - klog.V(5).Infof("ConstructVolumeSpec: TargetWWNs: %v, Lun: %v", - fcVolume.VolumeSource.FC.TargetWWNs, *fcVolume.VolumeSource.FC.Lun) - } else { - fcVolume = &v1.Volume{ - Name: volumeName, - VolumeSource: v1.VolumeSource{ - FC: &v1.FCVolumeSource{WWIDs: []string{volumeInfo}}, - }, - } - klog.V(5).Infof("ConstructVolumeSpec: WWIDs: %v", fcVolume.VolumeSource.FC.WWIDs) + fcVolume := &v1.Volume{ + Name: volumeName, + VolumeSource: v1.VolumeSource{ + FC: &v1.FCVolumeSource{WWIDs: wwids, Lun: &lun, TargetWWNs: wwns}, + }, } + klog.V(5).Infof("ConstructVolumeSpec: TargetWWNs: %v, Lun: %v, WWIDs: %v", + fcVolume.VolumeSource.FC.TargetWWNs, *fcVolume.VolumeSource.FC.Lun, fcVolume.VolumeSource.FC.WWIDs) return volume.NewSpecFromVolume(fcVolume), nil } @@ -310,36 +291,23 @@ func (plugin *fcPlugin) ConstructBlockVolumeSpec(podUID types.UID, volumeName, m } klog.V(5).Infof("globalMapPathUUID: %v, err: %v", globalMapPathUUID, err) - // Retrieve volumePluginDependentPath from globalMapPathUUID + // Retrieve globalPDPath from globalMapPathUUID // globalMapPathUUID examples: // wwn+lun: plugins/kubernetes.io/fc/volumeDevices/50060e801049cfd1-lun-0/{pod uuid} // wwid: plugins/kubernetes.io/fc/volumeDevices/3600508b400105e210000900000490000/{pod uuid} - arr := strings.Split(globalMapPathUUID, "/") - if len(arr) < 2 { - return nil, fmt.Errorf("Fail to retrieve volume plugin information from globalMapPathUUID: %v", globalMapPathUUID) - } - l := len(arr) - 2 - volumeInfo := arr[l] - + globalPDPath := filepath.Dir(globalMapPathUUID) // Create volume from wwn+lun or wwid - var fcPV *v1.PersistentVolume - if strings.Contains(volumeInfo, "-lun-") { - wwnLun := strings.Split(volumeInfo, "-lun-") - lun, err := strconv.Atoi(wwnLun[1]) - if err != nil { - return nil, err - } - lun32 := int32(lun) - fcPV = createPersistentVolumeFromFCVolumeSource(volumeName, - v1.FCVolumeSource{TargetWWNs: []string{wwnLun[0]}, Lun: &lun32}) - klog.V(5).Infof("ConstructBlockVolumeSpec: TargetWWNs: %v, Lun: %v", - fcPV.Spec.PersistentVolumeSource.FC.TargetWWNs, - *fcPV.Spec.PersistentVolumeSource.FC.Lun) - } else { - fcPV = createPersistentVolumeFromFCVolumeSource(volumeName, - v1.FCVolumeSource{WWIDs: []string{volumeInfo}}) - klog.V(5).Infof("ConstructBlockVolumeSpec: WWIDs: %v", fcPV.Spec.PersistentVolumeSource.FC.WWIDs) + wwns, lun, wwids, err := parsePDName(globalPDPath) + if err != nil { + return nil, fmt.Errorf("failed to retrieve volume plugin information from globalPDPath: %s", err) } + fcPV := createPersistentVolumeFromFCVolumeSource(volumeName, + v1.FCVolumeSource{TargetWWNs: wwns, Lun: &lun, WWIDs: wwids}) + klog.V(5).Infof("ConstructBlockVolumeSpec: TargetWWNs: %v, Lun: %v, WWIDs: %v", + fcPV.Spec.PersistentVolumeSource.FC.TargetWWNs, + *fcPV.Spec.PersistentVolumeSource.FC.Lun, + fcPV.Spec.PersistentVolumeSource.FC.WWIDs) + return volume.NewSpecFromPersistentVolume(fcPV, false), nil } diff --git a/pkg/volume/fc/fc_util.go b/pkg/volume/fc/fc_util.go index 55382270ec..71c72071ce 100644 --- a/pkg/volume/fc/fc_util.go +++ b/pkg/volume/fc/fc_util.go @@ -22,6 +22,7 @@ import ( "os" "path" "path/filepath" + "strconv" "strings" "k8s.io/api/core/v1" @@ -131,20 +132,47 @@ func scsiHostRescan(io ioHandler) { } } -// make a directory like /var/lib/kubelet/plugins/kubernetes.io/fc/target-lun-0 +// make a directory like /var/lib/kubelet/plugins/kubernetes.io/fc/target1-target2-lun-0 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) + w := strings.Join(wwns, "-") + return path.Join(host.GetPluginDir(fcPluginName), w+"-lun-"+lun) } - return path.Join(host.GetPluginDir(fcPluginName), wwids[0]) + return path.Join(host.GetPluginDir(fcPluginName), strings.Join(wwids, "-")) } // make a directory like /var/lib/kubelet/plugins/kubernetes.io/fc/volumeDevices/target-lun-0 func makeVDPDNameInternal(host volume.VolumeHost, wwns []string, lun string, wwids []string) string { if len(wwns) != 0 { - return path.Join(host.GetVolumeDevicePluginDir(fcPluginName), wwns[0]+"-lun-"+lun) + w := strings.Join(wwns, "-") + return path.Join(host.GetVolumeDevicePluginDir(fcPluginName), w+"-lun-"+lun) } - return path.Join(host.GetVolumeDevicePluginDir(fcPluginName), wwids[0]) + return path.Join(host.GetVolumeDevicePluginDir(fcPluginName), strings.Join(wwids, "-")) +} + +func parsePDName(path string) (wwns []string, lun int32, wwids []string, err error) { + // parse directory name created by makePDNameInternal or makeVDPDNameInternal + dirname := filepath.Base(path) + components := strings.Split(dirname, "-") + l := len(components) + if l == 1 { + // No '-', it must be single WWID + return nil, 0, components, nil + } + if components[l-2] == "lun" { + // it has -lun-, it's list of WWNs + lun number as the last component + if l == 2 { + return nil, 0, nil, fmt.Errorf("no wwn in: %s", dirname) + } + lun, err := strconv.Atoi(components[l-1]) + if err != nil { + return nil, 0, nil, err + } + + return components[:l-2], int32(lun), nil, nil + } + // no -lun-, it's just list of WWIDs + return nil, 0, components, nil } type fcUtil struct{} diff --git a/pkg/volume/fc/fc_util_test.go b/pkg/volume/fc/fc_util_test.go index 50c5308b94..537d4ea2d2 100644 --- a/pkg/volume/fc/fc_util_test.go +++ b/pkg/volume/fc/fc_util_test.go @@ -18,6 +18,7 @@ package fc import ( "os" + "reflect" "testing" "time" @@ -116,3 +117,68 @@ func TestSearchDiskWWID(t *testing.T) { t.Errorf("no fc disk found") } } + +func TestParsePDName(t *testing.T) { + tests := []struct { + name string + path string + wwns []string + lun int32 + wwids []string + expectError bool + }{ + { + name: "single WWID", + path: "/var/lib/kubelet/plugins/kubernetes.io/fc/60050763008084e6e0000000000001ae", + wwids: []string{"60050763008084e6e0000000000001ae"}, + }, + { + name: "multiple WWID", + path: "/var/lib/kubelet/plugins/kubernetes.io/fc/60050763008084e6e0000000000001ae-60050763008084e6e0000000000001af", + wwids: []string{"60050763008084e6e0000000000001ae", "60050763008084e6e0000000000001af"}, + }, + { + name: "single WWN", + path: "/var/lib/kubelet/plugins/kubernetes.io/fc/50050768030539b6-lun-0", + wwns: []string{"50050768030539b6"}, + lun: 0, + }, + { + name: "multiple WWNs", + path: "/var/lib/kubelet/plugins/kubernetes.io/fc/50050768030539b6-50050768030539b7-lun-0", + wwns: []string{"50050768030539b6", "50050768030539b7"}, + lun: 0, + }, + { + name: "no WWNs", + path: "/var/lib/kubelet/plugins/kubernetes.io/fc/lun-0", + expectError: true, + }, + { + name: "invalid lun", + path: "/var/lib/kubelet/plugins/kubernetes.io/fc/50050768030539b6-lun-x", + expectError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + wwns, lun, wwids, err := parsePDName(test.path) + if test.expectError && err == nil { + t.Errorf("expected error but got none") + } + if !test.expectError && err != nil { + t.Errorf("got unexpected error: %s", err) + } + if !reflect.DeepEqual(wwns, test.wwns) { + t.Errorf("expected WWNs %+v, got %+v", test.wwns, wwns) + } + if lun != test.lun { + t.Errorf("expected lun %d, got %d", test.lun, lun) + } + if !reflect.DeepEqual(wwids, test.wwids) { + t.Errorf("expected WWIDs %+v, got %+v", test.wwids, wwids) + } + }) + } +}