From b1f06aaed56e91a125714786806aa983e237179b Mon Sep 17 00:00:00 2001 From: mtanino Date: Fri, 9 Jun 2017 19:54:09 -0400 Subject: [PATCH] iSCSI plugin: Update devicepath with filepath.Glob result If iscsiTransport is not tcp, iSCSI plugin tries to find devicepath using filepath.Glob but never updates devicepath with the filepath.Glob result. This patch fixes the problem. Fixes #47253 --- pkg/volume/iscsi/iscsi_util.go | 51 ++++++++++++++++------------- pkg/volume/iscsi/iscsi_util_test.go | 22 ++++++++++--- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/pkg/volume/iscsi/iscsi_util.go b/pkg/volume/iscsi/iscsi_util.go index 11c01687fcd..7190859b542 100755 --- a/pkg/volume/iscsi/iscsi_util.go +++ b/pkg/volume/iscsi/iscsi_util.go @@ -89,32 +89,39 @@ func updateISCSINode(b iscsiDiskMounter, tp string) error { type StatFunc func(string) (os.FileInfo, error) type GlobFunc func(string) ([]string, error) -func waitForPathToExist(devicePath string, maxRetries int, deviceTransport string) bool { +func waitForPathToExist(devicePath *string, maxRetries int, deviceTransport string) bool { // This makes unit testing a lot easier return waitForPathToExistInternal(devicePath, maxRetries, deviceTransport, os.Stat, filepath.Glob) } -func waitForPathToExistInternal(devicePath string, maxRetries int, deviceTransport string, osStat StatFunc, filepathGlob GlobFunc) bool { - for i := 0; i < maxRetries; i++ { - var err error - if deviceTransport == "tcp" { - _, err = osStat(devicePath) - } else { - fpath, _ := filepathGlob(devicePath) - if fpath == nil { - err = os.ErrNotExist +func waitForPathToExistInternal(devicePath *string, maxRetries int, deviceTransport string, osStat StatFunc, filepathGlob GlobFunc) bool { + if devicePath != nil { + for i := 0; i < maxRetries; i++ { + var err error + if deviceTransport == "tcp" { + _, err = osStat(*devicePath) + } else { + fpath, _ := filepathGlob(*devicePath) + if fpath == nil { + err = os.ErrNotExist + } else { + // There might be a case that fpath contains multiple device paths if + // multiple PCI devices connect to same iscsi target. We handle this + // case at subsequent logic. Pick up only first path here. + *devicePath = fpath[0] + } } + if err == nil { + return true + } + if err != nil && !os.IsNotExist(err) { + return false + } + if i == maxRetries-1 { + break + } + time.Sleep(time.Second) } - if err == nil { - return true - } - if err != nil && !os.IsNotExist(err) { - return false - } - if i == maxRetries-1 { - break - } - time.Sleep(time.Second) } return false } @@ -214,7 +221,7 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error { } else { devicePath = strings.Join([]string{"/dev/disk/by-path/pci", "*", "ip", tp, "iscsi", b.Iqn, "lun", b.lun}, "-") } - exist := waitForPathToExist(devicePath, 1, iscsiTransport) + exist := waitForPathToExist(&devicePath, 1, iscsiTransport) if exist == false { // build discoverydb and discover iscsi target b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "new"}) @@ -245,7 +252,7 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error { lastErr = fmt.Errorf("iscsi: failed to attach disk: Error: %s (%v)", string(out), err) continue } - exist = waitForPathToExist(devicePath, 10, iscsiTransport) + exist = waitForPathToExist(&devicePath, 10, iscsiTransport) if !exist { glog.Errorf("Could not attach disk: Timeout after 10s") // update last error diff --git a/pkg/volume/iscsi/iscsi_util_test.go b/pkg/volume/iscsi/iscsi_util_test.go index 50dc4e33911..16f79a0664b 100755 --- a/pkg/volume/iscsi/iscsi_util_test.go +++ b/pkg/volume/iscsi/iscsi_util_test.go @@ -110,6 +110,12 @@ func fakeFilepathGlob(devicePath string) (globs []string, err error) { return []string{devicePath}, nil } +func fakeFilepathGlob2(devicePath string) (globs []string, err error) { + return []string{ + "/dev/disk/by-path/pci-0000:00:00.0-ip-127.0.0.1:3260-iqn.2014-12.com.example:test.tgt00-lun-0", + }, nil +} + func TestExtractTransportname(t *testing.T) { fakeIscsiadmOutput := []string{ "# BEGIN RECORD 2.0-873\n" + @@ -151,23 +157,29 @@ func TestExtractTransportname(t *testing.T) { func TestWaitForPathToExist(t *testing.T) { devicePath := []string{"/dev/disk/by-path/ip-127.0.0.1:3260-iqn.2014-12.com.example:test.tgt00-lun-0", - "/dev/disk/by-path/pci-0000:00:00.0-ip-127.0.0.1:3260-iqn.2014-12.com.example:test.tgt00-lun-0"} + "/dev/disk/by-path/pci-*-ip-127.0.0.1:3260-iqn.2014-12.com.example:test.tgt00-lun-0"} + fpath := "/dev/disk/by-path/pci-0000:00:00.0-ip-127.0.0.1:3260-iqn.2014-12.com.example:test.tgt00-lun-0" - exist := waitForPathToExistInternal(devicePath[0], 1, "tcp", fakeOsStat, filepath.Glob) + exist := waitForPathToExistInternal(&devicePath[0], 1, "tcp", fakeOsStat, filepath.Glob) if exist == false { t.Errorf("waitForPathToExist: could not find path %s", devicePath[0]) } - exist = waitForPathToExistInternal(devicePath[0], 1, "fake_iface", fakeOsStat, filepath.Glob) + exist = waitForPathToExistInternal(&devicePath[0], 1, "fake_iface", fakeOsStat, filepath.Glob) if exist != false { t.Errorf("waitForPathToExist: wrong code path called for %s", devicePath[0]) } - exist = waitForPathToExistInternal(devicePath[1], 1, "fake_iface", os.Stat, fakeFilepathGlob) + exist = waitForPathToExistInternal(&devicePath[1], 1, "fake_iface", os.Stat, fakeFilepathGlob) if exist == false { t.Errorf("waitForPathToExist: could not find path %s", devicePath[1]) } - exist = waitForPathToExistInternal(devicePath[1], 1, "tcp", os.Stat, fakeFilepathGlob) + exist = waitForPathToExistInternal(&devicePath[1], 1, "tcp", os.Stat, fakeFilepathGlob) if exist != false { t.Errorf("waitForPathToExist: wrong code path called for %s", devicePath[1]) } + + exist = waitForPathToExistInternal(&devicePath[1], 1, "fake_iface", os.Stat, fakeFilepathGlob2) + if devicePath[1] != fpath { + t.Errorf("waitForPathToExist: wrong code path called for %s", devicePath[1]) + } }