From 550a140db2541230eda40c7e7daeaeabd9400b1b Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 9 Nov 2018 16:21:19 -0500 Subject: [PATCH 1/2] resource/aws_ebs_snapshot: Allow retries for `SnapshotCreationPerVolumeRateExceeded` errors on creation Found via acceptance testing: ``` --- FAIL: TestAccAWSEbsSnapshotDataSource_MostRecent (27.42s) testing.go:538: Step 0 error: Error applying: 1 error occurred: * aws_ebs_snapshot.test: 1 error occurred: * aws_ebs_snapshot.test: SnapshotCreationPerVolumeRateExceeded: The maximum per volume CreateSnapshot request rate has been exceeded. Use an increasing or variable sleep interval between requests. ``` ``` --- PASS: TestAccAWSEBSSnapshot_withDescription (26.62s) --- PASS: TestAccAWSEbsSnapshotDataSource_basic (28.90s) --- PASS: TestAccAWSEbsSnapshotDataSource_Filter (43.31s) --- PASS: TestAccAWSEBSSnapshot_withKms (67.28s) --- PASS: TestAccAWSEbsSnapshotDataSource_MostRecent (68.51s) --- PASS: TestAccAWSEBSSnapshot_basic (85.15s) ``` --- aws/resource_aws_ebs_snapshot.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_ebs_snapshot.go b/aws/resource_aws_ebs_snapshot.go index 8e6eaba7fcde..7821ece72962 100644 --- a/aws/resource_aws_ebs_snapshot.go +++ b/aws/resource_aws_ebs_snapshot.go @@ -73,9 +73,24 @@ func resourceAwsEbsSnapshotCreate(d *schema.ResourceData, meta interface{}) erro request.Description = aws.String(v.(string)) } - res, err := conn.CreateSnapshot(request) + var res *ec2.Snapshot + err := resource.Retry(1*time.Minute, func() *resource.RetryError { + var err error + res, err = conn.CreateSnapshot(request) + + if isAWSErr(err, "SnapshotCreationPerVolumeRateExceeded", "The maximum per volume CreateSnapshot request rate has been exceeded") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + if err != nil { - return err + return fmt.Errorf("error creating EC2 EBS Snapshot: %s", err) } d.SetId(*res.SnapshotId) From 2b80d65b69c8143ef821a318763e72888add8d36 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 9 Nov 2018 16:23:00 -0500 Subject: [PATCH 2/2] data-source/aws_ebs_snapshot: Fix most_recent ordering and add covering acceptance test Also refactors the testing to be region agnostic. ``` --- PASS: TestAccAWSEbsSnapshotDataSource_basic (28.90s) --- PASS: TestAccAWSEbsSnapshotDataSource_Filter (43.31s) --- PASS: TestAccAWSEbsSnapshotDataSource_MostRecent (68.51s) ``` --- aws/data_source_aws_ebs_snapshot.go | 2 +- aws/data_source_aws_ebs_snapshot_test.go | 125 +++++++++++++++++------ 2 files changed, 92 insertions(+), 35 deletions(-) diff --git a/aws/data_source_aws_ebs_snapshot.go b/aws/data_source_aws_ebs_snapshot.go index e4b989411da0..cbcd941ec4e7 100644 --- a/aws/data_source_aws_ebs_snapshot.go +++ b/aws/data_source_aws_ebs_snapshot.go @@ -129,7 +129,7 @@ func dataSourceAwsEbsSnapshotRead(d *schema.ResourceData, meta interface{}) erro "specific search criteria, or set `most_recent` attribute to true.") } sort.Slice(resp.Snapshots, func(i, j int) bool { - return aws.TimeValue(resp.Snapshots[i].StartTime).Unix() < aws.TimeValue(resp.Snapshots[j].StartTime).Unix() + return aws.TimeValue(resp.Snapshots[i].StartTime).Unix() > aws.TimeValue(resp.Snapshots[j].StartTime).Unix() }) } diff --git a/aws/data_source_aws_ebs_snapshot_test.go b/aws/data_source_aws_ebs_snapshot_test.go index 91be8d4e905b..ebe24cd59544 100644 --- a/aws/data_source_aws_ebs_snapshot_test.go +++ b/aws/data_source_aws_ebs_snapshot_test.go @@ -9,6 +9,9 @@ import ( ) func TestAccAWSEbsSnapshotDataSource_basic(t *testing.T) { + dataSourceName := "data.aws_ebs_snapshot.test" + resourceName := "aws_ebs_snapshot.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -16,27 +19,54 @@ func TestAccAWSEbsSnapshotDataSource_basic(t *testing.T) { { Config: testAccCheckAwsEbsSnapshotDataSourceConfig, Check: resource.ComposeTestCheckFunc( - testAccCheckAwsEbsSnapshotDataSourceID("data.aws_ebs_snapshot.snapshot"), - resource.TestCheckResourceAttr("data.aws_ebs_snapshot.snapshot", "volume_size", "40"), - resource.TestCheckResourceAttr("data.aws_ebs_snapshot.snapshot", "tags.%", "0"), + testAccCheckAwsEbsSnapshotDataSourceID(dataSourceName), + resource.TestCheckResourceAttrPair(dataSourceName, "id", resourceName, "id"), + resource.TestCheckResourceAttrPair(dataSourceName, "description", resourceName, "description"), + resource.TestCheckResourceAttrPair(dataSourceName, "encrypted", resourceName, "encrypted"), + resource.TestCheckResourceAttrPair(dataSourceName, "kms_key_id", resourceName, "kms_key_id"), + resource.TestCheckResourceAttrPair(dataSourceName, "owner_alias", resourceName, "owner_alias"), + resource.TestCheckResourceAttrPair(dataSourceName, "owner_id", resourceName, "owner_id"), + resource.TestCheckResourceAttrPair(dataSourceName, "tags.%", resourceName, "tags.%"), + resource.TestCheckResourceAttrPair(dataSourceName, "volume_id", resourceName, "volume_id"), + resource.TestCheckResourceAttrPair(dataSourceName, "volume_size", resourceName, "volume_size"), + ), + }, + }, + }) +} + +func TestAccAWSEbsSnapshotDataSource_Filter(t *testing.T) { + dataSourceName := "data.aws_ebs_snapshot.test" + resourceName := "aws_ebs_snapshot.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCheckAwsEbsSnapshotDataSourceConfigFilter, + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsEbsSnapshotDataSourceID(dataSourceName), + resource.TestCheckResourceAttrPair(dataSourceName, "id", resourceName, "id"), ), }, }, }) } -func TestAccAWSEbsSnapshotDataSource_multipleFilters(t *testing.T) { +func TestAccAWSEbsSnapshotDataSource_MostRecent(t *testing.T) { + dataSourceName := "data.aws_ebs_snapshot.test" + resourceName := "aws_ebs_snapshot.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccCheckAwsEbsSnapshotDataSourceConfigWithMultipleFilters, + Config: testAccCheckAwsEbsSnapshotDataSourceConfigMostRecent, Check: resource.ComposeTestCheckFunc( - testAccCheckAwsEbsSnapshotDataSourceID("data.aws_ebs_snapshot.snapshot"), - resource.TestCheckResourceAttr("data.aws_ebs_snapshot.snapshot", "volume_size", "10"), - resource.TestCheckResourceAttr("data.aws_ebs_snapshot.snapshot", "tags.%", "1"), - resource.TestCheckResourceAttr("data.aws_ebs_snapshot.snapshot", "tags.Name", "TF ACC Snapshot"), + testAccCheckAwsEbsSnapshotDataSourceID(dataSourceName), + resource.TestCheckResourceAttrPair(dataSourceName, "id", resourceName, "id"), ), }, }, @@ -58,48 +88,75 @@ func testAccCheckAwsEbsSnapshotDataSourceID(n string) resource.TestCheckFunc { } const testAccCheckAwsEbsSnapshotDataSourceConfig = ` -resource "aws_ebs_volume" "example" { - availability_zone = "us-west-2a" +data "aws_availability_zones" "available" {} + +resource "aws_ebs_volume" "test" { + availability_zone = "${data.aws_availability_zones.available.names[0]}" type = "gp2" - size = 40 - tags { - Name = "External Volume" - } + size = 1 } -resource "aws_ebs_snapshot" "snapshot" { - volume_id = "${aws_ebs_volume.example.id}" +resource "aws_ebs_snapshot" "test" { + volume_id = "${aws_ebs_volume.test.id}" } -data "aws_ebs_snapshot" "snapshot" { - most_recent = true - snapshot_ids = ["${aws_ebs_snapshot.snapshot.id}"] +data "aws_ebs_snapshot" "test" { + snapshot_ids = ["${aws_ebs_snapshot.test.id}"] } ` -const testAccCheckAwsEbsSnapshotDataSourceConfigWithMultipleFilters = ` -resource "aws_ebs_volume" "external1" { - availability_zone = "us-west-2a" +const testAccCheckAwsEbsSnapshotDataSourceConfigFilter = ` +data "aws_availability_zones" "available" {} + +resource "aws_ebs_volume" "test" { + availability_zone = "${data.aws_availability_zones.available.names[0]}" type = "gp2" - size = 10 - tags { - Name = "External Volume 1" + size = 1 +} + +resource "aws_ebs_snapshot" "test" { + volume_id = "${aws_ebs_volume.test.id}" +} + +data "aws_ebs_snapshot" "test" { + filter { + name = "snapshot-id" + values = ["${aws_ebs_snapshot.test.id}"] } } +` -resource "aws_ebs_snapshot" "snapshot" { - volume_id = "${aws_ebs_volume.external1.id}" - tags { - Name = "TF ACC Snapshot" +const testAccCheckAwsEbsSnapshotDataSourceConfigMostRecent = ` +data "aws_availability_zones" "available" {} + +resource "aws_ebs_volume" "test" { + availability_zone = "${data.aws_availability_zones.available.names[0]}" + type = "gp2" + size = 1 +} + +resource "aws_ebs_snapshot" "incorrect" { + volume_id = "${aws_ebs_volume.test.id}" + + tags = { + Name = "tf-acc-test-ec2-ebs-snapshot-data-source-most-recent" + } +} + +resource "aws_ebs_snapshot" "test" { + volume_id = "${aws_ebs_snapshot.incorrect.volume_id}" + + tags = { + Name = "tf-acc-test-ec2-ebs-snapshot-data-source-most-recent" } } -data "aws_ebs_snapshot" "snapshot" { +data "aws_ebs_snapshot" "test" { most_recent = true - snapshot_ids = ["${aws_ebs_snapshot.snapshot.id}"] + filter { - name = "volume-size" - values = ["10"] + name = "tag:Name" + values = ["${aws_ebs_snapshot.test.tags.Name}"] } } `