Merge pull request #47281 from mtanino/issue/47253

Automatic merge from submit-queue

iSCSI plugin: Update devicepath with filepath.Glob result

**What this PR does / why we need it**:

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.

**Which issue this PR fixes** : fixes #47253
**Special notes for your reviewer**:

**Release note**:

```
NONE
```

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 {
+func waitForPathToExistInternal(devicePath *string, maxRetries int, deviceTransport string, osStat StatFunc, filepathGlob GlobFunc) bool {
-	for i := 0; i < maxRetries; i++ {
+	if devicePath != nil {
-		var err error
+		for i := 0; i < maxRetries; i++ {
-		if deviceTransport == "tcp" {
+			var err error
-			_, err = osStat(devicePath)
+			if deviceTransport == "tcp" {
-		} else {
+				_, err = osStat(*devicePath)
-			fpath, _ := filepathGlob(devicePath)
+			} else {
-			if fpath == nil {
+				fpath, _ := filepathGlob(*devicePath)
-				err = os.ErrNotExist
+				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

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])
+	}
 }