-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add check destroy for aws ebs snapshot copy resource & Fix error handling #9106
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -116,7 +116,7 @@ func resourceAwsEbsSnapshotCopyRead(d *schema.ResourceData, meta interface{}) er | |||||
SnapshotIds: []*string{aws.String(d.Id())}, | ||||||
} | ||||||
res, err := conn.DescribeSnapshots(req) | ||||||
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidSnapshotID.NotFound" { | ||||||
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidSnapshot.NotFound" { | ||||||
log.Printf("Snapshot %q Not found - removing from state", d.Id()) | ||||||
d.SetId("") | ||||||
return nil | ||||||
|
@@ -153,10 +153,14 @@ func resourceAwsEbsSnapshotCopyDelete(d *schema.ResourceData, meta interface{}) | |||||
} | ||||||
|
||||||
ebsErr, ok := err.(awserr.Error) | ||||||
if ebsErr.Code() == "SnapshotInUse" { | ||||||
if ok && ebsErr.Code() == "SnapshotInUse" { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving as non-blocking feedback for next time.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||||||
return resource.RetryableError(fmt.Errorf("EBS SnapshotInUse - trying again while it detaches")) | ||||||
} | ||||||
|
||||||
if ok && ebsErr.Code() == "InvalidSnapshot.NotFound" { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||||||
return nil | ||||||
} | ||||||
|
||||||
if !ok { | ||||||
return resource.NonRetryableError(err) | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,6 +6,7 @@ import ( | |||||
"testing" | ||||||
|
||||||
"github.com/aws/aws-sdk-go/aws" | ||||||
"github.com/aws/aws-sdk-go/aws/awserr" | ||||||
"github.com/aws/aws-sdk-go/service/ec2" | ||||||
"github.com/hashicorp/terraform/helper/resource" | ||||||
"github.com/hashicorp/terraform/helper/schema" | ||||||
|
@@ -15,8 +16,9 @@ import ( | |||||
func TestAccAWSEbsSnapshotCopy_basic(t *testing.T) { | ||||||
var v ec2.Snapshot | ||||||
resource.ParallelTest(t, resource.TestCase{ | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
Providers: testAccProviders, | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
Providers: testAccProviders, | ||||||
CheckDestroy: testAccCheckEbsSnapshotCopyDestroy, | ||||||
Steps: []resource.TestStep{ | ||||||
{ | ||||||
Config: testAccAwsEbsSnapshotCopyConfig, | ||||||
|
@@ -32,8 +34,9 @@ func TestAccAWSEbsSnapshotCopy_basic(t *testing.T) { | |||||
func TestAccAWSEbsSnapshotCopy_withDescription(t *testing.T) { | ||||||
var v ec2.Snapshot | ||||||
resource.ParallelTest(t, resource.TestCase{ | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
Providers: testAccProviders, | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
Providers: testAccProviders, | ||||||
CheckDestroy: testAccCheckEbsSnapshotCopyDestroy, | ||||||
Steps: []resource.TestStep{ | ||||||
{ | ||||||
Config: testAccAwsEbsSnapshotCopyConfigWithDescription, | ||||||
|
@@ -63,6 +66,7 @@ func TestAccAWSEbsSnapshotCopy_withRegions(t *testing.T) { | |||||
resource.ParallelTest(t, resource.TestCase{ | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
ProviderFactories: providerFactories, | ||||||
CheckDestroy: testAccCheckEbsSnapshotCopyDestroy, | ||||||
Steps: []resource.TestStep{ | ||||||
{ | ||||||
Config: testAccAwsEbsSnapshotCopyConfigWithRegions, | ||||||
|
@@ -78,8 +82,9 @@ func TestAccAWSEbsSnapshotCopy_withRegions(t *testing.T) { | |||||
func TestAccAWSEbsSnapshotCopy_withKms(t *testing.T) { | ||||||
var v ec2.Snapshot | ||||||
resource.ParallelTest(t, resource.TestCase{ | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
Providers: testAccProviders, | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
Providers: testAccProviders, | ||||||
CheckDestroy: testAccCheckEbsSnapshotCopyDestroy, | ||||||
Steps: []resource.TestStep{ | ||||||
{ | ||||||
Config: testAccAwsEbsSnapshotCopyConfigWithKms, | ||||||
|
@@ -93,6 +98,67 @@ func TestAccAWSEbsSnapshotCopy_withKms(t *testing.T) { | |||||
}) | ||||||
} | ||||||
|
||||||
func TestAccAWSEbsSnapshotCopy_disappears(t *testing.T) { | ||||||
var v ec2.Snapshot | ||||||
resource.ParallelTest(t, resource.TestCase{ | ||||||
PreCheck: func() { testAccPreCheck(t) }, | ||||||
Providers: testAccProviders, | ||||||
CheckDestroy: testAccCheckEbsSnapshotCopyDestroy, | ||||||
Steps: []resource.TestStep{ | ||||||
{ | ||||||
Config: testAccAwsEbsSnapshotCopyConfig, | ||||||
Check: resource.ComposeTestCheckFunc( | ||||||
testAccCheckEbsSnapshotCopyExists("aws_ebs_snapshot_copy.test", &v), | ||||||
testAccCheckEbsSnapshotCopyDisappears(&v), | ||||||
), | ||||||
ExpectNonEmptyPlan: true, | ||||||
}, | ||||||
}, | ||||||
}) | ||||||
} | ||||||
|
||||||
func testAccCheckEbsSnapshotCopyDestroy(s *terraform.State) error { | ||||||
conn := testAccProvider.Meta().(*AWSClient).ec2conn | ||||||
|
||||||
for _, rs := range s.RootModule().Resources { | ||||||
if rs.Type != "aws_ebs_snapshot_copy" { | ||||||
continue | ||||||
} | ||||||
|
||||||
resp, err := conn.DescribeSnapshots(&ec2.DescribeSnapshotsInput{ | ||||||
SnapshotIds: []*string{aws.String(rs.Primary.ID)}, | ||||||
}) | ||||||
|
||||||
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidSnapshot.NotFound" { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||||||
return nil | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||||||
} | ||||||
|
||||||
if err == nil { | ||||||
for _, snapshot := range resp.Snapshots { | ||||||
if aws.StringValue(snapshot.SnapshotId) == rs.Primary.ID { | ||||||
return fmt.Errorf("EBS Snapshot still exists") | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
return err | ||||||
} | ||||||
|
||||||
return nil | ||||||
} | ||||||
|
||||||
func testAccCheckEbsSnapshotCopyDisappears(snapshot *ec2.Snapshot) resource.TestCheckFunc { | ||||||
return func(s *terraform.State) error { | ||||||
conn := testAccProvider.Meta().(*AWSClient).ec2conn | ||||||
|
||||||
_, err := conn.DeleteSnapshot(&ec2.DeleteSnapshotInput{ | ||||||
SnapshotId: snapshot.SnapshotId, | ||||||
}) | ||||||
|
||||||
return err | ||||||
} | ||||||
} | ||||||
|
||||||
func testAccCheckEbsSnapshotCopyExists(n string, v *ec2.Snapshot) resource.TestCheckFunc { | ||||||
providers := []*schema.Provider{testAccProvider} | ||||||
return testAccCheckEbsSnapshotCopyExistsWithProviders(n, v, &providers) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can be simplified using
isAWSErr(err, CODE, MESSAGE)
in the future, e.g.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.