Skip to content

Commit

Permalink
Merge 9b2f077 into 1c0104b
Browse files Browse the repository at this point in the history
  • Loading branch information
@olgashtivelman
olgashtivelman committed Oct 24, 2018
2 parents 1c0104b + 9b2f077 commit 789e3f0
Show file tree
Hide file tree
Showing 7 changed files with 553 additions and 57 deletions.
20 changes: 11 additions & 9 deletions fakes/fake_block_device_utils.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 50 additions & 26 deletions remote/mounter/block_device_utils/block_device_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ package block_device_utils_test
import (
"errors"
"fmt"
"github.com/IBM/ubiquity/fakes"
"github.com/IBM/ubiquity/remote/mounter/block_device_utils"
"github.com/IBM/ubiquity/utils"
"github.com/IBM/ubiquity/fakes"

"context"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"io/ioutil"
"os/exec"
"strings"
"testing"
"context"
"os/exec"
)

var _ = Describe("block_device_utils_test", func() {
Expand Down Expand Up @@ -204,9 +204,9 @@ var _ = Describe("block_device_utils_test", func() {
|- 33:0:0:1 sdb 8:16 active ready running
- 34:0:0:1 sdc 8:32 active ready running`
fakeExec.ExecuteWithTimeoutReturnsOnCall(0, []byte(mpathOutput), nil)
returnError := &exec.ExitError{}
returnError := &exec.ExitError{}
//this execute with timeout makes the GetWwnByScsiInq to return an error
fakeExec.ExecuteWithTimeoutReturnsOnCall(1, []byte(""),returnError)
fakeExec.ExecuteWithTimeoutReturnsOnCall(1, []byte(""), returnError)
_, err := bdUtils.Discover(volumeId, true)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(&block_device_utils.VolumeNotFoundError{volumeId}))
Expand All @@ -221,13 +221,13 @@ var _ = Describe("block_device_utils_test", func() {
|- 36:0:0:2 sdg 8:96 failed faulty running
- 37:0:0:2 sde 8:64 failed faulty running`, device, volumeId)
fakeExec.ExecuteWithTimeoutReturnsOnCall(0, []byte(mpathOutput), nil)
returnError := &exec.ExitError{}
returnError := &exec.ExitError{}
//this execute with timeout makes the GetWwnByScsiInq to return an error
fakeExec.ExecuteWithTimeoutReturnsOnCall(2, []byte(""),returnError)
fakeExec.ExecuteWithTimeoutReturnsOnCall(2, []byte(""), returnError)
fakeExec.ExecuteReturns([]byte(mpathOutput), nil)
_, err := bdUtils.Discover(volumeId, true)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(&block_device_utils.FaultyDeviceError{device}))
Expect(err).To(Equal(&block_device_utils.FaultyDeviceError{fmt.Sprintf("/dev/mapper/%s", device)}))
})
It("should return command execution error if device is not faulty.", func() {
volumeId := "6001738cfc9035ea0000000000796463"
Expand All @@ -239,14 +239,14 @@ var _ = Describe("block_device_utils_test", func() {
|- 36:0:0:2 sdg 8:96 active ready running
- 37:0:0:2 sde 8:64 active ready running`, device, volumeId)
fakeExec.ExecuteWithTimeoutReturnsOnCall(0, []byte(mpathOutput), nil)
returnError := &exec.ExitError{}
returnError := &exec.ExitError{}
//this execute with timeout makes the GetWwnByScsiInq to return an error
fakeExec.ExecuteWithTimeoutReturnsOnCall(2, []byte(""),returnError)
fakeExec.ExecuteWithTimeoutReturnsOnCall(2, []byte(""), returnError)
fakeExec.ExecuteReturns([]byte(mpathOutput), nil)
_, err := bdUtils.Discover(volumeId, true)
Expect(err).To(HaveOccurred())
_, ok := err.(*block_device_utils.CommandExecuteError)
Expect(ok).To(BeTrue())
Expect(ok).To(BeTrue())
})
})
Context(".DiscoverBySgInq", func() {
Expand Down Expand Up @@ -274,11 +274,11 @@ var _ = Describe("block_device_utils_test", func() {
_, cmd, _ := fakeExec.ExecuteWithTimeoutArgsForCall(0)
Expect(cmd).To(Equal("sg_inq"))
})

It("should return the 3rd mpath device mpathhg", func() {
volWwn := "0x6001738cfc9035eb0000000000cea5f6"
expectedWwn := strings.TrimPrefix(volWwn, "0x")
mpathOutput := fmt.Sprint(`mpathhe (36001738cfc9035eb0000000000cerwr) dm-3 IBM ,2810XIV
mpathOutput := fmt.Sprintf(`mpathhe (36001738cfc9035eb0000000000cerwr) dm-3 IBM ,2810XIV
size=19G features='1 queue_if_no_path' hwhandler='0' wp=rw
-+- policy='service-time 0' prio=1 status=active
|- 32:0:0:1 sdb 8:16 fault faulty running
Expand All @@ -292,7 +292,7 @@ mpathhg (3%s) dm-3 IBM ,2810XIV
size=19G features='1 queue_if_no_path' hwhandler='0' wp=rw
-+- policy='service-time 0' prio=1 status=active
|- 33:0:0:1 sdb 8:16 active ready running
- 34:0:0:1 sdc 8:32 active ready running`,expectedWwn )
- 34:0:0:1 sdc 8:32 active ready running`, expectedWwn)
inqResult := fmt.Sprintf(`VPD INQUIRY: Device Identification page
Designation descriptor number 1, descriptor length: 20
designator_type: NAA, code_set: Binary
Expand All @@ -301,14 +301,15 @@ mpathhg (3%s) dm-3 IBM ,2810XIV
Vendor Specific Identifier: 0xcfc9035eb
Vendor Specific Identifier Extension: 0xcea5f6
[%s]`, volWwn)
fakeExec.ExecuteWithTimeoutReturnsOnCall(0 ,nil , cmdErr)
fakeExec.ExecuteWithTimeoutReturnsOnCall(1 ,nil , cmdErr)
fakeExec.ExecuteWithTimeoutReturnsOnCall(2 ,[]byte(fmt.Sprintf("%s", inqResult)) , nil)

fmt.Println(mpathOutput)
// the first call to sg_inq will only be on a non faulty device!
fakeExec.ExecuteWithTimeoutReturnsOnCall(0, []byte(fmt.Sprintf("%s", inqResult)), nil)
dev, err := bdUtils.DiscoverBySgInq(mpathOutput, expectedWwn)
Expect(dev).To(Equal("mpathhg"))
Expect(err).ToNot(HaveOccurred())
Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(3))
_, cmd, _ := fakeExec.ExecuteWithTimeoutArgsForCall(2)
Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(1))
_, cmd, _ := fakeExec.ExecuteWithTimeoutArgsForCall(0)
Expect(cmd).To(Equal("sg_inq"))
})

Expand Down Expand Up @@ -356,9 +357,9 @@ mpathhb (36001738cfc9035eb0000000000cea###) dm-3 ##,##
|- 33:0:0:1 sdb 8:16 active ready running
- 34:0:0:1 sdc 8:32 active ready running`
wwn := "wwn"
returnError := &exec.ExitError{}
returnError := &exec.ExitError{}
//this execute with timeout makes the GetWwnByScsiInq to return an error
fakeExec.ExecuteWithTimeoutReturns([]byte(""),returnError)
fakeExec.ExecuteWithTimeoutReturns([]byte(""), returnError)
_, err := bdUtils.DiscoverBySgInq(mpathOutput, wwn)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(&block_device_utils.VolumeNotFoundError{wwn}))
Expand All @@ -368,7 +369,7 @@ mpathhb (36001738cfc9035eb0000000000cea###) dm-3 ##,##
It("GetWwnByScsiInq fails if sg_inq command fails", func() {
dev := "dev"
fakeExec.ExecuteWithTimeoutReturns([]byte{}, cmdErr)
_, err := bdUtils.GetWwnByScsiInq(dev)
_, err := bdUtils.GetWwnByScsiInq("", dev)
Expect(err).To(HaveOccurred())
})
It("should return wwn for mpath device", func() {
Expand All @@ -383,7 +384,7 @@ mpathhb (36001738cfc9035eb0000000000cea###) dm-3 ##,##
Vendor Specific Identifier Extension: 0xcea5f6
[%s]`, expecedWwn)
fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", result)), nil)
wwn, err := bdUtils.GetWwnByScsiInq(dev)
wwn, err := bdUtils.GetWwnByScsiInq("", dev)
Expect(err).ToNot(HaveOccurred())
Expect(wwn).To(Equal(strings.TrimPrefix(expecedWwn, "0x")))
Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(1))
Expand All @@ -403,7 +404,7 @@ mpathhb (36001738cfc9035eb0000000000cea###) dm-3 ##,##
Vendor Specific Extension Identifier: 0xcea5f6
[%s]`, expecedWwn)
fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", result)), nil)
wwn, err := bdUtils.GetWwnByScsiInq(dev)
wwn, err := bdUtils.GetWwnByScsiInq("", dev)
Expect(err).ToNot(HaveOccurred())
Expect(wwn).To(Equal(strings.TrimPrefix(expecedWwn, "0x")))
Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(1))
Expand All @@ -423,13 +424,36 @@ mpathhb (36001738cfc9035eb0000000000cea###) dm-3 ##,##
Vendor Specific Identifier Extension: 0xcea5f6
[%s]`, expecedWwn)
fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", result)), nil)
_, err := bdUtils.GetWwnByScsiInq(dev)
_, err := bdUtils.GetWwnByScsiInq("", dev)
Expect(err).To(HaveOccurred())
Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(1))
_, cmd, args := fakeExec.ExecuteWithTimeoutArgsForCall(0)
Expect(cmd).To(Equal("sg_inq"))
Expect(args).To(Equal([]string{"-p", "0x83", dev}))
})
It("should return error if device is faulty", func() {
dev := "mpathhe"
mpath := `mpathhe (36001738cfc9035eb0000000000cea5f6) dm-3 IBM ,2810XIV
size=19G features='1 queue_if_no_path' hwhandler='0' wp=rw
-+- policy='service-time 0' prio=1 status=active
|- 33:0:0:1 sdb 8:16 fault faulty running
- 34:0:0:1 sdc 8:32 fault faulty running`
_, err := bdUtils.GetWwnByScsiInq(mpath, dev)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(&block_device_utils.FaultyDeviceError{dev}))
Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(0))
})
It("should return an error if check for faulty device failed", func() {
dev := "mpath"
mpath := `mpathhe (36001738cfc9035eb0000000000cea5f6) dm-3 IBM ,2810XIV
size=19G features='1 queue_if_no_path' hwhandler='0' wp=rw
-+- policy='service-time 0' prio=1 status=active
|- 33:0:0:1 sdb 8:16 fault faulty running
- 34:0:0:1 sdc 8:32 fault faulty running`
_, err := bdUtils.GetWwnByScsiInq(mpath, dev)
Expect(err).To(HaveOccurred())
Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(1))
})
})

Context(".Cleanup", func() {
Expand Down Expand Up @@ -733,7 +757,7 @@ wrong format on /ubiquity/mpoint type ext4 (rw,relatime,data=ordered)
/XXX/mpoint on /ubiquity/mpoint type ext4 (rw,relatime,data=ordered)
/dev/mapper/yyy on /ubiquity/yyy type ext4 (rw,relatime,data=ordered)
`
fakeExec.ExecuteWithTimeoutReturnsOnCall(0, nil, cmdErr) // the umount command should fail
fakeExec.ExecuteWithTimeoutReturnsOnCall(0, nil, cmdErr) // the umount command should fail
fakeExec.ExecuteWithTimeoutReturnsOnCall(1, []byte(mountOutput), nil) // mount for isMounted
err = bdUtils.UmountFs(mpoint, "6001738CFC9035EA0000000000795164")
Expect(err).To(Not(HaveOccurred()))
Expand Down
8 changes: 8 additions & 0 deletions remote/mounter/block_device_utils/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,11 @@ type FaultyDeviceError struct {
func (e *FaultyDeviceError) Error() string {
return fmt.Sprintf("Device [%s] is in faulty state", e.Dev)
}

type MultipathDeviceNotFoundError struct {
Dev string
}

func (e *MultipathDeviceNotFoundError) Error() string {
return fmt.Sprintf("Device [%s] is not found in multipath output", e.Dev)
}
46 changes: 25 additions & 21 deletions remote/mounter/block_device_utils/mpath.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,15 @@ func (b *blockDeviceUtils) Discover(volumeWwn string, deepDiscovery bool) (strin
mpath = b.mpathDevFullPath(dev)

// Validate that we have the correct wwn.
SqInqWwn, err := b.GetWwnByScsiInq(mpath)
SqInqWwn, err := b.GetWwnByScsiInq(string(outputBytes[:]), mpath)
if err != nil {
args = []string{"-ll", dev}
b.logger.Debug(fmt.Sprintf("dev : %s ",dev))
outputBytes, merr := b.exec.Execute(multipathCmd, args)
if merr == nil {
stringOutput := string(outputBytes[:])
if strings.Contains(stringOutput, "faulty"){
return mpath, b.logger.ErrorRet(&FaultyDeviceError{dev}, "failed")
}
} else {
b.logger.Error("Failed to run multipath command while executing sg_inq." , logs.Args{{"err", merr}})
switch err.(type) {
case *FaultyDeviceError:
return mpath, b.logger.ErrorRet(err, "failed")
default:
b.logger.Error("Failed to run sg_inq on multipath device.", logs.Args{{"err", err}, {"mpaht", mpath}})
}

return "", b.logger.ErrorRet(&CommandExecuteError{"sg_inq", err}, "failed")
}

Expand Down Expand Up @@ -146,10 +141,10 @@ func (b *blockDeviceUtils) DiscoverBySgInq(mpathOutput string, volumeWwn string)
// Get the multipath device name at the beginning of the line
dev = strings.Split(line, " ")[0]
mpathFullPath := b.mpathDevFullPath(dev)
wwn, err := b.GetWwnByScsiInq(mpathFullPath)
wwn, err := b.GetWwnByScsiInq(mpathOutput, mpathFullPath)
if err != nil {
// we ignore errors and keep trying other devices.
b.logger.Warning(fmt.Sprintf("device [%s] cannot be sg_inq to validate if its related to WWN [%s]. sg_inq error is [%s]. Skip to the next mpath device.",dev,volumeWwn, err))
b.logger.Warning(fmt.Sprintf("device [%s] cannot be sg_inq to validate if its related to WWN [%s]. sg_inq error is [%s]. Skip to the next mpath device.", dev, volumeWwn, err))
continue
}
if strings.ToLower(wwn) == strings.ToLower(volumeWwn) {
Expand All @@ -160,7 +155,7 @@ func (b *blockDeviceUtils) DiscoverBySgInq(mpathOutput string, volumeWwn string)
return "", b.logger.ErrorRet(&VolumeNotFoundError{volumeWwn}, "failed")
}

func (b *blockDeviceUtils) GetWwnByScsiInq(dev string) (string, error) {
func (b *blockDeviceUtils) GetWwnByScsiInq(mpathOutput string, dev string) (string, error) {
defer b.logger.Trace(logs.DEBUG, logs.Args{{"dev", dev}})()
/* scsi inq example
$> sg_inq -p 0x83 /dev/mapper/mpathhe
Expand Down Expand Up @@ -199,6 +194,15 @@ func (b *blockDeviceUtils) GetWwnByScsiInq(dev string) (string, error) {
if err := b.exec.IsExecutable(sgInqCmd); err != nil {
return "", b.logger.ErrorRet(&commandNotFoundError{sgInqCmd, err}, "failed")
}
isFaulty, err := isDeviceFaulty(mpathOutput, dev, b.logger)
if err != nil {
// we should not get here since we get the device from the multipath output so there is not reason for it to be missing
// but in case something weird occurs we need to continue to not hurt the current flow.
b.logger.Warning("an error occured while trying to check if device is faulty.", logs.Args{{"err", err}, {"device", dev}})
}
if isFaulty {
return "", b.logger.ErrorRet(&FaultyDeviceError{dev}, fmt.Sprintf("The device [%s] is faulty. Therefore skipping sg_inq check on this device", dev))
}

args := []string{"-p", "0x83", dev}
// add timeout in case the call never comes back.
Expand All @@ -209,7 +213,7 @@ func (b *blockDeviceUtils) GetWwnByScsiInq(dev string) (string, error) {
}
wwnRegex := "(?i)" + `\[0x(.*?)\]`
wwnRegexCompiled, err := regexp.Compile(wwnRegex)

if err != nil {
return "", b.logger.ErrorRet(err, "failed")
}
Expand Down Expand Up @@ -247,10 +251,10 @@ func (b *blockDeviceUtils) GetWwnByScsiInq(dev string) (string, error) {
return "", b.logger.ErrorRet(&VolumeNotFoundError{wwn}, "failed")
}

func (b *blockDeviceUtils) SetDmsetup(mpath string) error{
func (b *blockDeviceUtils) SetDmsetup(mpath string) error {
defer b.logger.Trace(logs.DEBUG)()
dev := path.Base(mpath)

dev := path.Base(mpath)
dmsetupCmd := "dmsetup"
if err := b.exec.IsExecutable(dmsetupCmd); err != nil {
return b.logger.ErrorRet(&commandNotFoundError{dmsetupCmd, err}, "failed")
Expand All @@ -259,7 +263,7 @@ func (b *blockDeviceUtils) SetDmsetup(mpath string) error{
if _, err := b.exec.ExecuteWithTimeout(CleanupTimeout, dmsetupCmd, args); err != nil {
return b.logger.ErrorRet(&CommandExecuteError{dmsetupCmd, err}, "failed")
}

return nil
}

Expand Down Expand Up @@ -287,7 +291,7 @@ func (b *blockDeviceUtils) Cleanup(mpath string) error {
if _, err := b.exec.ExecuteWithTimeout(CleanupTimeout, multipathCmd, args); err != nil {
return b.logger.ErrorRet(&CommandExecuteError{multipathCmd, err}, "failed")
}

b.logger.Info("flushed", logs.Args{{"mpath", mpath}})
return nil
}

0 comments on commit 789e3f0

Please sign in to comment.