From 1cbe263a72b5f0f5c7d30785c5735cf2a5dc727a Mon Sep 17 00:00:00 2001 From: Dominik Froehlich Date: Thu, 1 Mar 2018 17:38:24 +0100 Subject: [PATCH 1/4] tags should inherit timeout from tagged resources --- aws/tags.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aws/tags.go b/aws/tags.go index 6fe10873d53..78a9d591e97 100644 --- a/aws/tags.go +++ b/aws/tags.go @@ -84,7 +84,7 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { } if len(remove) > 0 { - err := resource.Retry(2*time.Minute, func() *resource.RetryError { + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { log.Printf("[DEBUG] Removing volume tags: %#v from %s", remove, d.Id()) _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ Resources: volumeIds, @@ -104,7 +104,7 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { } } if len(create) > 0 { - err := resource.Retry(2*time.Minute, func() *resource.RetryError { + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { log.Printf("[DEBUG] Creating vol tags: %s for %s", create, d.Id()) _, err := conn.CreateTags(&ec2.CreateTagsInput{ Resources: volumeIds, @@ -139,7 +139,7 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { // Set tags if len(remove) > 0 { - err := resource.Retry(5*time.Minute, func() *resource.RetryError { + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ Resources: []*string{aws.String(d.Id())}, @@ -159,7 +159,7 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { } } if len(create) > 0 { - err := resource.Retry(5*time.Minute, func() *resource.RetryError { + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) _, err := conn.CreateTags(&ec2.CreateTagsInput{ Resources: []*string{aws.String(d.Id())}, From db7189e97d5f5c10ebb334720b45e627549861d8 Mon Sep 17 00:00:00 2001 From: Dominik Froehlich Date: Mon, 12 Nov 2018 16:23:13 +0100 Subject: [PATCH 2/4] Implement retry on tags using isResourceTimeoutError --- aws/tags.go | 108 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 96 insertions(+), 12 deletions(-) diff --git a/aws/tags.go b/aws/tags.go index 78a9d591e97..694a096df28 100644 --- a/aws/tags.go +++ b/aws/tags.go @@ -84,7 +84,7 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { } if len(remove) > 0 { - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + err := resource.Retry(2*time.Minute, func() *resource.RetryError { log.Printf("[DEBUG] Removing volume tags: %#v from %s", remove, d.Id()) _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ Resources: volumeIds, @@ -93,18 +93,39 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) // retry + return resource.RetryableError(err) } return resource.NonRetryableError(err) } return nil }) if err != nil { - return err + if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + return err + } + // Allow additional time for slower uploads or EC2 throttling + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + log.Printf("[DEBUG] Removing volume tags: %#v from %s", remove, d.Id()) + _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ + Resources: volumeIds, + Tags: remove, + }) + if err != nil { + if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + log.Printf("[DEBUG] Received %s, retrying DeleteTags", err) + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { + return err + } } } if len(create) > 0 { - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + err := resource.Retry(2*time.Minute, func() *resource.RetryError { log.Printf("[DEBUG] Creating vol tags: %s for %s", create, d.Id()) _, err := conn.CreateTags(&ec2.CreateTagsInput{ Resources: volumeIds, @@ -113,14 +134,35 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) // retry + return resource.RetryableError(err) } return resource.NonRetryableError(err) } return nil }) if err != nil { - return err + if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + return err + } + // Allow additional time for slower uploads or EC2 throttling + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + log.Printf("[DEBUG] Creating vol tags: %s for %s", create, d.Id()) + _, err := conn.CreateTags(&ec2.CreateTagsInput{ + Resources: volumeIds, + Tags: create, + }) + if err != nil { + if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + log.Printf("[DEBUG] Received %s, retrying CreateTags", err) + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { + return err + } } } } @@ -139,7 +181,7 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { // Set tags if len(remove) > 0 { - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + err := resource.Retry(2*time.Minute, func() *resource.RetryError { log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ Resources: []*string{aws.String(d.Id())}, @@ -148,18 +190,39 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) // retry + return resource.RetryableError(err) } return resource.NonRetryableError(err) } return nil }) if err != nil { - return err + if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + return err + } + // Allow additional time for slower uploads or EC2 throttling + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) + _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(d.Id())}, + Tags: remove, + }) + if err != nil { + if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + log.Printf("[DEBUG] Received %s, retrying DeleteTags", err) + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { + return err + } } } if len(create) > 0 { - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + err := resource.Retry(2*time.Minute, func() *resource.RetryError { log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) _, err := conn.CreateTags(&ec2.CreateTagsInput{ Resources: []*string{aws.String(d.Id())}, @@ -168,14 +231,35 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) // retry + return resource.RetryableError(err) } return resource.NonRetryableError(err) } return nil }) if err != nil { - return err + if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + return err + } + // Allow additional time for slower uploads or EC2 throttling + err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) + _, err := conn.CreateTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(d.Id())}, + Tags: create, + }) + if err != nil { + if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { + log.Printf("[DEBUG] Received %s, retrying CreateTags", err) + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { + return err + } } } } From 1256563bd748218d17ceaf385639882e25daddd6 Mon Sep 17 00:00:00 2001 From: Dominik Froehlich Date: Wed, 14 Nov 2018 08:38:13 +0100 Subject: [PATCH 3/4] Retry without time bounds on EC2 throttling --- aws/tags.go | 80 ++++++++++++----------------------------------------- 1 file changed, 18 insertions(+), 62 deletions(-) diff --git a/aws/tags.go b/aws/tags.go index 694a096df28..6e85d8be3f3 100644 --- a/aws/tags.go +++ b/aws/tags.go @@ -93,34 +93,23 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) + return resource.RetryableError(err) // retry } return resource.NonRetryableError(err) } return nil }) if err != nil { - if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - return err - } - // Allow additional time for slower uploads or EC2 throttling - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + // Retry without time bounds for EC2 throttling + if isResourceTimeoutError(err) { log.Printf("[DEBUG] Removing volume tags: %#v from %s", remove, d.Id()) _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ Resources: volumeIds, Tags: remove, }) if err != nil { - if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - log.Printf("[DEBUG] Received %s, retrying DeleteTags", err) - return resource.RetryableError(err) - } - return resource.NonRetryableError(err) + return err } - return nil - }) - if err != nil { - return err } } } @@ -134,34 +123,23 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) + return resource.RetryableError(err) // retry } return resource.NonRetryableError(err) } return nil }) if err != nil { - if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - return err - } - // Allow additional time for slower uploads or EC2 throttling - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + // Retry without time bounds for EC2 throttling + if isResourceTimeoutError(err) { log.Printf("[DEBUG] Creating vol tags: %s for %s", create, d.Id()) _, err := conn.CreateTags(&ec2.CreateTagsInput{ Resources: volumeIds, Tags: create, }) if err != nil { - if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - log.Printf("[DEBUG] Received %s, retrying CreateTags", err) - return resource.RetryableError(err) - } - return resource.NonRetryableError(err) + return err } - return nil - }) - if err != nil { - return err } } } @@ -181,7 +159,7 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { // Set tags if len(remove) > 0 { - err := resource.Retry(2*time.Minute, func() *resource.RetryError { + err := resource.Retry(5*time.Minute, func() *resource.RetryError { log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ Resources: []*string{aws.String(d.Id())}, @@ -190,39 +168,28 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) + return resource.RetryableError(err) // retry } return resource.NonRetryableError(err) } return nil }) if err != nil { - if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - return err - } - // Allow additional time for slower uploads or EC2 throttling - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + // Retry without time bounds for EC2 throttling + if isResourceTimeoutError(err) { log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) _, err := conn.DeleteTags(&ec2.DeleteTagsInput{ Resources: []*string{aws.String(d.Id())}, Tags: remove, }) if err != nil { - if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - log.Printf("[DEBUG] Received %s, retrying DeleteTags", err) - return resource.RetryableError(err) - } - return resource.NonRetryableError(err) + return err } - return nil - }) - if err != nil { - return err } } } if len(create) > 0 { - err := resource.Retry(2*time.Minute, func() *resource.RetryError { + err := resource.Retry(5*time.Minute, func() *resource.RetryError { log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) _, err := conn.CreateTags(&ec2.CreateTagsInput{ Resources: []*string{aws.String(d.Id())}, @@ -231,34 +198,23 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { ec2err, ok := err.(awserr.Error) if ok && strings.Contains(ec2err.Code(), ".NotFound") { - return resource.RetryableError(err) + return resource.RetryableError(err) // retry } return resource.NonRetryableError(err) } return nil }) if err != nil { - if !isResourceTimeoutError(err) && !isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - return err - } - // Allow additional time for slower uploads or EC2 throttling - err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + // Retry without time bounds for EC2 throttling + if isResourceTimeoutError(err) { log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) _, err := conn.CreateTags(&ec2.CreateTagsInput{ Resources: []*string{aws.String(d.Id())}, Tags: create, }) if err != nil { - if isAWSErr(err, "InvalidParameterValueException", "Your request has been throttled by EC2") { - log.Printf("[DEBUG] Received %s, retrying CreateTags", err) - return resource.RetryableError(err) - } - return resource.NonRetryableError(err) + return err } - return nil - }) - if err != nil { - return err } } } From 5df2e9affd2b70bc221cd6ea6e877793bee088e6 Mon Sep 17 00:00:00 2001 From: Dominik Froehlich Date: Wed, 14 Nov 2018 17:16:50 +0100 Subject: [PATCH 4/4] Return err if errored but not isResourceTimeoutError --- aws/tags.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/aws/tags.go b/aws/tags.go index 6e85d8be3f3..f716b5c0d77 100644 --- a/aws/tags.go +++ b/aws/tags.go @@ -110,6 +110,8 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { return err } + } else { + return err } } } @@ -140,6 +142,8 @@ func setVolumeTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { return err } + } else { + return err } } } @@ -185,6 +189,8 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { return err } + } else { + return err } } } @@ -215,6 +221,8 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error { if err != nil { return err } + } else { + return err } } }