From ced96aa90bafa8b997958197c4a9ddc220caef36 Mon Sep 17 00:00:00 2001 From: Paul Stack Date: Fri, 3 Feb 2017 17:21:25 +0000 Subject: [PATCH] provider/aws: Set aws_db_cluster to snapshot by default (#11668) As requested in #11448 Terraform didn't snapshot AWS DB Instances by default. We are going to change that behaviour going forward. --- .../providers/aws/resource_aws_db_instance.go | 2 +- .../aws/resource_aws_db_instance_test.go | 65 +++++++++++-------- .../providers/aws/r/db_instance.html.markdown | 2 +- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/builtin/providers/aws/resource_aws_db_instance.go b/builtin/providers/aws/resource_aws_db_instance.go index b1635f540..fca7a8227 100644 --- a/builtin/providers/aws/resource_aws_db_instance.go +++ b/builtin/providers/aws/resource_aws_db_instance.go @@ -207,7 +207,7 @@ func resourceAwsDbInstance() *schema.Resource { "skip_final_snapshot": { Type: schema.TypeBool, Optional: true, - Default: true, + Default: false, }, "copy_tags_to_snapshot": { diff --git a/builtin/providers/aws/resource_aws_db_instance_test.go b/builtin/providers/aws/resource_aws_db_instance_test.go index cf23a847d..51e8eec5b 100644 --- a/builtin/providers/aws/resource_aws_db_instance_test.go +++ b/builtin/providers/aws/resource_aws_db_instance_test.go @@ -150,15 +150,13 @@ func TestAccAWSDBInstanceReplica(t *testing.T) { }) } -func TestAccAWSDBInstanceSnapshot(t *testing.T) { +func TestAccAWSDBInstanceNoSnapshot(t *testing.T) { var snap rds.DBInstance resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - // testAccCheckAWSDBInstanceSnapshot verifies a database snapshot is - // created, and subequently deletes it - CheckDestroy: testAccCheckAWSDBInstanceSnapshot, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceNoSnapshot, Steps: []resource.TestStep{ { Config: testAccSnapshotInstanceConfig(), @@ -170,18 +168,20 @@ func TestAccAWSDBInstanceSnapshot(t *testing.T) { }) } -func TestAccAWSDBInstanceNoSnapshot(t *testing.T) { - var nosnap rds.DBInstance +func TestAccAWSDBInstanceSnapshot(t *testing.T) { + var snap rds.DBInstance resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSDBInstanceNoSnapshot, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + // testAccCheckAWSDBInstanceSnapshot verifies a database snapshot is + // created, and subequently deletes it + CheckDestroy: testAccCheckAWSDBInstanceSnapshot, Steps: []resource.TestStep{ { - Config: testAccNoSnapshotInstanceConfig(), + Config: testAccSnapshotInstanceConfigWithSnapshot(), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSDBInstanceExists("aws_db_instance.no_snapshot", &nosnap), + testAccCheckAWSDBInstanceExists("aws_db_instance.snapshot", &snap), ), }, }, @@ -442,7 +442,7 @@ func testAccCheckAWSDBInstanceSnapshot(s *terraform.State) error { } log.Printf("[INFO] Trying to locate the DBInstance Final Snapshot") - snapshot_identifier := "foobarbaz-test-terraform-final-snapshot-1" + snapshot_identifier := "foobarbaz-test-terraform-final-snapshot-2" _, snapErr := conn.DescribeDBSnapshots( &rds.DescribeDBSnapshotsInput{ DBSnapshotIdentifier: aws.String(snapshot_identifier), @@ -593,6 +593,7 @@ resource "aws_db_instance" "bar" { # documented. Terraform will downcase this to match (as opposed to throw a # validation error). maintenance_window = "Fri:09:00-Fri:09:30" + skip_final_snapshot = true backup_retention_period = 0 @@ -640,6 +641,8 @@ resource "aws_db_instance" "bar" { storage_encrypted = true kms_key_id = "${aws_kms_key.foo.arn}" + skip_final_snapshot = true + parameter_group_name = "default.mysql5.6" } ` @@ -664,6 +667,7 @@ resource "aws_db_instance" "bar" { username = "foo" backup_retention_period = 0 + skip_final_snapshot = true parameter_group_name = "default.mysql5.6" option_group_name = "${aws_db_option_group.bar.name}" @@ -684,6 +688,7 @@ func testAccReplicaInstanceConfig(val int) string { username = "foo" backup_retention_period = 1 + skip_final_snapshot = true parameter_group_name = "default.mysql5.6" } @@ -698,6 +703,7 @@ func testAccReplicaInstanceConfig(val int) string { instance_class = "${aws_db_instance.bar.instance_class}" password = "${aws_db_instance.bar.password}" username = "${aws_db_instance.bar.username}" + skip_final_snapshot = true tags { Name = "tf-replica-db" } @@ -711,7 +717,7 @@ provider "aws" { region = "us-east-1" } resource "aws_db_instance" "snapshot" { - identifier = "tf-snapshot-%d" + identifier = "tf-test-%d" allocated_storage = 5 engine = "mysql" @@ -727,22 +733,18 @@ resource "aws_db_instance" "snapshot" { parameter_group_name = "default.mysql5.6" - skip_final_snapshot = false - copy_tags_to_snapshot = true + skip_final_snapshot = true final_snapshot_identifier = "foobarbaz-test-terraform-final-snapshot-1" - tags { - Name = "tf-tags-db" - } }`, acctest.RandInt()) } -func testAccNoSnapshotInstanceConfig() string { +func testAccSnapshotInstanceConfigWithSnapshot() string { return fmt.Sprintf(` provider "aws" { region = "us-east-1" } -resource "aws_db_instance" "no_snapshot" { - identifier = "tf-test-%s" +resource "aws_db_instance" "snapshot" { + identifier = "tf-snapshot-%d" allocated_storage = 5 engine = "mysql" @@ -752,15 +754,18 @@ resource "aws_db_instance" "no_snapshot" { password = "barbarbarbar" publicly_accessible = true username = "foo" - security_group_names = ["default"] + security_group_names = ["default"] backup_retention_period = 1 parameter_group_name = "default.mysql5.6" - skip_final_snapshot = true + copy_tags_to_snapshot = true final_snapshot_identifier = "foobarbaz-test-terraform-final-snapshot-2" + tags { + Name = "tf-tags-db" + } } -`, acctest.RandString(5)) +`, acctest.RandInt()) } func testAccSnapshotInstanceConfig_enhancedMonitoring(rName string) string { @@ -827,6 +832,7 @@ resource "aws_db_instance" "bar" { username = "foo" password = "barbarbar" parameter_group_name = "default.mysql5.6" + skip_final_snapshot = true apply_immediately = true @@ -849,6 +855,7 @@ resource "aws_db_instance" "bar" { parameter_group_name = "default.mysql5.6" port = 3306 allocated_storage = 10 + skip_final_snapshot = true apply_immediately = true }`, rName) @@ -867,6 +874,7 @@ resource "aws_db_instance" "bar" { parameter_group_name = "default.mysql5.6" port = 3305 allocated_storage = 10 + skip_final_snapshot = true apply_immediately = true }`, rName) @@ -916,6 +924,7 @@ resource "aws_db_instance" "bar" { db_subnet_group_name = "${aws_db_subnet_group.foo.name}" port = 3305 allocated_storage = 10 + skip_final_snapshot = true backup_retention_period = 0 apply_immediately = true @@ -996,6 +1005,7 @@ resource "aws_db_instance" "bar" { db_subnet_group_name = "${aws_db_subnet_group.bar.name}" port = 3305 allocated_storage = 10 + skip_final_snapshot = true backup_retention_period = 0 @@ -1046,6 +1056,7 @@ resource "aws_db_instance" "mssql" { password = "somecrazypassword" engine = "sqlserver-ex" backup_retention_period = 0 + skip_final_snapshot = true #publicly_accessible = true @@ -1113,6 +1124,7 @@ resource "aws_db_instance" "mssql" { password = "somecrazypassword" engine = "sqlserver-ex" backup_retention_period = 0 + skip_final_snapshot = true #publicly_accessible = true @@ -1148,5 +1160,6 @@ resource "aws_db_instance" "bar" { name = "baz" password = "barbarbarbar" username = "foo" + skip_final_snapshot = true } `, acctest.RandInt()) 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 97eabe5c5..fdadeb5b9 100644 --- a/website/source/docs/providers/aws/r/db_instance.html.markdown +++ b/website/source/docs/providers/aws/r/db_instance.html.markdown @@ -60,7 +60,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, using the value from `final_snapshot_identifier`. Default is true. +* `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, using the value from `final_snapshot_identifier`. 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`