Merge pull request #5512 from hashicorp/fix5501

builder/virtualbox-ovf retry removing VM.

builder/virtualbox/common/driver_4_2.go

@@ -9,6 +9,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	packer "github.com/hashicorp/packer/common"
 )
 
 type VBox42Driver struct {
@@ -50,7 +52,15 @@ func (d *VBox42Driver) CreateSCSIController(vmName string, name string) error {
 }
 
 func (d *VBox42Driver) Delete(name string) error {
-	return d.VBoxManage("unregistervm", name, "--delete")
+	return packer.Retry(1, 1, 5, func(i uint) (bool, error) {
+		if err := d.VBoxManage("unregistervm", name, "--delete"); err != nil {
+			if i+1 == 5 {
+				return false, err
+			}
+			return false, nil
+		}
+		return true, nil
+	})
 }
 
 func (d *VBox42Driver) Iso() (string, error) {

builder/virtualbox/iso/step_create_vm.go

@@ -2,10 +2,10 @@ package iso
 
 import (
 	"fmt"
+
 	vboxcommon "github.com/hashicorp/packer/builder/virtualbox/common"
 	"github.com/hashicorp/packer/packer"
 	"github.com/mitchellh/multistep"
-	"time"
 )
 
 // This step creates the actual virtual machine.
@@ -73,18 +73,8 @@ func (s *stepCreateVM) Cleanup(state multistep.StateBag) {
 		return
 	}
 
-	ui.Say("Unregistering and deleting virtual machine...")
-	var err error = nil
-	for i := 0; i < 5; i++ {
-		err = driver.Delete(s.vmName)
-		if err == nil {
-			break
-		}
-
-		time.Sleep(1 * time.Second * time.Duration(i))
-	}
-
-	if err != nil {
-		ui.Error(fmt.Sprintf("Error deleting virtual machine: %s", err))
+	ui.Say("Deregistering and deleting VM...")
+	if err := driver.Delete(s.vmName); err != nil {
+		ui.Error(fmt.Sprintf("Error deleting VM: %s", err))
 	}
 }

builder/virtualbox/ovf/step_import.go

@@ -2,6 +2,7 @@ package ovf
 
 import (
 	"fmt"
+
 	vboxcommon "github.com/hashicorp/packer/builder/virtualbox/common"
 	"github.com/hashicorp/packer/packer"
 	"github.com/mitchellh/multistep"
@@ -49,7 +50,7 @@ func (s *StepImport) Cleanup(state multistep.StateBag) {
 		return
 	}
 
-	ui.Say("Unregistering and deleting imported VM...")
+	ui.Say("Deregistering and deleting imported VM...")
 	if err := driver.Delete(s.vmName); err != nil {
 		ui.Error(fmt.Sprintf("Error deleting VM: %s", err))
 	}

common/retry.go

@@ -9,17 +9,21 @@ import (
 var RetryExhaustedError error = fmt.Errorf("Function never succeeded in Retry")
 
 // RetryableFunc performs an action and returns a bool indicating whether the
-// function is done, or if it should keep retrying, and an erorr which will
+// function is done, or if it should keep retrying, and an error which will
 // abort the retry and be returned by the Retry function. The 0-indexed attempt
 // is passed with each call.
 type RetryableFunc func(uint) (bool, error)
 
-// Retry retries a function up to numTries times with exponential backoff.
-// If numTries == 0, retry indefinitely. If interval == 0, Retry will not delay retrying and there will be
-// no exponential backoff. If maxInterval == 0, maxInterval is set to +Infinity.
-// Intervals are in seconds.
-// Returns an error if initial > max intervals, if retries are exhausted, or if the passed function returns
-// an error.
+/*
+Retry retries a function up to numTries times with exponential backoff.
+If numTries == 0, retry indefinitely.
+If interval == 0, Retry will not delay retrying and there will be no
+exponential backoff.
+If maxInterval == 0, maxInterval is set to +Infinity.
+Intervals are in seconds.
+Returns an error if initial > max intervals, if retries are exhausted, or if the passed function returns
+an error.
+*/
 func Retry(initialInterval float64, maxInterval float64, numTries uint, function RetryableFunc) error {
 	if maxInterval == 0 {
 		maxInterval = math.Inf(1)