Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Azure disk: dealing with missing disk probe #44295

Merged
merged 1 commit into from
May 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/volume/azure_dd/attacher.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,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 {
Expand Down
92 changes: 60 additions & 32 deletions pkg/volume/azure_dd/vhd_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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
// VHD disks under sysfs are like /sys/bus/scsi/devices/3:0:1:0
// return empty string if no disk is found
// exclude those used by azure as resource and OS root in /dev/disk/azure
func listAzureDiskPath(io ioHandler) []string {
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
}

// as observed, targets 0-3 are used by OS disks. Skip them
if target > 3 {
l, err := strconv.Atoi(arr[3])
if lun == l {
// find the matching LUN
// read vendor and model to ensure it is a VHD disk
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 parse lun from %v (%v), err %v", arr[3], name, err)
glog.Errorf("failed to cat device vendor and model, err: %v", err)
continue
}
if lun == l {
// find the matching LUN
// read vendor and model to ensure it is a VHD disk
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 {
glog.Errorf("failed to cat device vendor and model, err: %v", err)
continue
}
matched, err := regexp.MatchString("^MSFT[ ]{0,}\nVIRTUAL DISK[ ]{0,}\n$", strings.ToUpper(string(out)))
if err != nil || !matched {
glog.V(4).Infof("doesn't match VHD, output %v, error %v", string(out), err)
continue
matched, err := regexp.MatchString("^MSFT[ ]{0,}\nVIRTUAL DISK[ ]{0,}\n$", strings.ToUpper(string(out)))
if err != nil || !matched {
glog.V(4).Infof("doesn't match VHD, output %v, error %v", string(out), err)
continue
}
// find a disk, validate name
dir := path.Join(sys_path, name, "block")
if dev, err := io.ReadDir(dir); err == nil {
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
}
}
// 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 {
if !found {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be found?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!found here indicates that the device is not used by OS

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, you are right. Maybe add a little comments indicating you are validate that the disks are not belong to system disk...

return "/dev/" + dev[0].Name(), nil
}
}
Expand Down
28 changes: 23 additions & 5 deletions pkg/volume/azure_dd/vhd_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,14 @@ func (fi *fakeFileInfo) Sys() interface{} {
}

var (
lun = 1
lunStr = "1"
diskPath = "4:0:0:" + lunStr
devName = "sda"
lun = 1
lunStr = "1"
diskPath = "4:0:0:" + lunStr
devName = "sdd"
lun1 = 2
lunStr1 = "2"
diskPath1 = "3:0:0:" + lunStr1
devName1 = "sde"
)

type fakeIOHandler struct{}
Expand Down Expand Up @@ -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")
}
Expand All @@ -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{
Expand All @@ -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)
}
}