Skip to content
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.

Sign up for GitHub

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

resource/aws_db_instance: Prevent error when using snapshot_identifier with multi_az enabled and sqlserver engine #5613

Merged
merged 1 commit into from
Aug 20, 2018
Merged

resource/aws_db_instance: Prevent error when using snapshot_identifier with multi_az enabled and sqlserver engine #5613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 58 additions & 24 deletions aws/resource_aws_db_instance.go
View file
Open in desktop
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
Open in desktop
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