From 1514e2ddd9e55eaf4e57a9658432836a0940b84b Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 21 Mar 2023 17:08:31 -0400 Subject: [PATCH 1/4] r/aws_rds: handle pending engine_version updates --- internal/service/rds/cluster.go | 6 ++++- internal/service/rds/cluster_instance.go | 6 ++++- internal/service/rds/engine_version_test.go | 23 +++++++++++++++++- internal/service/rds/instance.go | 6 ++++- internal/service/rds/verify.go | 26 ++++++++++++++++----- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/internal/service/rds/cluster.go b/internal/service/rds/cluster.go index 6897ec9f744f..3ead3aa17873 100644 --- a/internal/service/rds/cluster.go +++ b/internal/service/rds/cluster.go @@ -1464,7 +1464,11 @@ func removeIAMRoleFromCluster(ctx context.Context, conn *rds.RDS, clusterID, rol func clusterSetResourceDataEngineVersionFromCluster(d *schema.ResourceData, c *rds.DBCluster) { oldVersion := d.Get("engine_version").(string) newVersion := aws.StringValue(c.EngineVersion) - compareActualEngineVersion(d, oldVersion, newVersion) + var pendingVersion string + if c.PendingModifiedValues != nil && c.PendingModifiedValues.EngineVersion != nil { + pendingVersion = aws.StringValue(c.PendingModifiedValues.EngineVersion) + } + compareActualEngineVersion(d, oldVersion, newVersion, pendingVersion) } func FindDBClusterByID(ctx context.Context, conn *rds.RDS, id string) (*rds.DBCluster, error) { diff --git a/internal/service/rds/cluster_instance.go b/internal/service/rds/cluster_instance.go index 420f709a3638..c9dbee5ff8c0 100644 --- a/internal/service/rds/cluster_instance.go +++ b/internal/service/rds/cluster_instance.go @@ -563,5 +563,9 @@ func resourceClusterInstanceDelete(ctx context.Context, d *schema.ResourceData, func clusterSetResourceDataEngineVersionFromClusterInstance(d *schema.ResourceData, c *rds.DBInstance) { oldVersion := d.Get("engine_version").(string) newVersion := aws.StringValue(c.EngineVersion) - compareActualEngineVersion(d, oldVersion, newVersion) + var pendingVersion string + if c.PendingModifiedValues != nil && c.PendingModifiedValues.EngineVersion != nil { + pendingVersion = aws.StringValue(c.PendingModifiedValues.EngineVersion) + } + compareActualEngineVersion(d, oldVersion, newVersion, pendingVersion) } diff --git a/internal/service/rds/engine_version_test.go b/internal/service/rds/engine_version_test.go index 492fa68e88d2..beed7faa38b2 100644 --- a/internal/service/rds/engine_version_test.go +++ b/internal/service/rds/engine_version_test.go @@ -10,10 +10,17 @@ func TestCompareActualEngineVersion(t *testing.T) { type testCase struct { configuredVersion string actualVersion string + pendingVersion string expectedEngineVersion string expectedEngineVersionActual string } tests := map[string]testCase{ + "import": { + configuredVersion: "", // no "old" value on import + actualVersion: "8.1", + expectedEngineVersion: "8.1", + expectedEngineVersionActual: "8.1", + }, "point version upgrade": { configuredVersion: "8.0", actualVersion: "8.0.27", @@ -32,6 +39,20 @@ func TestCompareActualEngineVersion(t *testing.T) { expectedEngineVersion: "9.0.0", expectedEngineVersionActual: "9.0.0", }, + "pending minor version upgrade": { + configuredVersion: "8.1.1", + actualVersion: "8.0", + pendingVersion: "8.1.1", + expectedEngineVersion: "8.1.1", + expectedEngineVersionActual: "8.0", + }, + "pending major version upgrade": { + configuredVersion: "9.0.0", + actualVersion: "8.1", + pendingVersion: "9.0.0", + expectedEngineVersion: "9.0.0", + expectedEngineVersionActual: "8.1", + }, "aurora upgrade": { configuredVersion: "5.7.mysql_aurora.2.07", actualVersion: "5.7.serverless_mysql_aurora.2.08.3", @@ -66,7 +87,7 @@ func TestCompareActualEngineVersion(t *testing.T) { r := ResourceCluster() d := r.Data(nil) d.Set("engine_version", test.configuredVersion) - compareActualEngineVersion(d, test.configuredVersion, test.actualVersion) + compareActualEngineVersion(d, test.configuredVersion, test.actualVersion, test.pendingVersion) if want, got := test.expectedEngineVersion, d.Get("engine_version"); got != want { t.Errorf("unexpected engine_version; want: %q, got: %q", want, got) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index c62ece13b483..dca07cccc611 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -2247,7 +2247,11 @@ func isStorageTypeGP3BelowAllocatedStorageThreshold(d *schema.ResourceData) bool func dbSetResourceDataEngineVersionFromInstance(d *schema.ResourceData, c *rds.DBInstance) { oldVersion := d.Get("engine_version").(string) newVersion := aws.StringValue(c.EngineVersion) - compareActualEngineVersion(d, oldVersion, newVersion) + var pendingVersion string + if c.PendingModifiedValues != nil && c.PendingModifiedValues.EngineVersion != nil { + pendingVersion = aws.StringValue(c.PendingModifiedValues.EngineVersion) + } + compareActualEngineVersion(d, oldVersion, newVersion, pendingVersion) } type dbInstanceARN struct { diff --git a/internal/service/rds/verify.go b/internal/service/rds/verify.go index c7df67724dc2..e98f2dbc296a 100644 --- a/internal/service/rds/verify.go +++ b/internal/service/rds/verify.go @@ -4,16 +4,30 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) -func compareActualEngineVersion(d *schema.ResourceData, oldVersion string, newVersion string) { - newVersionSubstr := newVersion +// compareActualEngineVersion sets engine version related attributes +// +// `engine_version_actual` is always set to newVersion +// +// `engine_version` is set to newVersion unless: +// - old and pending versions are equal (ie. the update is awaiting a +// maintenance window) +// - old and new versions are not exactly equal, but match after accounting +// for an omitted patch value in the configuration (ie. old="1.3", +// new="1.3.27" will not trigger a set) +func compareActualEngineVersion(d *schema.ResourceData, oldVersion, newVersion, pendingVersion string) { + d.Set("engine_version_actual", newVersion) + + if oldVersion != "" && oldVersion == pendingVersion { + return + } + newVersionSubstr := newVersion if len(newVersion) > len(oldVersion) { newVersionSubstr = string([]byte(newVersion)[0 : len(oldVersion)+1]) } - - if oldVersion != newVersion && string(append([]byte(oldVersion), []byte(".")...)) != newVersionSubstr { - d.Set("engine_version", newVersion) + if oldVersion != newVersion && oldVersion+"." == newVersionSubstr { + return } - d.Set("engine_version_actual", newVersion) + d.Set("engine_version", newVersion) } From 5b6e465713cad3c9ed18aa2469e9a98a3b8aff76 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 23 Mar 2023 16:01:20 -0400 Subject: [PATCH 2/4] r/aws_rds_cluster: engine_version, db_instance_parameter_group_name update adjustments --- internal/service/rds/cluster.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/service/rds/cluster.go b/internal/service/rds/cluster.go index 3ead3aa17873..54b2d0d815d9 100644 --- a/internal/service/rds/cluster.go +++ b/internal/service/rds/cluster.go @@ -1149,8 +1149,11 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, meta int input.DBClusterParameterGroupName = aws.String(d.Get("db_cluster_parameter_group_name").(string)) } - if d.HasChange("db_instance_parameter_group_name") { - input.DBInstanceParameterGroupName = aws.String(d.Get("db_instance_parameter_group_name").(string)) + // DB instance parameter group name is not currently returned from the + // DescribeDBClusters API. This means there is no drift detection, so when + // set, the configured attribute should always be sent on modify. + if v, ok := d.GetOk("db_instance_parameter_group_name"); ok || d.HasChange("db_instance_parameter_group_name") { + input.DBInstanceParameterGroupName = aws.String(v.(string)) } if d.HasChange("deletion_protection") { @@ -1180,6 +1183,13 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, meta int input.EngineVersion = aws.String(d.Get("engine_version").(string)) } + // This can happen when updates are deferred (apply_immediately = false), and + // multiple applies occur before the maintenance window. In this case, + // continue sending the desired engine_version as part of the modify request. + if d.Get("engine_version").(string) != d.Get("engine_version_actual").(string) { + input.EngineVersion = aws.String(d.Get("engine_version").(string)) + } + if d.HasChange("iam_database_authentication_enabled") { input.EnableIAMDatabaseAuthentication = aws.Bool(d.Get("iam_database_authentication_enabled").(bool)) } From bfa197ec2b787ee2f1766995d7c985312af11e70 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 23 Mar 2023 20:30:45 -0400 Subject: [PATCH 3/4] r/aws_rds_cluster: allowMajorVersionUpgradeNoApplyImmediately test --- internal/service/rds/cluster_test.go | 69 ++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/internal/service/rds/cluster_test.go b/internal/service/rds/cluster_test.go index 80af74131bf2..66c6da783213 100644 --- a/internal/service/rds/cluster_test.go +++ b/internal/service/rds/cluster_test.go @@ -217,7 +217,7 @@ func TestAccRDSCluster_allowMajorVersionUpgrade(t *testing.T) { CheckDestroy: testAccCheckClusterDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccClusterConfig_allowMajorVersionUpgrade(rName, true, engine, engineVersion1), + Config: testAccClusterConfig_allowMajorVersionUpgrade(rName, true, engine, engineVersion1, true), Check: resource.ComposeTestCheckFunc( testAccCheckClusterExists(ctx, resourceName, &dbCluster1), resource.TestCheckResourceAttr(resourceName, "allow_major_version_upgrade", "true"), @@ -240,7 +240,65 @@ func TestAccRDSCluster_allowMajorVersionUpgrade(t *testing.T) { }, }, { - Config: testAccClusterConfig_allowMajorVersionUpgrade(rName, true, engine, engineVersion2), + Config: testAccClusterConfig_allowMajorVersionUpgrade(rName, true, engine, engineVersion2, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists(ctx, resourceName, &dbCluster2), + resource.TestCheckResourceAttr(resourceName, "allow_major_version_upgrade", "true"), + resource.TestCheckResourceAttr(resourceName, "engine", engine), + resource.TestCheckResourceAttr(resourceName, "engine_version", engineVersion2), + ), + }, + }, + }) +} + +func TestAccRDSCluster_allowMajorVersionUpgradeNoApplyImmediately(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var dbCluster1, dbCluster2 rds.DBCluster + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_rds_cluster.test" + // If these hardcoded versions become a maintenance burden, use DescribeDBEngineVersions + // either by having a new data source created or implementing the testing similar + // to TestAccDMSReplicationInstance_engineVersion + engine := "aurora-postgresql" + engineVersion1 := "12.9" + engineVersion2 := "13.5" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckClusterDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccClusterConfig_allowMajorVersionUpgrade(rName, true, engine, engineVersion1, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists(ctx, resourceName, &dbCluster1), + resource.TestCheckResourceAttr(resourceName, "allow_major_version_upgrade", "true"), + resource.TestCheckResourceAttr(resourceName, "engine", engine), + resource.TestCheckResourceAttr(resourceName, "engine_version", engineVersion1), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "allow_major_version_upgrade", + "apply_immediately", + "cluster_identifier_prefix", + "db_instance_parameter_group_name", + "enable_global_write_forwarding", + "master_password", + "skip_final_snapshot", + }, + }, + { + Config: testAccClusterConfig_allowMajorVersionUpgrade(rName, true, engine, engineVersion2, false), Check: resource.ComposeTestCheckFunc( testAccCheckClusterExists(ctx, resourceName, &dbCluster2), resource.TestCheckResourceAttr(resourceName, "allow_major_version_upgrade", "true"), @@ -2370,11 +2428,11 @@ resource "aws_rds_cluster" "test" { `, identifierPrefix) } -func testAccClusterConfig_allowMajorVersionUpgrade(rName string, allowMajorVersionUpgrade bool, engine string, engineVersion string) string { +func testAccClusterConfig_allowMajorVersionUpgrade(rName string, allowMajorVersionUpgrade bool, engine string, engineVersion string, applyImmediately bool) string { return fmt.Sprintf(` resource "aws_rds_cluster" "test" { allow_major_version_upgrade = %[1]t - apply_immediately = true + apply_immediately = %[5]t cluster_identifier = %[2]q engine = %[3]q engine_version = %[4]q @@ -2391,6 +2449,7 @@ data "aws_rds_orderable_db_instance" "test" { # Upgrading requires a healthy primary instance resource "aws_rds_cluster_instance" "test" { + apply_immediately = %[5]t cluster_identifier = aws_rds_cluster.test.id engine = data.aws_rds_orderable_db_instance.test.engine engine_version = data.aws_rds_orderable_db_instance.test.engine_version @@ -2401,7 +2460,7 @@ resource "aws_rds_cluster_instance" "test" { ignore_changes = [engine_version] } } -`, allowMajorVersionUpgrade, rName, engine, engineVersion) +`, allowMajorVersionUpgrade, rName, engine, engineVersion, applyImmediately) } func testAccClusterConfig_allowMajorVersionUpgradeCustomParameters(rName string, allowMajorVersionUpgrade bool, engine string, engineVersion string, applyImmediate bool) string { From dd79b922c54c7f86c78bc0b95c82fa4bc48fa870 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Fri, 24 Mar 2023 11:12:27 -0400 Subject: [PATCH 4/4] chore: changelog --- .changelog/30247.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changelog/30247.txt diff --git a/.changelog/30247.txt b/.changelog/30247.txt new file mode 100644 index 000000000000..436360c49baa --- /dev/null +++ b/.changelog/30247.txt @@ -0,0 +1,16 @@ +```release-note:bug +resource/aws_rds_cluster: Fix inconsistent final plan errors when `engine_version` updates are not applied immediately +``` + +```release-note:bug +resource/aws_rds_cluster_instance: Fix inconsistent final plan errors when `engine_version` updates are not applied immediately +``` + +```release-note:bug +resource/aws_rds_instance: Fix inconsistent final plan errors when `engine_version` updates are not applied immediately +``` + +```release-note:bug +resource/aws_rds_cluster: Send `db_instance_parameter_group_name` on all modify requests when set +``` +