From d9cac8a60b031d305dbee223280ca84f557a88ba Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 13 Aug 2019 00:12:26 -0400 Subject: [PATCH] resource/aws_db_instance: Ensure monitoring attributes are always written to state and retry ModifyDBInstance on IAM eventual consistency error References: - https://github.com/terraform-providers/terraform-provider-aws/issues/315 - https://github.com/terraform-providers/terraform-provider-aws/issues/2188 - https://github.com/terraform-providers/terraform-provider-aws/issues/5559 Previously before code updates: ``` --- FAIL: TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled (430.95s) testing.go:568: Step 2 error: errors during apply: Error: Error modifying DB Instance tf-acc-test-1165998526456666486: InvalidParameterValue: IAM role ARN value is invalid or does not include the required permissions for: ENHANCED_MONITORING status code: 400, request id: 524f599d-3870-48b3-843e-28885ae3f75c on /var/folders/v0/_d108fkx1pbbg4_sh864_7740000gn/T/tf-test908490254/main.tf line 29: (source code not available) --- PASS: TestAccAWSDBInstance_MonitoringRoleInterval (565.47s) --- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled (593.87s) --- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved (626.12s) ``` Output from acceptance testing: ``` --- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled (587.93s) --- PASS: TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled (614.89s) --- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved (656.13s) --- PASS: TestAccAWSDBInstance_MonitoringRoleInterval (702.57s) ``` --- aws/resource_aws_db_instance.go | 30 ++- aws/resource_aws_db_instance_test.go | 281 +++++++++++++++++++++------ 2 files changed, 240 insertions(+), 71 deletions(-) diff --git a/aws/resource_aws_db_instance.go b/aws/resource_aws_db_instance.go index f331cb0e6fa..f9c8f4358a0 100644 --- a/aws/resource_aws_db_instance.go +++ b/aws/resource_aws_db_instance.go @@ -1320,13 +1320,8 @@ func resourceAwsDbInstanceRead(d *schema.ResourceData, meta interface{}) error { d.Set("option_group_name", v.OptionGroupMemberships[0].OptionGroupName) } - if v.MonitoringInterval != nil { - d.Set("monitoring_interval", v.MonitoringInterval) - } - - if v.MonitoringRoleArn != nil { - d.Set("monitoring_role_arn", v.MonitoringRoleArn) - } + d.Set("monitoring_interval", v.MonitoringInterval) + d.Set("monitoring_role_arn", v.MonitoringRoleArn) if err := d.Set("enabled_cloudwatch_logs_exports", flattenStringList(v.EnabledCloudwatchLogsExports)); err != nil { return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err) @@ -1641,7 +1636,26 @@ func resourceAwsDbInstanceUpdate(d *schema.ResourceData, meta interface{}) error log.Printf("[DEBUG] Send DB Instance Modification request: %t", requestUpdate) if requestUpdate { log.Printf("[DEBUG] DB Instance Modification request: %s", req) - _, err := conn.ModifyDBInstance(req) + + err := resource.Retry(2*time.Minute, func() *resource.RetryError { + _, err := conn.ModifyDBInstance(req) + + // Retry for IAM eventual consistency + if isAWSErr(err, "InvalidParameterValue", "IAM role ARN value is invalid or does not include the required permissions") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if isResourceTimeoutError(err) { + _, err = conn.ModifyDBInstance(req) + } + if err != nil { return fmt.Errorf("Error modifying DB Instance %s: %s", d.Id(), err) } diff --git a/aws/resource_aws_db_instance_test.go b/aws/resource_aws_db_instance_test.go index 99fd458e637..75a3edd11d2 100644 --- a/aws/resource_aws_db_instance_test.go +++ b/aws/resource_aws_db_instance_test.go @@ -1555,9 +1555,10 @@ func TestAccAWSDBInstance_SnapshotIdentifier_VpcSecurityGroupIds_Tags(t *testing }) } -func TestAccAWSDBInstance_enhancedMonitoring(t *testing.T) { +func TestAccAWSDBInstance_MonitoringRoleInterval(t *testing.T) { var dbInstance rds.DBInstance - rName := acctest.RandString(5) + resourceName := "aws_db_instance.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -1565,11 +1566,146 @@ func TestAccAWSDBInstance_enhancedMonitoring(t *testing.T) { CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccSnapshotInstanceConfig_enhancedMonitoring(rName), + Config: testAccDbInstanceConfigMonitoringInterval(rName, 30), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSDBInstanceExists("aws_db_instance.enhanced_monitoring", &dbInstance), - resource.TestCheckResourceAttr( - "aws_db_instance.enhanced_monitoring", "monitoring_interval", "5"), + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "monitoring_interval", "30"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "apply_immediately", + "final_snapshot_identifier", + "password", + "skip_final_snapshot", + }, + }, + { + Config: testAccDbInstanceConfigMonitoringInterval(rName, 60), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "monitoring_interval", "60"), + ), + }, + }, + }) +} + +func TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled(t *testing.T) { + var dbInstance rds.DBInstance + iamRoleResourceName := "aws_iam_role.test" + resourceName := "aws_db_instance.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDbInstanceConfigMonitoringRoleArn(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttrPair(resourceName, "monitoring_role_arn", iamRoleResourceName, "arn"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "apply_immediately", + "final_snapshot_identifier", + "password", + "skip_final_snapshot", + }, + }, + { + Config: testAccDbInstanceConfigMonitoringInterval(rName, 0), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttr(resourceName, "monitoring_interval", "0"), + ), + }, + }, + }) +} + +func TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved(t *testing.T) { + var dbInstance rds.DBInstance + iamRoleResourceName := "aws_iam_role.test" + resourceName := "aws_db_instance.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDbInstanceConfigMonitoringRoleArn(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttrPair(resourceName, "monitoring_role_arn", iamRoleResourceName, "arn"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "apply_immediately", + "final_snapshot_identifier", + "password", + "skip_final_snapshot", + }, + }, + { + Config: testAccDbInstanceConfigMonitoringRoleArnRemoved(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + ), + }, + }, + }) +} + +func TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled(t *testing.T) { + var dbInstance rds.DBInstance + iamRoleResourceName := "aws_iam_role.test" + resourceName := "aws_db_instance.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDbInstanceConfigMonitoringRoleArnRemoved(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "apply_immediately", + "final_snapshot_identifier", + "password", + "skip_final_snapshot", + }, + }, + { + Config: testAccDbInstanceConfigMonitoringRoleArn(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists(resourceName, &dbInstance), + resource.TestCheckResourceAttrPair(resourceName, "monitoring_role_arn", iamRoleResourceName, "arn"), ), }, }, @@ -2690,10 +2826,12 @@ resource "aws_db_instance" "snapshot" { `, rInt, rInt) } -func testAccSnapshotInstanceConfig_enhancedMonitoring(rName string) string { +func testAccDbInstanceConfigMonitoringInterval(rName string, monitoringInterval int) string { return fmt.Sprintf(` -resource "aws_iam_role" "enhanced_policy_role" { - name = "enhanced-monitoring-role-%s" +data "aws_partition" "current" {} + +resource "aws_iam_role" "test" { + name = %[1]q assume_role_policy = <