diff --git a/fakes/fake_executor.go b/fakes/fake_executor.go index 23bc4def..27919a65 100644 --- a/fakes/fake_executor.go +++ b/fakes/fake_executor.go @@ -14,7 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package fakes import ( @@ -144,6 +143,21 @@ type FakeExecutor struct { result1 string result2 error } + ExecuteWithTimeoutStub func(mSeconds int, command string, args []string) ([]byte, error) + executeWithTimeoutMutex sync.RWMutex + executeWithTimeoutArgsForCall []struct { + mSeconds int + command string + args []string + } + executeWithTimeoutReturns struct { + result1 []byte + result2 error + } + executeWithTimeoutReturnsOnCall map[int]struct { + result1 []byte + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -640,6 +654,64 @@ func (fake *FakeExecutor) EvalSymlinksReturnsOnCall(i int, result1 string, resul }{result1, result2} } +func (fake *FakeExecutor) ExecuteWithTimeout(mSeconds int, command string, args []string) ([]byte, error) { + var argsCopy []string + if args != nil { + argsCopy = make([]string, len(args)) + copy(argsCopy, args) + } + fake.executeWithTimeoutMutex.Lock() + ret, specificReturn := fake.executeWithTimeoutReturnsOnCall[len(fake.executeWithTimeoutArgsForCall)] + fake.executeWithTimeoutArgsForCall = append(fake.executeWithTimeoutArgsForCall, struct { + mSeconds int + command string + args []string + }{mSeconds, command, argsCopy}) + fake.recordInvocation("ExecuteWithTimeout", []interface{}{mSeconds, command, argsCopy}) + fake.executeWithTimeoutMutex.Unlock() + if fake.ExecuteWithTimeoutStub != nil { + return fake.ExecuteWithTimeoutStub(mSeconds, command, args) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fake.executeWithTimeoutReturns.result1, fake.executeWithTimeoutReturns.result2 +} + +func (fake *FakeExecutor) ExecuteWithTimeoutCallCount() int { + fake.executeWithTimeoutMutex.RLock() + defer fake.executeWithTimeoutMutex.RUnlock() + return len(fake.executeWithTimeoutArgsForCall) +} + +func (fake *FakeExecutor) ExecuteWithTimeoutArgsForCall(i int) (int, string, []string) { + fake.executeWithTimeoutMutex.RLock() + defer fake.executeWithTimeoutMutex.RUnlock() + return fake.executeWithTimeoutArgsForCall[i].mSeconds, fake.executeWithTimeoutArgsForCall[i].command, fake.executeWithTimeoutArgsForCall[i].args +} + +func (fake *FakeExecutor) ExecuteWithTimeoutReturns(result1 []byte, result2 error) { + fake.ExecuteWithTimeoutStub = nil + fake.executeWithTimeoutReturns = struct { + result1 []byte + result2 error + }{result1, result2} +} + +func (fake *FakeExecutor) ExecuteWithTimeoutReturnsOnCall(i int, result1 []byte, result2 error) { + fake.ExecuteWithTimeoutStub = nil + if fake.executeWithTimeoutReturnsOnCall == nil { + fake.executeWithTimeoutReturnsOnCall = make(map[int]struct { + result1 []byte + result2 error + }) + } + fake.executeWithTimeoutReturnsOnCall[i] = struct { + result1 []byte + result2 error + }{result1, result2} +} + func (fake *FakeExecutor) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -663,6 +735,8 @@ func (fake *FakeExecutor) Invocations() map[string][][]interface{} { defer fake.isNotExistMutex.RUnlock() fake.evalSymlinksMutex.RLock() defer fake.evalSymlinksMutex.RUnlock() + fake.executeWithTimeoutMutex.RLock() + defer fake.executeWithTimeoutMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/remote/mounter/block_device_utils/block_device_utils_test.go b/remote/mounter/block_device_utils/block_device_utils_test.go index d5ffce03..84c49c7b 100644 --- a/remote/mounter/block_device_utils/block_device_utils_test.go +++ b/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})) }) diff --git a/remote/mounter/block_device_utils/mpath.go b/remote/mounter/block_device_utils/mpath.go index 0cbe6ff1..8f380f79 100644 --- a/remote/mounter/block_device_utils/mpath.go +++ b/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") } diff --git a/utils/executor.go b/utils/executor.go index 1cb458c4..f87ba10f 100644 --- a/utils/executor.go +++ b/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) }