From fcb31f99f2edfdc19a6cf7592db2214726856e85 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 6 Jan 2016 13:36:39 +0100 Subject: [PATCH 1/2] FIX #2762 retry Virtualbox commands Signed-off-by: David Gageot --- drivers/virtualbox/vbm.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/virtualbox/vbm.go b/drivers/virtualbox/vbm.go index c2b5180acd..632b61ddbc 100644 --- a/drivers/virtualbox/vbm.go +++ b/drivers/virtualbox/vbm.go @@ -14,6 +14,10 @@ import ( "github.com/docker/machine/libmachine/log" ) +const ( + retryCountOnObjectNotReadyError = 5 +) + var ( reColonLine = regexp.MustCompile(`(.+):\s+(.*)`) reEqualLine = regexp.MustCompile(`(.+)=(.*)`) @@ -49,6 +53,10 @@ func (v *VBoxCmdManager) vbmOut(args ...string) (string, error) { } func (v *VBoxCmdManager) vbmOutErr(args ...string) (string, string, error) { + return v.vbmOutErrRetry(retryCountOnObjectNotReadyError, args...) +} + +func (v *VBoxCmdManager) vbmOutErrRetry(retry int, args ...string) (string, string, error) { cmd := exec.Command(vboxManageCmd, args...) log.Debugf("COMMAND: %v %v", vboxManageCmd, strings.Join(args, " ")) var stdout bytes.Buffer @@ -68,6 +76,13 @@ func (v *VBoxCmdManager) vbmOutErr(args ...string) (string, string, error) { } } + // Sometimes, we just need to retry... + if retry > 0 { + if strings.Contains(stderrStr, "error: The object is not ready") { + return v.vbmOutErrRetry(retry-1, args...) + } + } + if err == nil || strings.HasPrefix(err.Error(), "exit status ") { // VBoxManage will sometimes not set the return code, but has a fatal error // such as VBoxManage.exe: error: VT-x is not available. (VERR_VMX_NO_VMX) From 6dc080167465e04068b3abe469a5d18368bc9044 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 7 Jan 2016 09:19:08 +0100 Subject: [PATCH 2/2] Add unit tests for vbm Signed-off-by: David Gageot --- drivers/virtualbox/vbm.go | 22 ++++++-- drivers/virtualbox/vbm_test.go | 97 ++++++++++++++++++++++++++++++++ drivers/virtualbox/virtualbox.go | 2 +- 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/drivers/virtualbox/vbm.go b/drivers/virtualbox/vbm.go index 632b61ddbc..36e6f40e0e 100644 --- a/drivers/virtualbox/vbm.go +++ b/drivers/virtualbox/vbm.go @@ -11,11 +11,15 @@ import ( "strconv" + "time" + "github.com/docker/machine/libmachine/log" ) const ( retryCountOnObjectNotReadyError = 5 + objectNotReady = "error: The object is not ready" + retryDelay = 100 * time.Millisecond ) var ( @@ -40,7 +44,16 @@ type VBoxManager interface { } // VBoxCmdManager communicates with VirtualBox through the commandline using `VBoxManage`. -type VBoxCmdManager struct{} +type VBoxCmdManager struct { + runCmd func(cmd *exec.Cmd) error +} + +// NewVBoxManager creates a VBoxManager instance. +func NewVBoxManager() *VBoxCmdManager { + return &VBoxCmdManager{ + runCmd: func(cmd *exec.Cmd) error { return cmd.Run() }, + } +} func (v *VBoxCmdManager) vbm(args ...string) error { _, _, err := v.vbmOutErr(args...) @@ -63,7 +76,7 @@ func (v *VBoxCmdManager) vbmOutErrRetry(retry int, args ...string) (string, stri var stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr - err := cmd.Run() + err := v.runCmd(cmd) stderrStr := stderr.String() if len(args) > 0 { log.Debugf("STDOUT:\n{\n%v}", stdout.String()) @@ -77,8 +90,9 @@ func (v *VBoxCmdManager) vbmOutErrRetry(retry int, args ...string) (string, stri } // Sometimes, we just need to retry... - if retry > 0 { - if strings.Contains(stderrStr, "error: The object is not ready") { + if retry > 1 { + if strings.Contains(stderrStr, objectNotReady) { + time.Sleep(retryDelay) return v.vbmOutErrRetry(retry-1, args...) } } diff --git a/drivers/virtualbox/vbm_test.go b/drivers/virtualbox/vbm_test.go index 8b6caf2ebe..a1baa1b34f 100644 --- a/drivers/virtualbox/vbm_test.go +++ b/drivers/virtualbox/vbm_test.go @@ -3,6 +3,12 @@ package virtualbox import ( "testing" + "os/exec" + + "errors" + + "fmt" + "github.com/stretchr/testify/assert" ) @@ -43,3 +49,94 @@ func TestInvalidCheckVBoxManageVersion(t *testing.T) { assert.EqualError(t, err, test.expectedError) } } + +func TestVbmOutErr(t *testing.T) { + var cmdRun *exec.Cmd + vBoxManager := NewVBoxManager() + vBoxManager.runCmd = func(cmd *exec.Cmd) error { + cmdRun = cmd + fmt.Fprint(cmd.Stdout, "Printed to StdOut") + fmt.Fprint(cmd.Stderr, "Printed to StdErr") + return nil + } + + stdOut, stdErr, err := vBoxManager.vbmOutErr("arg1", "arg2") + + assert.Equal(t, []string{vboxManageCmd, "arg1", "arg2"}, cmdRun.Args) + assert.Equal(t, "Printed to StdOut", stdOut) + assert.Equal(t, "Printed to StdErr", stdErr) + assert.NoError(t, err) +} + +func TestVbmOutErrError(t *testing.T) { + vBoxManager := NewVBoxManager() + vBoxManager.runCmd = func(cmd *exec.Cmd) error { return errors.New("BUG") } + + _, _, err := vBoxManager.vbmOutErr("arg1", "arg2") + + assert.EqualError(t, err, "BUG") +} + +func TestVbmOutErrNotFound(t *testing.T) { + vBoxManager := NewVBoxManager() + vBoxManager.runCmd = func(cmd *exec.Cmd) error { return &exec.Error{Err: exec.ErrNotFound} } + + _, _, err := vBoxManager.vbmOutErr("arg1", "arg2") + + assert.Equal(t, ErrVBMNotFound, err) +} + +func TestVbmOutErrFailingWithExitStatus(t *testing.T) { + vBoxManager := NewVBoxManager() + vBoxManager.runCmd = func(cmd *exec.Cmd) error { + fmt.Fprint(cmd.Stderr, "error: Unable to run vbox") + return errors.New("exit status BUG") + } + + _, _, err := vBoxManager.vbmOutErr("arg1", "arg2", "arg3") + + assert.EqualError(t, err, vboxManageCmd+" arg1 arg2 arg3 failed:\nerror: Unable to run vbox") +} + +func TestVbmOutErrRetryOnce(t *testing.T) { + var cmdRun *exec.Cmd + var runCount int + vBoxManager := NewVBoxManager() + vBoxManager.runCmd = func(cmd *exec.Cmd) error { + cmdRun = cmd + + runCount++ + if runCount == 1 { + fmt.Fprint(cmd.Stderr, "error: The object is not ready") + return errors.New("Fail the first time it's called") + } + + fmt.Fprint(cmd.Stdout, "Printed to StdOut") + return nil + } + + stdOut, stdErr, err := vBoxManager.vbmOutErr("command", "arg") + + assert.Equal(t, 2, runCount) + assert.Equal(t, []string{vboxManageCmd, "command", "arg"}, cmdRun.Args) + assert.Equal(t, "Printed to StdOut", stdOut) + assert.Empty(t, stdErr) + assert.NoError(t, err) +} + +func TestVbmOutErrRetryMax(t *testing.T) { + var runCount int + vBoxManager := NewVBoxManager() + vBoxManager.runCmd = func(cmd *exec.Cmd) error { + runCount++ + fmt.Fprint(cmd.Stderr, "error: The object is not ready") + return errors.New("Always fail") + } + + stdOut, stdErr, err := vBoxManager.vbmOutErr("command", "arg") + + assert.Equal(t, 5, runCount) + assert.Empty(t, stdOut) + assert.Equal(t, "error: The object is not ready", stdErr) + assert.Error(t, err) +} diff --git a/drivers/virtualbox/virtualbox.go b/drivers/virtualbox/virtualbox.go index 4cafcbd23a..da61b8538d 100644 --- a/drivers/virtualbox/virtualbox.go +++ b/drivers/virtualbox/virtualbox.go @@ -62,7 +62,7 @@ type Driver struct { // NewDriver creates a new VirtualBox driver with default settings. func NewDriver(hostName, storePath string) *Driver { return &Driver{ - VBoxManager: &VBoxCmdManager{}, + VBoxManager: NewVBoxManager(), BaseDriver: &drivers.BaseDriver{ MachineName: hostName, StorePath: storePath,