From 5796b133730973d16ad2143d4aca1e081d13f19e Mon Sep 17 00:00:00 2001 From: stack72 Date: Tue, 10 Nov 2015 23:05:07 +0000 Subject: [PATCH 1/3] Adding skip_final_snapshop bool to th db_instance. This will allow us to specify whether a snapshot is needed directly rather than checking for an empty string --- .../providers/aws/resource_aws_db_instance.go | 16 +++++++++++----- .../providers/aws/r/db_instance.html.markdown | 1 + 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/builtin/providers/aws/resource_aws_db_instance.go b/builtin/providers/aws/resource_aws_db_instance.go index c078b5791..ac6205cd2 100644 --- a/builtin/providers/aws/resource_aws_db_instance.go +++ b/builtin/providers/aws/resource_aws_db_instance.go @@ -188,6 +188,12 @@ func resourceAwsDbInstance() *schema.Resource { }, }, + "skip_final_snapshot": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "copy_tags_to_snapshot": &schema.Schema{ Type: schema.TypeBool, Optional: true, @@ -619,11 +625,11 @@ func resourceAwsDbInstanceDelete(d *schema.ResourceData, meta interface{}) error opts := rds.DeleteDBInstanceInput{DBInstanceIdentifier: aws.String(d.Id())} - finalSnapshot := d.Get("final_snapshot_identifier").(string) - if finalSnapshot == "" { - opts.SkipFinalSnapshot = aws.Bool(true) - } else { - opts.FinalDBSnapshotIdentifier = aws.String(finalSnapshot) + skipFinalSnapshot := d.Get("skip_final_snapshot").(bool) + opts.SkipFinalSnapshot = aws.Bool(skipFinalSnapshot) + + if name, present := d.GetOk("final_snapshot_identifier"); present && !skipFinalSnapshot { + opts.FinalDBSnapshotIdentifier = aws.String(name.(string)) } log.Printf("[DEBUG] DB Instance destroy configuration: %v", opts) diff --git a/website/source/docs/providers/aws/r/db_instance.html.markdown b/website/source/docs/providers/aws/r/db_instance.html.markdown index 7f36f3385..8cf95de3a 100644 --- a/website/source/docs/providers/aws/r/db_instance.html.markdown +++ b/website/source/docs/providers/aws/r/db_instance.html.markdown @@ -45,6 +45,7 @@ The following arguments are supported: * `final_snapshot_identifier` - (Optional) The name of your final DB snapshot when this DB instance is deleted. If omitted, no final snapshot will be made. +* `skip_final_snapshot` - (Optional) Determines whether a final DB snapshot is created before the DB instance is deleted. If true is specified, no DBSnapshot is created. If false is specified, a DB snapshot is created before the DB instance is deleted. Default is false. * `copy_tags_to_snapshot` – (Optional, boolean) On delete, copy all Instance `tags` to the final snapshot (if `final_snapshot_identifier` is specified). Default `false` From 6082e3e732cb4a1f7405e27757a4217ed61b9446 Mon Sep 17 00:00:00 2001 From: stack72 Date: Wed, 11 Nov 2015 16:39:24 +0000 Subject: [PATCH 2/3] Changing the db_instance to throw an error is a final snapshot is required but yet no identified is given --- builtin/providers/aws/resource_aws_db_instance.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/builtin/providers/aws/resource_aws_db_instance.go b/builtin/providers/aws/resource_aws_db_instance.go index ac6205cd2..4699112bc 100644 --- a/builtin/providers/aws/resource_aws_db_instance.go +++ b/builtin/providers/aws/resource_aws_db_instance.go @@ -628,8 +628,12 @@ func resourceAwsDbInstanceDelete(d *schema.ResourceData, meta interface{}) error skipFinalSnapshot := d.Get("skip_final_snapshot").(bool) opts.SkipFinalSnapshot = aws.Bool(skipFinalSnapshot) - if name, present := d.GetOk("final_snapshot_identifier"); present && !skipFinalSnapshot { - opts.FinalDBSnapshotIdentifier = aws.String(name.(string)) + if !skipFinalSnapshot { + if name, present := d.GetOk("final_snapshot_identifier"); present { + opts.FinalDBSnapshotIdentifier = aws.String(name.(string)) + } else { + return fmt.Errorf("DB Instance FinalSnapshotIdentifier is required when a final snapshot is required") + } } log.Printf("[DEBUG] DB Instance destroy configuration: %v", opts) From 2b0c7aa4e9a039734ca4193c20594ac0c9df6955 Mon Sep 17 00:00:00 2001 From: stack72 Date: Wed, 11 Nov 2015 17:24:59 +0000 Subject: [PATCH 3/3] Making the changes to db_instance skip_final_snapshot on the feedback from @catsby --- .../providers/aws/resource_aws_db_instance.go | 2 +- .../aws/resource_aws_db_instance_test.go | 183 ++++++++++++++++++ .../providers/aws/r/db_instance.html.markdown | 2 +- 3 files changed, 185 insertions(+), 2 deletions(-) diff --git a/builtin/providers/aws/resource_aws_db_instance.go b/builtin/providers/aws/resource_aws_db_instance.go index 4699112bc..a034b7953 100644 --- a/builtin/providers/aws/resource_aws_db_instance.go +++ b/builtin/providers/aws/resource_aws_db_instance.go @@ -191,7 +191,7 @@ func resourceAwsDbInstance() *schema.Resource { "skip_final_snapshot": &schema.Schema{ Type: schema.TypeBool, Optional: true, - Default: false, + Default: true, }, "copy_tags_to_snapshot": &schema.Schema{ diff --git a/builtin/providers/aws/resource_aws_db_instance_test.go b/builtin/providers/aws/resource_aws_db_instance_test.go index a2c2f69ca..74ed455f7 100644 --- a/builtin/providers/aws/resource_aws_db_instance_test.go +++ b/builtin/providers/aws/resource_aws_db_instance_test.go @@ -12,6 +12,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/rds" + "log" ) func TestAccAWSDBInstance_basic(t *testing.T) { @@ -67,6 +68,42 @@ func TestAccAWSDBInstanceReplica(t *testing.T) { }) } +func TestAccAWSDBInstanceSnapshot(t *testing.T) { + var snap rds.DBInstance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceSnapshot, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccSnapshotInstanceConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists("aws_db_instance.snapshot", &snap), + ), + }, + }, + }) +} + +func TestAccAWSDBInstanceNoSnapshot(t *testing.T) { + var nosnap rds.DBInstance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceNoSnapshot, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccNoSnapshotInstanceConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists("aws_db_instance.no_snapshot", &nosnap), + ), + }, + }, + }) +} + func testAccCheckAWSDBInstanceDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).rdsconn @@ -132,6 +169,104 @@ func testAccCheckAWSDBInstanceReplicaAttributes(source, replica *rds.DBInstance) } } +func testAccCheckAWSDBInstanceSnapshot(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).rdsconn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_db_instance" { + continue + } + + var err error + resp, err := conn.DescribeDBInstances( + &rds.DescribeDBInstancesInput{ + DBInstanceIdentifier: aws.String(rs.Primary.ID), + }) + + if err != nil { + newerr, _ := err.(awserr.Error) + if newerr.Code() != "DBInstanceNotFound" { + return err + } + + } else { + if len(resp.DBInstances) != 0 && + *resp.DBInstances[0].DBInstanceIdentifier == rs.Primary.ID { + return fmt.Errorf("DB Instance still exists") + } + } + + log.Printf("[INFO] Trying to locate the DBInstance Final Snapshot") + snapshot_identifier := "foobarbaz-test-terraform-final-snapshot-1" + _, snapErr := conn.DescribeDBSnapshots( + &rds.DescribeDBSnapshotsInput{ + DBSnapshotIdentifier: aws.String(snapshot_identifier), + }) + + if snapErr != nil { + newerr, _ := snapErr.(awserr.Error) + if newerr.Code() == "DBSnapshotNotFound" { + return fmt.Errorf("Snapshot %s not found", snapshot_identifier) + } + } else { + log.Printf("[INFO] Deleting the Snapshot %s", snapshot_identifier) + _, snapDeleteErr := conn.DeleteDBSnapshot( + &rds.DeleteDBSnapshotInput{ + DBSnapshotIdentifier: aws.String(snapshot_identifier), + }) + if snapDeleteErr != nil { + return err + } + } + } + + return nil +} + +func testAccCheckAWSDBInstanceNoSnapshot(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).rdsconn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_db_instance" { + continue + } + + var err error + resp, err := conn.DescribeDBInstances( + &rds.DescribeDBInstancesInput{ + DBInstanceIdentifier: aws.String(rs.Primary.ID), + }) + + if err != nil { + newerr, _ := err.(awserr.Error) + if newerr.Code() != "DBInstanceNotFound" { + return err + } + + } else { + if len(resp.DBInstances) != 0 && + *resp.DBInstances[0].DBInstanceIdentifier == rs.Primary.ID { + return fmt.Errorf("DB Instance still exists") + } + } + + snapshot_identifier := "foobarbaz-test-terraform-final-snapshot-2" + _, snapErr := conn.DescribeDBSnapshots( + &rds.DescribeDBSnapshotsInput{ + DBSnapshotIdentifier: aws.String(snapshot_identifier), + }) + + if snapErr != nil { + newerr, _ := snapErr.(awserr.Error) + if newerr.Code() != "DBSnapshotNotFound" { + return fmt.Errorf("Snapshot %s found and it shouldn't have been", snapshot_identifier) + } + } + } + + return nil +} + func testAccCheckAWSDBInstanceExists(n string, v *rds.DBInstance) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -226,3 +361,51 @@ func testAccReplicaInstanceConfig(val int) string { } `, val, val) } + +var testAccSnapshotInstanceConfig = ` +provider "aws" { + region = "us-east-1" +} +resource "aws_db_instance" "snapshot" { + identifier = "foobarbaz-test-terraform-snapshot-1" + + allocated_storage = 5 + engine = "mysql" + engine_version = "5.6.21" + instance_class = "db.t1.micro" + name = "baz" + password = "barbarbarbar" + username = "foo" + security_group_names = ["default"] + backup_retention_period = 1 + + parameter_group_name = "default.mysql5.6" + + skip_final_snapshot = false + final_snapshot_identifier = "foobarbaz-test-terraform-final-snapshot-1" +} +` + +var testAccNoSnapshotInstanceConfig = ` +provider "aws" { + region = "us-east-1" +} +resource "aws_db_instance" "no_snapshot" { + identifier = "foobarbaz-test-terraform-snapshot-2" + + allocated_storage = 5 + engine = "mysql" + engine_version = "5.6.21" + instance_class = "db.t1.micro" + name = "baz" + password = "barbarbarbar" + username = "foo" + security_group_names = ["default"] + backup_retention_period = 1 + + parameter_group_name = "default.mysql5.6" + + skip_final_snapshot = true + final_snapshot_identifier = "foobarbaz-test-terraform-final-snapshot-2" +} +` diff --git a/website/source/docs/providers/aws/r/db_instance.html.markdown b/website/source/docs/providers/aws/r/db_instance.html.markdown index 8cf95de3a..2ff282d10 100644 --- a/website/source/docs/providers/aws/r/db_instance.html.markdown +++ b/website/source/docs/providers/aws/r/db_instance.html.markdown @@ -45,7 +45,7 @@ The following arguments are supported: * `final_snapshot_identifier` - (Optional) The name of your final DB snapshot when this DB instance is deleted. If omitted, no final snapshot will be made. -* `skip_final_snapshot` - (Optional) Determines whether a final DB snapshot is created before the DB instance is deleted. If true is specified, no DBSnapshot is created. If false is specified, a DB snapshot is created before the DB instance is deleted. Default is false. +* `skip_final_snapshot` - (Optional) Determines whether a final DB snapshot is created before the DB instance is deleted. If true is specified, no DBSnapshot is created. If false is specified, a DB snapshot is created before the DB instance is deleted. Default is true. * `copy_tags_to_snapshot` – (Optional, boolean) On delete, copy all Instance `tags` to the final snapshot (if `final_snapshot_identifier` is specified). Default `false`