Skip to content

Commit

Permalink
Merge pull request #5613 from terraform-providers/b-aws_db_instance-m…
Browse files Browse the repository at this point in the history 
…ultiaz-sqlserver

resource/aws_db_instance: Prevent error when using snapshot_identifier with multi_az enabled and sqlserver engine
  • Loading branch information
@bflad
bflad authored Aug 20, 2018
2 parents 05faf8d + f60e13e commit 3624386
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 24 deletions.
82 changes: 58 additions & 24 deletions aws/resource_aws_db_instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,12 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error

return resourceAwsDbInstanceRead(d, meta)
} else if _, ok := d.GetOk("snapshot_identifier"); ok {
// RestoreDBInstanceFromDBSnapshot does not support all parameters
// correctly apply in one pass. For missing parameters or unsupported
// configurations, we need to call ModifyDBInstance afterwards to
// prevent Terraform operators from API errors or double apply.
var requiresModifyDbInstance bool

opts := rds.RestoreDBInstanceFromDBSnapshotInput{
DBInstanceClass: aws.String(d.Get("instance_class").(string)),
DBInstanceIdentifier: aws.String(d.Get("identifier").(string)),
Expand Down Expand Up @@ -711,54 +717,74 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error
}

if attr, ok := d.GetOk("multi_az"); ok {
opts.MultiAZ = aws.Bool(attr.(bool))
// When using SQL Server engine with MultiAZ enabled, its not
// possible to immediately enable mirroring since
// BackupRetentionPeriod is not available as a parameter to
// RestoreDBInstanceFromDBSnapshot and you receive an error. e.g.
// InvalidParameterValue: Mirroring cannot be applied to instances with backup retention set to zero.
// If we know the engine, prevent the error upfront.
if v, ok := d.GetOk("engine"); ok && strings.HasPrefix(strings.ToLower(v.(string)), "sqlserver") {
requiresModifyDbInstance = true
} else {
opts.MultiAZ = aws.Bool(attr.(bool))
}
}

if attr, ok := d.GetOk("option_group_name"); ok {
opts.OptionGroupName = aws.String(attr.(string))

}

if _, ok := d.GetOk("password"); ok {
requiresModifyDbInstance = true
}

if attr, ok := d.GetOk("port"); ok {
opts.Port = aws.Int64(int64(attr.(int)))
}
if attr, ok := d.GetOk("tde_credential_arn"); ok {
opts.TdeCredentialArn = aws.String(attr.(string))

if attr := d.Get("security_group_names").(*schema.Set); attr.Len() > 0 {
requiresModifyDbInstance = true
}

if attr, ok := d.GetOk("storage_type"); ok {
opts.StorageType = aws.String(attr.(string))
}

log.Printf("[DEBUG] DB Instance restore from snapshot configuration: %s", opts)
_, err := conn.RestoreDBInstanceFromDBSnapshot(&opts)
if err != nil {
return fmt.Errorf("Error creating DB Instance: %s", err)
if attr, ok := d.GetOk("tde_credential_arn"); ok {
opts.TdeCredentialArn = aws.String(attr.(string))
}

var sgUpdate bool
var passwordUpdate bool

if _, ok := d.GetOk("password"); ok {
passwordUpdate = true
if attr := d.Get("vpc_security_group_ids").(*schema.Set); attr.Len() > 0 {
requiresModifyDbInstance = true
}

if attr := d.Get("vpc_security_group_ids").(*schema.Set); attr.Len() > 0 {
sgUpdate = true
log.Printf("[DEBUG] DB Instance restore from snapshot configuration: %s", opts)
_, err := conn.RestoreDBInstanceFromDBSnapshot(&opts)

// When using SQL Server engine with MultiAZ enabled, its not
// possible to immediately enable mirroring since
// BackupRetentionPeriod is not available as a parameter to
// RestoreDBInstanceFromDBSnapshot and you receive an error. e.g.
// InvalidParameterValue: Mirroring cannot be applied to instances with backup retention set to zero.
// Since engine is not a required argument when using snapshot_identifier
// and the RDS API determines this condition, we catch the error
// and remove the invalid configuration for it to be fixed afterwards.
if isAWSErr(err, "InvalidParameterValue", "Mirroring cannot be applied to instances with backup retention set to zero") {
opts.MultiAZ = aws.Bool(false)
requiresModifyDbInstance = true
_, err = conn.RestoreDBInstanceFromDBSnapshot(&opts)
}
if attr := d.Get("security_group_names").(*schema.Set); attr.Len() > 0 {
sgUpdate = true

if err != nil {
return fmt.Errorf("Error creating DB Instance: %s", err)
}
if sgUpdate || passwordUpdate {
log.Printf("[INFO] DB is restoring from snapshot with default security, but custom security should be set, will now update after snapshot is restored!")

// wait for instance to get up and then modify security
if requiresModifyDbInstance {
d.SetId(d.Get("identifier").(string))

log.Printf("[INFO] DB Instance ID: %s", d.Id())

log.Println(
"[INFO] Waiting for DB Instance to be available")
log.Printf("[INFO] DB Instance %q configuration requires ModifyDBInstance after RestoreDBInstanceFromDBSnapshot", d.Id())
log.Printf("[INFO] Waiting for DB Instance %q to be available", d.Id())

stateConf := &resource.StateChangeConf{
Pending: resourceAwsDbInstanceCreatePendingStates,
Expand Down Expand Up @@ -1123,9 +1149,17 @@ func resourceAwsDbInstanceUpdate(d *schema.ResourceData, meta interface{}) error
ApplyImmediately: aws.Bool(d.Get("apply_immediately").(bool)),
DBInstanceIdentifier: aws.String(d.Id()),
}

// ModifyDBInstance might be called during resource creation
// to fix unsupported configurations, e.g. missing parameters
// from RestoreDBInstanceFromDBSnapshot. In this case, we should
// always apply immediately.
if d.IsNewResource() {
req.ApplyImmediately = aws.Bool(true)
}
d.SetPartial("apply_immediately")

if !d.Get("apply_immediately").(bool) {
if !aws.BoolValue(req.ApplyImmediately) {
log.Println("[INFO] Only settings updating, instance changes will be applied in next maintenance window")
}

Expand Down
111 changes: 111 additions & 0 deletions aws/resource_aws_db_instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,60 @@ func TestAccAWSDBInstance_SnapshotIdentifier(t *testing.T) {
})
}

func TestAccAWSDBInstance_SnapshotIdentifier_MultiAZ(t *testing.T) {
var dbInstance, sourceDbInstance rds.DBInstance
var dbSnapshot rds.DBSnapshot

rName := acctest.RandomWithPrefix("tf-acc-test")
sourceDbResourceName := "aws_db_instance.source"
snapshotResourceName := "aws_db_snapshot.test"
resourceName := "aws_db_instance.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSDBInstanceConfig_SnapshotIdentifier_MultiAZ(rName, true),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(sourceDbResourceName, &sourceDbInstance),
testAccCheckDbSnapshotExists(snapshotResourceName, &dbSnapshot),
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttr(resourceName, "multi_az", "true"),
),
},
},
})
}

func TestAccAWSDBInstance_SnapshotIdentifier_MultiAZ_SQLServer(t *testing.T) {
var dbInstance, sourceDbInstance rds.DBInstance
var dbSnapshot rds.DBSnapshot

rName := acctest.RandomWithPrefix("tf-acc-test")
sourceDbResourceName := "aws_db_instance.source"
snapshotResourceName := "aws_db_snapshot.test"
resourceName := "aws_db_instance.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDBInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSDBInstanceConfig_SnapshotIdentifier_MultiAZ_SQLServer(rName, true),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists(sourceDbResourceName, &sourceDbInstance),
testAccCheckDbSnapshotExists(snapshotResourceName, &dbSnapshot),
testAccCheckAWSDBInstanceExists(resourceName, &dbInstance),
resource.TestCheckResourceAttr(resourceName, "multi_az", "true"),
),
},
},
})
}

func TestAccAWSDBInstance_SnapshotIdentifier_Tags(t *testing.T) {
var dbInstance, sourceDbInstance rds.DBInstance
var dbSnapshot rds.DBSnapshot
Expand Down Expand Up @@ -2101,6 +2155,63 @@ resource "aws_db_instance" "test" {
`, rName, rName, rName)
}

func testAccAWSDBInstanceConfig_SnapshotIdentifier_MultiAZ(rName string, multiAz bool) string {
return fmt.Sprintf(`
resource "aws_db_instance" "source" {
allocated_storage = 5
engine = "mariadb"
identifier = "%s-source"
instance_class = "db.t2.micro"
password = "avoid-plaintext-passwords"
username = "tfacctest"
skip_final_snapshot = true
}
resource "aws_db_snapshot" "test" {
db_instance_identifier = "${aws_db_instance.source.id}"
db_snapshot_identifier = %q
}
resource "aws_db_instance" "test" {
identifier = %q
instance_class = "${aws_db_instance.source.instance_class}"
multi_az = %t
snapshot_identifier = "${aws_db_snapshot.test.id}"
skip_final_snapshot = true
}
`, rName, rName, rName, multiAz)
}

func testAccAWSDBInstanceConfig_SnapshotIdentifier_MultiAZ_SQLServer(rName string, multiAz bool) string {
return fmt.Sprintf(`
resource "aws_db_instance" "source" {
allocated_storage = 20
engine = "sqlserver-se"
identifier = "%s-source"
instance_class = "db.m4.large"
license_model = "license-included"
password = "avoid-plaintext-passwords"
username = "tfacctest"
skip_final_snapshot = true
}
resource "aws_db_snapshot" "test" {
db_instance_identifier = "${aws_db_instance.source.id}"
db_snapshot_identifier = %q
}
resource "aws_db_instance" "test" {
# InvalidParameterValue: Mirroring cannot be applied to instances with backup retention set to zero.
backup_retention_period = 1
identifier = %q
instance_class = "${aws_db_instance.source.instance_class}"
multi_az = %t
snapshot_identifier = "${aws_db_snapshot.test.id}"
skip_final_snapshot = true
}
`, rName, rName, rName, multiAz)
}

func testAccAWSDBInstanceConfig_SnapshotIdentifier_Tags(rName string) string {
return fmt.Sprintf(`
resource "aws_db_instance" "source" {
Expand Down

0 comments on commit 3624386

Please sign in to comment.