Added a timeout for an execute call, and a timeout for sg_inq. + unit…

remote/mounter/block_device_utils/block_device_utils_test.go

@@ -129,15 +129,16 @@ var _ = Describe("block_device_utils_test", func() {
 							[%s]`, volumeId)
 			fakeExec.ExecuteReturnsOnCall(0, []byte(fmt.Sprintf("%s (%s) dm-1", result, volumeId)),
 				nil)
-			fakeExec.ExecuteReturnsOnCall(1, []byte(fmt.Sprintf("%s", inq_result)), nil) // for getWwnByScsiInq
+			fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", inq_result)), nil) // for getWwnByScsiInq
 			mpath, err := bdUtils.Discover(strings.TrimPrefix(volumeId, "0x"))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(mpath).To(Equal("/dev/mapper/" + result))
-			Expect(fakeExec.ExecuteCallCount()).To(Equal(2))
+			Expect(fakeExec.ExecuteCallCount()).To(Equal(1))
+			Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(1))
 			cmd, args := fakeExec.ExecuteArgsForCall(0)
 			Expect(cmd).To(Equal("multipath"))
 			Expect(args).To(Equal([]string{"-ll"}))
-			cmd, args = fakeExec.ExecuteArgsForCall(1)
+			_, cmd, args = fakeExec.ExecuteWithTimeoutArgsForCall(0)
 			Expect(cmd).To(Equal("sg_inq"))
 			Expect(args).To(Equal([]string{"-p",  "0x83", "/dev/mapper/mpath"}))
 		})
@@ -168,7 +169,7 @@ var _ = Describe("block_device_utils_test", func() {
 							[%s]`, volumeId)
 			fakeExec.ExecuteReturnsOnCall(0, []byte(fmt.Sprintf("%s (%s) dm-1", result, volumeId)),
 				nil)
-			fakeExec.ExecuteReturnsOnCall(1, []byte(fmt.Sprintf("%s", inq_result)), nil) // for getWwnByScsiInq
+			fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", inq_result)), nil) // for getWwnByScsiInq
 			fakeExec.StatReturns(nil, cmdErr)
 			_, err := bdUtils.Discover(strings.TrimPrefix(volumeId, "0x"))
 			Expect(err).To(HaveOccurred())
@@ -188,13 +189,13 @@ var _ = Describe("block_device_utils_test", func() {
 							[%s]`, wrongVolumeId)
 			fakeExec.ExecuteReturnsOnCall(0, []byte(fmt.Sprintf("%s (%s) dm-1", result, volumeId)),
 				nil)
-			fakeExec.ExecuteReturnsOnCall(1, []byte(fmt.Sprintf("%s", inq_result)), nil) // for getWwnByScsiInq
+			fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", inq_result)), nil) // for getWwnByScsiInq
 			_, err := bdUtils.Discover(strings.TrimPrefix(volumeId, "0x"))
 			Expect(err).To(HaveOccurred())
 			cmd, args := fakeExec.ExecuteArgsForCall(0)
 			Expect(cmd).To(Equal("multipath"))
 			Expect(args).To(Equal([]string{"-ll"}))
-			cmd, args = fakeExec.ExecuteArgsForCall(1)
+			_, cmd, args = fakeExec.ExecuteWithTimeoutArgsForCall(0)
 			Expect(cmd).To(Equal("sg_inq"))
 			Expect(args).To(Equal([]string{"-p",  "0x83", "/dev/mapper/mpath"}))
 		})
@@ -223,12 +224,12 @@ var _ = Describe("block_device_utils_test", func() {
 							Vendor Specific Identifier: 0xcfc9035eb
 							Vendor Specific Identifier Extension: 0xcea5f6
 							[%s]`, volWwn)
-			fakeExec.ExecuteReturns([]byte(fmt.Sprintf("%s", inq_result)), nil)
+			fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", inq_result)), nil)
 			dev, err := bdUtils.DiscoverBySgInq(mpathOutput, expectedWwn)
 			Expect(dev).To(Equal("mpathhe"))
 			Expect(err).ToNot(HaveOccurred())
-			Expect(fakeExec.ExecuteCallCount()).To(Equal(1))
-			cmd, _ := fakeExec.ExecuteArgsForCall(0)
+			Expect(fakeExec.ExecuteWithTimeoutCallCount()).To(Equal(1))
+			_, cmd, _ := fakeExec.ExecuteWithTimeoutArgsForCall(0)
 			Expect(cmd).To(Equal("sg_inq"))
 		})
 		It("should return wwn command fails", func() {
@@ -246,7 +247,7 @@ var _ = Describe("block_device_utils_test", func() {
 	Context( ".GetWwnByScsiInq", func(){
 		It("GetWwnByScsiInq fails if sg_inq command fails", func() {
 			dev := "dev"
-			fakeExec.ExecuteReturns([]byte{}, cmdErr)
+			fakeExec.ExecuteWithTimeoutReturns([]byte{}, cmdErr)
 			_, err := bdUtils.GetWwnByScsiInq(dev)
 			Expect(err).To(HaveOccurred())
 		})
@@ -261,12 +262,12 @@ var _ = Describe("block_device_utils_test", func() {
 							Vendor Specific Identifier: 0xcfc9035eb
 							Vendor Specific Identifier Extension: 0xcea5f6
 							[%s]`, expecedWwn)
-			fakeExec.ExecuteReturns([]byte(fmt.Sprintf("%s", result)), nil)
+			fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", result)), nil)
 			wwn, err := bdUtils.GetWwnByScsiInq(dev)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(wwn).To(Equal(strings.TrimPrefix(expecedWwn, "0x")))
-			Expect(fakeExec.ExecuteCallCount()).To(Equal(1))
-			cmd, args := fakeExec.ExecuteArgsForCall(0)
+			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}))
 		})
@@ -281,11 +282,11 @@ var _ = Describe("block_device_utils_test", func() {
 							Vendor Specific Identifier: 0xcfc9035eb
 							Vendor Specific Identifier Extension: 0xcea5f6
 							[%s]`, expecedWwn)
-			fakeExec.ExecuteReturns([]byte(fmt.Sprintf("%s", result)), nil)
+			fakeExec.ExecuteWithTimeoutReturns([]byte(fmt.Sprintf("%s", result)), nil)
 			_, err := bdUtils.GetWwnByScsiInq(dev)
 			Expect(err).To(HaveOccurred())
-			Expect(fakeExec.ExecuteCallCount()).To(Equal(1))
-			cmd, args := fakeExec.ExecuteArgsForCall(0)
+			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}))
 		})

remote/mounter/block_device_utils/mpath.go

@@ -164,7 +164,9 @@ func (b *blockDeviceUtils) GetWwnByScsiInq(dev string) (string, error) {
 	}
 
 	args := []string{"-p",  "0x83", dev}
-	outputBytes, err := b.exec.Execute(sgInqCmd, args)
+	// add timeout in case the call never comes back.
+	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")
 	}

utils/executor.go

@@ -22,6 +22,9 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"context"
+	"time"
+	"fmt"
 )
 
 //go:generate counterfeiter -o ../fakes/fake_executor.go . Executor
@@ -36,6 +39,7 @@ type Executor interface { // basic host dependent functions
 	IsExecutable(string) error
 	IsNotExist(error) bool
 	EvalSymlinks(path string) (string, error)
+	ExecuteWithTimeout(mSeconds int ,command string, args []string) ([]byte, error)
 }
 
 type executor struct {
@@ -67,6 +71,36 @@ func (e *executor) Execute(command string, args []string) ([]byte, error) {
 
 	return stdOut, err
 }
+
+func (e *executor) ExecuteWithTimeout(mSeconds int ,command string, args []string) ([]byte, error) {
+
+    // Create a new context and add a timeout to it
+    ctx, cancel := context.WithTimeout(context.Background(), time.Duration(mSeconds)*time.Millisecond)
+    defer cancel() // The cancel should be deferred so resources are cleaned up
+
+    // Create the command with our context
+    cmd := exec.CommandContext(ctx, command, args...)
+
+    // This time we can simply use Output() to get the result.
+    out, err := cmd.Output()
+
+    // We want to check the context error to see if the timeout was executed.
+    // The error returned by cmd.Output() will be OS specific based on what
+    // happens when a process is killed.
+    if ctx.Err() == context.DeadlineExceeded {
+        e.logger.Debug(fmt.Sprintf("Command %s timeout reached", command))
+        return nil, ctx.Err()
+    }
+
+    // If there's no context error, we know the command completed (or errored).
+    e.logger.Debug(fmt.Sprintf("Output from command:", string(out)))
+    if err != nil {
+        e.logger.Debug(fmt.Sprintf("Non-zero exit code:", err))
+    }
+
+    return out, err
+}
+
 func (e *executor) Stat(path string) (os.FileInfo, error) {
 	return os.Stat(path)
 }