Merge pull request #2763 from dgageot/retry-vbox

Retry VirtualBox commands

drivers/virtualbox/vbm.go

@@ -11,9 +11,17 @@ import (
 
 	"strconv"
 
+	"time"
+
 	"github.com/docker/machine/libmachine/log"
 )
 
+const (
+	retryCountOnObjectNotReadyError = 5
+	objectNotReady                  = "error: The object is not ready"
+	retryDelay                      = 100 * time.Millisecond
+)
+
 var (
 	reColonLine       = regexp.MustCompile(`(.+):\s+(.*)`)
 	reEqualLine       = regexp.MustCompile(`(.+)=(.*)`)
@@ -36,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...)
@@ -49,13 +66,17 @@ 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
 	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())
@@ -68,6 +89,14 @@ func (v *VBoxCmdManager) vbmOutErr(args ...string) (string, string, error) {
 		}
 	}
 
+	// Sometimes, we just need to retry...
+	if retry > 1 {
+		if strings.Contains(stderrStr, objectNotReady) {
+			time.Sleep(retryDelay)
+			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)

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)
+}

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,