Making the changes to db_instance skip_final_snapshot on the feedback from @catsby

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{

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"
+}
+`

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`