From f2e909356e4962eec1540eeff95100ec9c8e7c03 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 4 Sep 2013 21:20:41 -0700 Subject: [PATCH] builder/digitalocean: retry on any pending event errors /cc @pearkes - I hate this thing. --- CHANGELOG.md | 1 + builder/digitalocean/api.go | 80 ++++++++++++--------- builder/digitalocean/builder.go | 16 ----- builder/digitalocean/builder_test.go | 32 --------- builder/digitalocean/step_create_droplet.go | 7 -- builder/digitalocean/step_power_off.go | 17 ----- builder/digitalocean/step_shutdown.go | 10 --- builder/digitalocean/wait.go | 35 ++++----- 8 files changed, 59 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 947c655b2..f39554ab0 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/builder/digitalocean/api.go b/builder/digitalocean/api.go index fbd1b7d50..384b4e8c9 100644 --- a/builder/digitalocean/api.go +++ b/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) - if err != nil { - return decodedResponse, 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"] + var lastErr error + for attempts := 1; attempts < 5; attempts++ { + resp, err := client.Get(url) + if err != nil { + return nil, err } - 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 } diff --git a/builder/digitalocean/builder.go b/builder/digitalocean/builder.go index 25129e01a..8e74ec9b1 100644 --- a/builder/digitalocean/builder.go +++ b/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( diff --git a/builder/digitalocean/builder_test.go b/builder/digitalocean/builder_test.go index af84323ab..cd19f0f31 100644 --- a/builder/digitalocean/builder_test.go +++ b/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() diff --git a/builder/digitalocean/step_create_droplet.go b/builder/digitalocean/step_create_droplet.go index dcc369ac4..2bdc321b2 100644 --- a/builder/digitalocean/step_create_droplet.go +++ b/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) diff --git a/builder/digitalocean/step_power_off.go b/builder/digitalocean/step_power_off.go index 553436f01..bc27fbadd 100644 --- a/builder/digitalocean/step_power_off.go +++ b/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 } diff --git a/builder/digitalocean/step_shutdown.go b/builder/digitalocean/step_shutdown.go index 31c9ae8ea..01275d47d 100644 --- a/builder/digitalocean/step_shutdown.go +++ b/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) diff --git a/builder/digitalocean/wait.go b/builder/digitalocean/wait.go index 75ad01f9d..f770eb7db 100644 --- a/builder/digitalocean/wait.go +++ b/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) - break + result <- err + 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) - -ActiveWaitLoop: - for { - select { - case <-active: - // We connected. Just break the loop. - break ActiveWaitLoop - case <-timeout: - err := errors.New("Timeout while waiting to for droplet to become active") - return err - } + select { + case err := <-result: + return err + case <-time.After(c.stateTimeout): + err := fmt.Errorf("Timeout while waiting to for droplet to become '%s'", desiredState) + return err } - - // If we got this far, there were no errors - return nil }