builder/digitalocean: retry on any pending event errors

/cc @pearkes - I hate this thing.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 IMPROVEMENTS:
 
+* builder/digitalocean: Retry on any pending event errors.
 * builder/openstack: Can now specify a project. [GH-382]
 
 BUG FIXES:

builder/digitalocean/api.go

@@ -14,6 +14,7 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+	"time"
 )
 
 const DIGITALOCEAN_API_URL = "https://api.digitalocean.com"
@@ -191,46 +192,57 @@ func NewRequest(d DigitalOceanClient, path string, params url.Values) (map[strin
 
 	url := fmt.Sprintf("%s/%s?%s", DIGITALOCEAN_API_URL, path, params.Encode())
 
-	var decodedResponse map[string]interface{}
-
 	// Do some basic scrubbing so sensitive information doesn't appear in logs
 	scrubbedUrl := strings.Replace(url, d.ClientID, "CLIENT_ID", -1)
 	scrubbedUrl = strings.Replace(scrubbedUrl, d.APIKey, "API_KEY", -1)
 	log.Printf("sending new request to digitalocean: %s", scrubbedUrl)
 
-	resp, err := client.Get(url)
+	var lastErr error
-	if err != nil {
+	for attempts := 1; attempts < 5; attempts++ {
-		return decodedResponse, err
+		resp, err := client.Get(url)
-	}
+		if err != nil {
-
+			return nil, err
-	body, err := ioutil.ReadAll(resp.Body)
-
-	resp.Body.Close()
-	if err != nil {
-		return decodedResponse, err
-	}
-
-	log.Printf("response from digitalocean: %s", body)
-
-	err = json.Unmarshal(body, &decodedResponse)
-
-	// Check for bad JSON
-	if err != nil {
-		err = errors.New(fmt.Sprintf("Failed to decode JSON response (HTTP %v) from DigitalOcean: %s",
-			resp.StatusCode, body))
-		return decodedResponse, err
-	}
-
-	// Check for errors sent by digitalocean
-	status := decodedResponse["status"]
-	if status != "OK" {
-		// Get the actual error message if there is one
-		if status == "ERROR" {
-			status = decodedResponse["error_message"]
 		}
-		err = errors.New(fmt.Sprintf("Received bad response (HTTP %v) from DigitalOcean: %s", resp.StatusCode, status))
+
-		return decodedResponse, err
+		body, err := ioutil.ReadAll(resp.Body)
+		resp.Body.Close()
+		if err != nil {
+			return nil, err
+		}
+
+		log.Printf("response from digitalocean: %s", body)
+
+		var decodedResponse map[string]interface{}
+		err = json.Unmarshal(body, &decodedResponse)
+		if err != nil {
+			err = errors.New(fmt.Sprintf("Failed to decode JSON response (HTTP %v) from DigitalOcean: %s",
+				resp.StatusCode, body))
+			return decodedResponse, err
+		}
+
+		// Check for errors sent by digitalocean
+		status := decodedResponse["status"].(string)
+		if status == "OK" {
+			return decodedResponse, nil
+		}
+
+		if status == "ERROR" {
+			status = decodedResponse["error_message"].(string)
+		}
+
+		lastErr = errors.New(fmt.Sprintf("Received error from DigitalOcean (%d): %s",
+			resp.StatusCode, status))
+		log.Println(lastErr)
+		if strings.Contains(status, "has a pending event") {
+			// Retry, DigitalOcean sends these dumb "pending event"
+			// errors all the time.
+			time.Sleep(5 * time.Second)
+			continue
+		}
+
+		// Some other kind of error. Just return.
+		return decodedResponse, lastErr
 	}
 
-	return decodedResponse, nil
+	return nil, lastErr
 }

builder/digitalocean/builder.go

@@ -34,13 +34,11 @@ type config struct {
 	SSHPort      uint   `mapstructure:"ssh_port"`
 
 	RawSSHTimeout   string `mapstructure:"ssh_timeout"`
-	RawEventDelay   string `mapstructure:"event_delay"`
 	RawStateTimeout string `mapstructure:"state_timeout"`
 
 	// These are unexported since they're set by other fields
 	// being set.
 	sshTimeout   time.Duration
-	eventDelay   time.Duration
 	stateTimeout time.Duration
 
 	tpl *packer.ConfigTemplate
@@ -113,12 +111,6 @@ func (b *Builder) Prepare(raws ...interface{}) error {
 		b.config.RawSSHTimeout = "1m"
 	}
 
-	if b.config.RawEventDelay == "" {
-		// Default to 5 second delays after creating events
-		// to allow DO to process
-		b.config.RawEventDelay = "5s"
-	}
-
 	if b.config.RawStateTimeout == "" {
 		// Default to 6 minute timeouts waiting for
 		// desired state. i.e waiting for droplet to become active
@@ -131,7 +123,6 @@ func (b *Builder) Prepare(raws ...interface{}) error {
 		"snapshot_name": &b.config.SnapshotName,
 		"ssh_username":  &b.config.SSHUsername,
 		"ssh_timeout":   &b.config.RawSSHTimeout,
-		"event_delay":   &b.config.RawEventDelay,
 		"state_timeout": &b.config.RawStateTimeout,
 	}
 
@@ -162,13 +153,6 @@ func (b *Builder) Prepare(raws ...interface{}) error {
 	}
 	b.config.sshTimeout = sshTimeout
 
-	eventDelay, err := time.ParseDuration(b.config.RawEventDelay)
-	if err != nil {
-		errs = packer.MultiErrorAppend(
-			errs, fmt.Errorf("Failed parsing event_delay: %s", err))
-	}
-	b.config.eventDelay = eventDelay
-
 	stateTimeout, err := time.ParseDuration(b.config.RawStateTimeout)
 	if err != nil {
 		errs = packer.MultiErrorAppend(

builder/digitalocean/builder_test.go

@@ -258,38 +258,6 @@ func TestBuilderPrepare_SSHTimeout(t *testing.T) {
 
 }
 
-func TestBuilderPrepare_EventDelay(t *testing.T) {
-	var b Builder
-	config := testConfig()
-
-	// Test default
-	err := b.Prepare(config)
-	if err != nil {
-		t.Fatalf("should not have error: %s", err)
-	}
-
-	if b.config.RawEventDelay != "5s" {
-		t.Errorf("invalid: %d", b.config.RawEventDelay)
-	}
-
-	// Test set
-	config["event_delay"] = "10s"
-	b = Builder{}
-	err = b.Prepare(config)
-	if err != nil {
-		t.Fatalf("should not have error: %s", err)
-	}
-
-	// Test bad
-	config["event_delay"] = "tubes"
-	b = Builder{}
-	err = b.Prepare(config)
-	if err == nil {
-		t.Fatal("should have error")
-	}
-
-}
-
 func TestBuilderPrepare_StateTimeout(t *testing.T) {
 	var b Builder
 	config := testConfig()

builder/digitalocean/step_create_droplet.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
-	"log"
 	"time"
 )
 
@@ -56,12 +55,6 @@ func (s *stepCreateDroplet) Cleanup(state multistep.StateBag) {
 	// Destroy the droplet we just created
 	ui.Say("Destroying droplet...")
 
-	// Sleep arbitrarily before sending destroy request
-	// Otherwise we get "pending event" errors, even though there isn't
-	// one.
-	log.Printf("Sleeping for %v, event_delay", c.RawEventDelay)
-	time.Sleep(c.eventDelay)
-
 	var err error
 	for i := 0; i < 5; i++ {
 		err = client.DestroyDroplet(s.dropletId)

builder/digitalocean/step_power_off.go

@@ -5,26 +5,17 @@ import (
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 	"log"
-	"time"
 )
 
 type stepPowerOff struct{}
 
 func (s *stepPowerOff) Run(state multistep.StateBag) multistep.StepAction {
 	client := state.Get("client").(*DigitalOceanClient)
-	c := state.Get("config").(config)
 	ui := state.Get("ui").(packer.Ui)
 	dropletId := state.Get("droplet_id").(uint)
 
-	// Sleep arbitrarily before sending power off request
-	// Otherwise we get "pending event" errors, even though there isn't
-	// one.
-	log.Printf("Sleeping for %v, event_delay", c.RawEventDelay)
-	time.Sleep(c.eventDelay)
-
 	// Poweroff the droplet so it can be snapshot
 	err := client.PowerOffDroplet(dropletId)
-
 	if err != nil {
 		err := fmt.Errorf("Error powering off droplet: %s", err)
 		state.Put("error", err)
@@ -33,14 +24,6 @@ func (s *stepPowerOff) Run(state multistep.StateBag) multistep.StepAction {
 	}
 
 	log.Println("Waiting for poweroff event to complete...")
-
-	// This arbitrary sleep is because we can't wait for the state
-	// of the droplet to be 'off', as stepShutdown should already
-	// have accomplished that, and the state indicator is the same.
-	// We just have to assume that this event will process quickly.
-	log.Printf("Sleeping for %v, event_delay", c.RawEventDelay)
-	time.Sleep(c.eventDelay)
-
 	return multistep.ActionContinue
 }
 

builder/digitalocean/step_shutdown.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
-	"log"
-	"time"
 )
 
 type stepShutdown struct{}
@@ -16,14 +14,7 @@ func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 	ui := state.Get("ui").(packer.Ui)
 	dropletId := state.Get("droplet_id").(uint)
 
-	// Sleep arbitrarily before sending the request
-	// Otherwise we get "pending event" errors, even though there isn't
-	// one.
-	log.Printf("Sleeping for %v, event_delay", c.RawEventDelay)
-	time.Sleep(c.eventDelay)
-
 	err := client.ShutdownDroplet(dropletId)
-
 	if err != nil {
 		err := fmt.Errorf("Error shutting down droplet: %s", err)
 		state.Put("error", err)
@@ -32,7 +23,6 @@ func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 	}
 
 	ui.Say("Waiting for droplet to shutdown...")
-
 	err = waitForDropletState("off", dropletId, client, c)
 	if err != nil {
 		err := fmt.Errorf("Error waiting for droplet to become 'off': %s", err)

builder/digitalocean/wait.go

@@ -1,7 +1,7 @@
 package digitalocean
 
 import (
-	"errors"
+	"fmt"
 	"log"
 	"time"
 )
@@ -9,8 +9,7 @@ import (
 // waitForState simply blocks until the droplet is in
 // a state we expect, while eventually timing out.
 func waitForDropletState(desiredState string, dropletId uint, client *DigitalOceanClient, c config) error {
-	active := make(chan bool, 1)
+	result := make(chan error, 1)
-
 	go func() {
 		attempts := 0
 		for {
@@ -19,36 +18,26 @@ func waitForDropletState(desiredState string, dropletId uint, client *DigitalOce
 			log.Printf("Checking droplet status... (attempt: %d)", attempts)
 			_, status, err := client.DropletStatus(dropletId)
 			if err != nil {
-				log.Println(err)
+				result <- err
-				break
+				return
 			}
 
 			if status == desiredState {
-				break
+				result <- nil
+				return
 			}
 
 			// Wait 3 seconds in between
 			time.Sleep(3 * time.Second)
 		}
-
-		active <- true
 	}()
 
 	log.Printf("Waiting for up to %s for droplet to become %s", c.RawStateTimeout, desiredState)
-	timeout := time.After(c.stateTimeout)
+	select {
-
+	case err := <-result:
-ActiveWaitLoop:
+		return err
-	for {
+	case <-time.After(c.stateTimeout):
-		select {
+		err := fmt.Errorf("Timeout while waiting to for droplet to become '%s'", desiredState)
-		case <-active:
+		return err
-			// We connected. Just break the loop.
-			break ActiveWaitLoop
-		case <-timeout:
-			err := errors.New("Timeout while waiting to for droplet to become active")
-			return err
-		}
 	}
-
-	// If we got this far, there were no errors
-	return nil
 }