Skip to content

Commit

Permalink
UB-1407: merge from dev
Browse files Browse the repository at this point in the history
  • Loading branch information
Olga Shtivelman committed Aug 23, 2018
2 parents 9a90fca + 382fc6e commit f7fadb9
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 26 deletions.
6 changes: 3 additions & 3 deletions glide.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import:
- package: github.com/djherbis/times
version: v1.0.1
- package: github.com/gorilla/context
version: 08b5f424b9271eedf6f9f0ce86cb9396ed337a42
version: v1.1.1
- package: github.com/jinzhu/inflection
version: 04140366298a54a039076d798123ffa108fff46c
- package: github.com/lib/pq
Expand All @@ -16,7 +16,7 @@ import:
- package: github.com/gorilla/mux
version: v1.5.0
- package: github.com/natefinch/lumberjack
version: v2.0
version: aee4629129445bbdfb69aa565537dcfa16544311
- package: github.com/jinzhu/gorm
version: v1.0
subpackages:
Expand All @@ -28,7 +28,7 @@ import:
- package: github.com/pborman/uuid
version: ca53cad383cad2479bbba7f7a1a05797ec1386e4
- package: k8s.io/apimachinery
version: 68f9c3a1feb3140df59c67ced62d3a5df8e6c9c2
version: kubernetes-1.9.2
subpackages:
- pkg/types
- pkg/util/uuid
Expand Down
30 changes: 30 additions & 0 deletions remote/mounter/block_device_utils/block_device_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"strings"
"testing"
"context"
"os/exec"
)

var _ = Describe("block_device_utils_test", func() {
Expand Down Expand Up @@ -195,6 +196,21 @@ var _ = Describe("block_device_utils_test", func() {
_, err := bdUtils.Discover(volumeId, true)
Expect(err).To(HaveOccurred())
})
It("should return actual error when sg_inq command fails", func() {
volumeId := "0x6001738cfc9035eb0000000000cea5f6"
mpathOutput := `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 active ready running
- 34:0:0:1 sdc 8:32 active ready running`
fakeExec.ExecuteWithTimeoutReturnsOnCall(0, []byte(mpathOutput), nil)
returnError := &exec.ExitError{}
//this execute with timeout makes the GetWwnByScsiInq to return an error
fakeExec.ExecuteWithTimeoutReturnsOnCall(1, []byte(""),returnError)
_, err := bdUtils.Discover(volumeId, true)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(&block_device_utils.CommandExecuteError{"sg_inq",returnError}))
})
})
Context(".DiscoverBySgInq", func() {
It("should return mpathhe", func() {
Expand Down Expand Up @@ -258,6 +274,20 @@ mpathhb (36001738cfc9035eb0000000000cea###) dm-3 ##,##
_, err := bdUtils.DiscoverBySgInq(mpathOutput, wwn)
Expect(err).To(HaveOccurred())
})
It("should return actual error when sg_inq command fails", func() {
mpathOutput := `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 active ready running
- 34:0:0:1 sdc 8:32 active ready running`
wwn := "wwn"
returnError := &exec.ExitError{}
//this execute with timeout makes the GetWwnByScsiInq to return an error
fakeExec.ExecuteWithTimeoutReturns([]byte(""),returnError)
_, err := bdUtils.DiscoverBySgInq(mpathOutput, wwn)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(&block_device_utils.CommandExecuteError{"sg_inq",returnError}))
})
})
Context(".GetWwnByScsiInq", func() {
It("GetWwnByScsiInq fails if sg_inq command fails", func() {
Expand Down
10 changes: 5 additions & 5 deletions remote/mounter/block_device_utils/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ func (e *commandNotFoundError) Error() string {
return fmt.Sprintf("command [%v] is not found [%v]", e.cmd, e.err)
}

type commandExecuteError struct {
cmd string
err error
type CommandExecuteError struct {
Cmd string
Err error
}

func (e *commandExecuteError) Error() string {
return fmt.Sprintf("command [%v] execution failure [%v]", e.cmd, e.err)
func (e *CommandExecuteError) Error() string {
return fmt.Sprintf("command [%v] execution failure [%v]", e.Cmd, e.Err)
}

type VolumeNotFoundError struct {
Expand Down
13 changes: 6 additions & 7 deletions remote/mounter/block_device_utils/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"regexp"
"strings"
"syscall"

"github.com/IBM/ubiquity/utils/logs"
)

Expand All @@ -49,7 +48,7 @@ func (b *blockDeviceUtils) CheckFs(mpath string) (bool, error) {
// TODO we can improve it by double check the fs type of this device and maybe log warning if its not the same fstype we expacted
needFs = true
} else {
return false, b.logger.ErrorRet(&commandExecuteError{blkidCmd, err}, "failed")
return false, b.logger.ErrorRet(&CommandExecuteError{blkidCmd, err}, "failed")
}
}
b.logger.Info("checked", logs.Args{{"needFs", needFs}, {"mpath", mpath}, {blkidCmd, outputBytes}})
Expand All @@ -64,7 +63,7 @@ func (b *blockDeviceUtils) MakeFs(mpath string, fsType string) error {
}
args := []string{"-t", fsType, mpath}
if _, err := b.exec.Execute(mkfsCmd, args); err != nil {
return b.logger.ErrorRet(&commandExecuteError{mkfsCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{mkfsCmd, err}, "failed")
}
b.logger.Info("created", logs.Args{{"fsType", fsType}, {"mpath", mpath}})
return nil
Expand All @@ -78,7 +77,7 @@ func (b *blockDeviceUtils) MountFs(mpath string, mpoint string) error {
}
args := []string{mpath, mpoint}
if _, err := b.exec.ExecuteWithTimeout(TimeoutMilisecondMountCmdMountFs, mountCmd, args); err != nil {
return b.logger.ErrorRet(&commandExecuteError{mountCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{mountCmd, err}, "failed")
}
b.logger.Info("mounted", logs.Args{{"mpoint", mpoint}})
return nil
Expand All @@ -103,7 +102,7 @@ func (b *blockDeviceUtils) UmountFs(mpoint string) error {
b.logger.Info("Device already unmounted.", logs.Args{{"mpoint", mpoint}})
return nil
}
return b.logger.ErrorRet(&commandExecuteError{umountCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{umountCmd, err}, "failed")
}
b.logger.Info("umounted", logs.Args{{"mpoint", mpoint}})
return nil
Expand All @@ -112,7 +111,7 @@ func (b *blockDeviceUtils) UmountFs(mpoint string) error {
func (b *blockDeviceUtils) executeMountCmdToViewMountpoints() ([]byte, error) {
/*
Check if mount command exist (if not return error commandNotFoundError)
then trigger the mount command with no params (if failed return error commandExecuteError)
then trigger the mount command with no params (if failed return error CommandExecuteError)
and return the output as []byte.
*/

Expand All @@ -124,7 +123,7 @@ func (b *blockDeviceUtils) executeMountCmdToViewMountpoints() ([]byte, error) {

outputBytes, err := b.exec.ExecuteWithTimeout(TimeoutMilisecondMountCmdIsDeviceMounted, mountCmd, nil)
if err != nil {
return nil, b.logger.ErrorRet(&commandExecuteError{mountCmd, err}, "failed")
return nil, b.logger.ErrorRet(&CommandExecuteError{mountCmd, err}, "failed")
}

return outputBytes, nil
Expand Down
18 changes: 9 additions & 9 deletions remote/mounter/block_device_utils/mpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ func (b *blockDeviceUtils) ReloadMultipath() error {
args := []string{}
_, err := b.exec.ExecuteWithTimeout(MultipathTimeout, multipathCmd, args)
if err != nil {
return b.logger.ErrorRet(&commandExecuteError{multipathCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{multipathCmd, err}, "failed")
}

args = []string{"-r"}
_, err = b.exec.ExecuteWithTimeout(MultipathTimeout, multipathCmd, args)
if err != nil {
return b.logger.ErrorRet(&commandExecuteError{multipathCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{multipathCmd, err}, "failed")
}

return nil
Expand All @@ -62,7 +62,7 @@ func (b *blockDeviceUtils) Discover(volumeWwn string, deepDiscovery bool) (strin
args := []string{"-ll"}
outputBytes, err := b.exec.ExecuteWithTimeout(DiscoverTimeout, multipathCmd, args)
if err != nil {
return "", b.logger.ErrorRet(&commandExecuteError{multipathCmd, err}, "failed")
return "", b.logger.ErrorRet(&CommandExecuteError{multipathCmd, err}, "failed")
}
scanner := bufio.NewScanner(strings.NewReader(string(outputBytes[:])))
pattern := "(?i)" + volumeWwn
Expand All @@ -87,7 +87,7 @@ func (b *blockDeviceUtils) Discover(volumeWwn string, deepDiscovery bool) (strin
dev, err = b.DiscoverBySgInq(string(outputBytes[:]), volumeWwn)
if err != nil {
b.logger.Debug(fmt.Sprintf("mpath device was NOT found for WWN [%s] even after sg_inq on all mpath devices.", volumeWwn))
return "", b.logger.ErrorRet(&VolumeNotFoundError{volumeWwn}, "failed")
return "", b.logger.ErrorRet(err, "failed")
} else {
b.logger.Warning(fmt.Sprintf("device [%s] found for WWN [%s] after running sg_inq on all mpath devices although it was not found in multipath -ll. (Note: Could indicate multipathing issue).", dev, volumeWwn))
mpath = b.mpathDevFullPath(dev)
Expand All @@ -98,7 +98,7 @@ func (b *blockDeviceUtils) Discover(volumeWwn string, deepDiscovery bool) (strin
// Validate that we have the correct wwn.
SqInqWwn, err := b.GetWwnByScsiInq(mpath)
if err != nil {
return "", b.logger.ErrorRet(&commandExecuteError{"sg_inq", err}, "failed")
return "", b.logger.ErrorRet(&CommandExecuteError{"sg_inq", err}, "failed")
}

if strings.ToLower(SqInqWwn) != strings.ToLower(volumeWwn) {
Expand Down Expand Up @@ -136,7 +136,7 @@ func (b *blockDeviceUtils) DiscoverBySgInq(mpathOutput string, volumeWwn string)
mpathFullPath := b.mpathDevFullPath(dev)
wwn, err := b.GetWwnByScsiInq(mpathFullPath)
if err != nil {
return "", b.logger.ErrorRet(&VolumeNotFoundError{volumeWwn}, "failed")
return "", b.logger.ErrorRet(err, "failed")
}
if strings.ToLower(wwn) == strings.ToLower(volumeWwn) {
return dev, nil
Expand Down Expand Up @@ -191,7 +191,7 @@ func (b *blockDeviceUtils) GetWwnByScsiInq(dev string) (string, error) {
b.logger.Debug(fmt.Sprintf("Calling [%s] with timeout", sgInqCmd))
outputBytes, err := b.exec.ExecuteWithTimeout(3000, sgInqCmd, args)
if err != nil {
return "", b.logger.ErrorRet(&commandExecuteError{sgInqCmd, err}, "failed")
return "", b.logger.ErrorRet(&CommandExecuteError{sgInqCmd, err}, "failed")
}
wwnRegex := "(?i)" + `\[0x(.*?)\]`
wwnRegexCompiled, err := regexp.Compile(wwnRegex)
Expand Down Expand Up @@ -256,14 +256,14 @@ func (b *blockDeviceUtils) Cleanup(mpath string) error {

args := []string{"message", dev, "0", "fail_if_no_path"}
if _, err := b.exec.ExecuteWithTimeout(CleanupTimeout, dmsetupCmd, args); err != nil {
return b.logger.ErrorRet(&commandExecuteError{dmsetupCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{dmsetupCmd, err}, "failed")
}
if err := b.exec.IsExecutable(multipathCmd); err != nil {
return b.logger.ErrorRet(&commandNotFoundError{multipathCmd, err}, "failed")
}
args = []string{"-f", dev}
if _, err := b.exec.ExecuteWithTimeout(CleanupTimeout, multipathCmd, args); err != nil {
return b.logger.ErrorRet(&commandExecuteError{multipathCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{multipathCmd, err}, "failed")
}
b.logger.Info("flushed", logs.Args{{"mpath", mpath}})
return nil
Expand Down
4 changes: 2 additions & 2 deletions remote/mounter/block_device_utils/rescan.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (b *blockDeviceUtils) RescanISCSI() error {
}
args := []string{"-m", "session", "--rescan"}
if _, err := b.exec.ExecuteWithTimeout(rescanIscsiTimeout, rescanCmd, args); err != nil {
return b.logger.ErrorRet(&commandExecuteError{rescanCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{rescanCmd, err}, "failed")
}
return nil
}
Expand All @@ -66,7 +66,7 @@ func (b *blockDeviceUtils) RescanSCSI() error {
}
args := []string{"-r"} // TODO should use -r only in clean up
if _, err := b.exec.ExecuteWithTimeout(rescanScsiTimeout, rescanCmd, args); err != nil {
return b.logger.ErrorRet(&commandExecuteError{rescanCmd, err}, "failed")
return b.logger.ErrorRet(&CommandExecuteError{rescanCmd, err}, "failed")
}
return nil
}

0 comments on commit f7fadb9

Please sign in to comment.