Merge pull request #44295 from rootfs/azure-devicepath-nil

Automatic merge from submit-queue (batch tested with PRs 45283, 45289, 45248, 44295)

Azure disk: dealing with missing disk probe

**What this PR does / why we need it**:
While Azure disks are expected to attach to SCSI host 3 and above on general purpose instances, on certain Azure instances disks are under SCSI host 2. 

This fix searches all LUNs but excludes those used by Azure sys disks, based on udev rules [here](https://raw.githubusercontent.com/Azure/WALinuxAgent/master/config/66-azure-storage.rules)

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

pkg/volume/azure_dd/attacher.go

@@ -172,6 +172,10 @@ func (attacher *azureDiskAttacher) WaitForAttach(spec *volume.Spec, lunStr strin
 	err = wait.Poll(checkSleepDuration, timeout, func() (bool, error) {
 		glog.V(4).Infof("Checking Azure disk %q(lun %s) is attached.", volumeSource.DiskName, lunStr)
 		if devicePath, err = findDiskByLun(lun, &osIOHandler{}, exe); err == nil {
+			if len(devicePath) == 0 {
+				glog.Warningf("cannot find attached Azure disk %q(lun %s) locally.", volumeSource.DiskName, lunStr)
+				return false, fmt.Errorf("cannot find attached Azure disk %q(lun %s) locally.", volumeSource.DiskName, lunStr)
+			}
 			glog.V(4).Infof("Successfully found attached Azure disk %q(lun %s, device path %s).", volumeSource.DiskName, lunStr, devicePath)
 			return true, nil
 		} else {

pkg/volume/azure_dd/vhd_util.go

@@ -31,6 +31,7 @@ import (
 type ioHandler interface {
 	ReadDir(dirname string) ([]os.FileInfo, error)
 	WriteFile(filename string, data []byte, perm os.FileMode) error
+	Readlink(name string) (string, error)
 }
 
 type osIOHandler struct{}
@@ -41,56 +42,83 @@ func (handler *osIOHandler) ReadDir(dirname string) ([]os.FileInfo, error) {
 func (handler *osIOHandler) WriteFile(filename string, data []byte, perm os.FileMode) error {
 	return ioutil.WriteFile(filename, data, perm)
 }
+func (handler *osIOHandler) Readlink(name string) (string, error) {
+	return os.Readlink(name)
+}
 
-// given a LUN find the VHD device path like /dev/sdb
+// exclude those used by azure as resource and OS root in /dev/disk/azure
-// VHD disks under sysfs are like /sys/bus/scsi/devices/3:0:1:0
+func listAzureDiskPath(io ioHandler) []string {
-// return empty string if no disk is found
+	azureDiskPath := "/dev/disk/azure/"
+	var azureDiskList []string
+	if dirs, err := io.ReadDir(azureDiskPath); err == nil {
+		for _, f := range dirs {
+			name := f.Name()
+			diskPath := azureDiskPath + name
+			if link, linkErr := io.Readlink(diskPath); linkErr == nil {
+				sd := link[(strings.LastIndex(link, "/") + 1):]
+				azureDiskList = append(azureDiskList, sd)
+			}
+		}
+	}
+	glog.V(12).Infof("Azure sys disks paths: %v", azureDiskList)
+	return azureDiskList
+}
+
+// given a LUN find the VHD device path like /dev/sdd
+// exclude those disks used by Azure resources and OS root
 func findDiskByLun(lun int, io ioHandler, exe exec.Interface) (string, error) {
+	azureDisks := listAzureDiskPath(io)
+	return findDiskByLunWithConstraint(lun, io, exe, azureDisks)
+}
+
+// look for device /dev/sdX and validate it is a VHD
+// return empty string if no disk is found
+func findDiskByLunWithConstraint(lun int, io ioHandler, exe exec.Interface, azureDisks []string) (string, error) {
 	var err error
 	sys_path := "/sys/bus/scsi/devices"
 	if dirs, err := io.ReadDir(sys_path); err == nil {
 		for _, f := range dirs {
 			name := f.Name()
-			// look for path like /sys/bus/scsi/devices/3:0:1:0
+			// look for path like /sys/bus/scsi/devices/3:0:0:1
 			arr := strings.Split(name, ":")
 			if len(arr) < 4 {
 				continue
 			}
-			target, err := strconv.Atoi(arr[0])
+			// extract LUN from the path.
+			// LUN is the last index of the array, i.e. 1 in /sys/bus/scsi/devices/3:0:0:1
+			l, err := strconv.Atoi(arr[3])
 			if err != nil {
-				glog.Errorf("failed to parse target from %v (%v), err %v", arr[0], name, err)
+				// unknown path format, continue to read the next one
+				glog.Errorf("failed to parse lun from %v (%v), err %v", arr[3], name, err)
 				continue
 			}
-
+			if lun == l {
-			// as observed, targets 0-3 are used by OS disks. Skip them
+				// find the matching LUN
-			if target > 3 {
+				// read vendor and model to ensure it is a VHD disk
-				l, err := strconv.Atoi(arr[3])
+				vendor := path.Join(sys_path, name, "vendor")
+				model := path.Join(sys_path, name, "model")
+				out, err := exe.Command("cat", vendor, model).CombinedOutput()
 				if err != nil {
-					// unknown path format, continue to read the next one
+					glog.Errorf("failed to cat device vendor and model, err: %v", err)
-					glog.Errorf("failed to parse lun from %v (%v), err %v", arr[3], name, err)
 					continue
 				}
-				if lun == l {
+				matched, err := regexp.MatchString("^MSFT[ ]{0,}\nVIRTUAL DISK[ ]{0,}\n$", strings.ToUpper(string(out)))
-					// find the matching LUN
+				if err != nil || !matched {
-					// read vendor and model to ensure it is a VHD disk
+					glog.V(4).Infof("doesn't match VHD, output %v, error %v", string(out), err)
-					vendor := path.Join(sys_path, name, "vendor")
+					continue
-					model := path.Join(sys_path, name, "model")
+				}
-					out, err := exe.Command("cat", vendor, model).CombinedOutput()
+				// find a disk, validate name
-					if err != nil {
+				dir := path.Join(sys_path, name, "block")
-						glog.Errorf("failed to cat device vendor and model, err: %v", err)
+				if dev, err := io.ReadDir(dir); err == nil {
-						continue
+					found := false
+					for _, diskName := range azureDisks {
+						glog.V(12).Infof("validating disk %q with sys disk %q", dev[0].Name(), diskName)
+						if string(dev[0].Name()) == diskName {
+							found = true
+							break
+						}
 					}
-					matched, err := regexp.MatchString("^MSFT[ ]{0,}\nVIRTUAL DISK[ ]{0,}\n$", strings.ToUpper(string(out)))
+					if !found {
-					if err != nil || !matched {
-						glog.V(4).Infof("doesn't match VHD, output %v, error %v", string(out), err)
-						continue
-					}
-					// find it!
-					dir := path.Join(sys_path, name, "block")
-					dev, err := io.ReadDir(dir)
-					if err != nil {
-						glog.Errorf("failed to read %s", dir)
-					} else {
 						return "/dev/" + dev[0].Name(), nil
 					}
 				}

pkg/volume/azure_dd/vhd_util_test.go

@@ -53,10 +53,14 @@ func (fi *fakeFileInfo) Sys() interface{} {
 }
 
 var (
-	lun      = 1
+	lun       = 1
-	lunStr   = "1"
+	lunStr    = "1"
-	diskPath = "4:0:0:" + lunStr
+	diskPath  = "4:0:0:" + lunStr
-	devName  = "sda"
+	devName   = "sdd"
+	lun1      = 2
+	lunStr1   = "2"
+	diskPath1 = "3:0:0:" + lunStr1
+	devName1  = "sde"
 )
 
 type fakeIOHandler struct{}
@@ -85,6 +89,11 @@ func (handler *fakeIOHandler) ReadDir(dirname string) ([]os.FileInfo, error) {
 			name: devName,
 		}
 		return []os.FileInfo{n}, nil
+	case "/sys/bus/scsi/devices/" + diskPath1 + "/block":
+		n := &fakeFileInfo{
+			name: devName1,
+		}
+		return []os.FileInfo{n}, nil
 	}
 	return nil, fmt.Errorf("bad dir")
 }
@@ -93,6 +102,10 @@ func (handler *fakeIOHandler) WriteFile(filename string, data []byte, perm os.Fi
 	return nil
 }
 
+func (handler *fakeIOHandler) Readlink(name string) (string, error) {
+	return "/dev/azure/disk/sda", nil
+}
+
 func TestIoHandler(t *testing.T) {
 	var fcmd exec.FakeCmd
 	fcmd = exec.FakeCmd{
@@ -101,16 +114,21 @@ func TestIoHandler(t *testing.T) {
 			func() ([]byte, error) {
 				return []byte("Msft    \nVirtual Disk \n"), nil
 			},
+			// cat
+			func() ([]byte, error) {
+				return []byte("Msft    \nVirtual Disk \n"), nil
+			},
 		},
 	}
 	fake := exec.FakeExec{
 		CommandScript: []exec.FakeCommandAction{
 			func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) },
 		},
 	}
 	disk, err := findDiskByLun(lun, &fakeIOHandler{}, &fake)
 	// if no disk matches lun, exit
 	if disk != "/dev/"+devName || err != nil {
-		t.Errorf("no data disk disk found: disk %v err %v", disk, err)
+		t.Errorf("no data disk found: disk %v err %v", disk, err)
 	}
 }