From 25da592bbb602e3703c28b38b7661b3d2e4fa77f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 19 Jul 2024 10:17:29 -0400 Subject: [PATCH 1/4] r/aws_rds_cluster: Wait for no pending modified values on Update if 'apply_immediately' is true. --- internal/service/rds/cluster.go | 83 ++++++++++++--------- internal/service/rds/cluster_snapshot.go | 4 +- internal/service/rds/consts.go | 41 +++++----- internal/service/rds/exports_test.go | 1 + internal/service/rds/find.go | 2 +- internal/service/rds/service_package_gen.go | 2 +- internal/service/rds/sweep.go | 2 +- internal/service/rds/wait.go | 6 +- 8 files changed, 79 insertions(+), 62 deletions(-) diff --git a/internal/service/rds/cluster.go b/internal/service/rds/cluster.go index 2a2ad3bc96b..e514926f2c8 100644 --- a/internal/service/rds/cluster.go +++ b/internal/service/rds/cluster.go @@ -27,6 +27,7 @@ import ( tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + itypes "github.com/hashicorp/terraform-provider-aws/internal/types" "github.com/hashicorp/terraform-provider-aws/internal/verify" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -40,7 +41,7 @@ const ( // @SDKResource("aws_rds_cluster", name="Cluster") // @Tags(identifierAttribute="arn") // @Testing(tagsTest=false) -func ResourceCluster() *schema.Resource { +func resourceCluster() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceClusterCreate, ReadWithoutTimeout: resourceClusterRead, @@ -1174,7 +1175,7 @@ func resourceClusterCreate(ctx context.Context, d *schema.ResourceData, meta int return sdkdiag.AppendErrorf(diags, "updating RDS Cluster (%s): %s", d.Id(), err) } - if _, err := waitDBClusterUpdated(ctx, conn, d.Id(), d.Timeout(schema.TimeoutCreate)); err != nil { + if _, err := waitDBClusterUpdated(ctx, conn, d.Id(), true, d.Timeout(schema.TimeoutCreate)); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for RDS Cluster (%s) update: %s", d.Id(), err) } } @@ -1333,8 +1334,9 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, meta int "replication_source_identifier", "skip_final_snapshot", names.AttrTags, names.AttrTagsAll) { + applyImmediately := d.Get(names.AttrApplyImmediately).(bool) input := &rds.ModifyDBClusterInput{ - ApplyImmediately: aws.Bool(d.Get(names.AttrApplyImmediately).(bool)), + ApplyImmediately: aws.Bool(applyImmediately), DBClusterIdentifier: aws.String(d.Id()), } @@ -1509,7 +1511,7 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, meta int return sdkdiag.AppendErrorf(diags, "updating RDS Cluster (%s): %s", d.Id(), err) } - if _, err := waitDBClusterUpdated(ctx, conn, d.Id(), d.Timeout(schema.TimeoutUpdate)); err != nil { + if _, err := waitDBClusterUpdated(ctx, conn, d.Id(), applyImmediately, d.Timeout(schema.TimeoutUpdate)); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for RDS Cluster (%s) update: %s", d.Id(), err) } } @@ -1636,7 +1638,7 @@ func resourceClusterDelete(ctx context.Context, d *schema.ResourceData, meta int return false, fmt.Errorf("modifying RDS Cluster (%s) DeletionProtection=false: %s", d.Id(), err) } - if _, err := waitDBClusterUpdated(ctx, conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil { + if _, err := waitDBClusterUpdated(ctx, conn, d.Id(), false, d.Timeout(schema.TimeoutDelete)); err != nil { return false, fmt.Errorf("waiting for RDS Cluster (%s) update: %s", d.Id(), err) } } @@ -1785,7 +1787,7 @@ func findDBClusters(ctx context.Context, conn *rds.RDS, input *rds.DescribeDBClu return output, nil } -func statusDBCluster(ctx context.Context, conn *rds.RDS, id string) retry.StateRefreshFunc { +func statusDBCluster(ctx context.Context, conn *rds.RDS, id string, waitNoPendingModifiedValues bool) retry.StateRefreshFunc { return func() (interface{}, string, error) { output, err := FindDBClusterByID(ctx, conn, id) @@ -1797,23 +1799,29 @@ func statusDBCluster(ctx context.Context, conn *rds.RDS, id string) retry.StateR return nil, "", err } - return output, aws.StringValue(output.Status), nil + status := aws.StringValue(output.Status) + + if status == clusterStatusAvailable && waitNoPendingModifiedValues && !itypes.IsZero(output.PendingModifiedValues) { + status = clusterStatusAvailableWithPendingModifiedValues + } + + return output, status, nil } } func waitDBClusterCreated(ctx context.Context, conn *rds.RDS, id string, timeout time.Duration) (*rds.DBCluster, error) { stateConf := &retry.StateChangeConf{ Pending: []string{ - ClusterStatusBackingUp, - ClusterStatusCreating, - ClusterStatusMigrating, - ClusterStatusModifying, - ClusterStatusPreparingDataMigration, - ClusterStatusRebooting, - ClusterStatusResettingMasterCredentials, + clusterStatusBackingUp, + clusterStatusCreating, + clusterStatusMigrating, + clusterStatusModifying, + clusterStatusPreparingDataMigration, + clusterStatusRebooting, + clusterStatusResettingMasterCredentials, }, - Target: []string{ClusterStatusAvailable}, - Refresh: statusDBCluster(ctx, conn, id), + Target: []string{clusterStatusAvailable}, + Refresh: statusDBCluster(ctx, conn, id, false), Timeout: timeout, MinTimeout: 10 * time.Second, Delay: 30 * time.Second, @@ -1828,19 +1836,24 @@ func waitDBClusterCreated(ctx context.Context, conn *rds.RDS, id string, timeout return nil, err } -func waitDBClusterUpdated(ctx context.Context, conn *rds.RDS, id string, timeout time.Duration) (*rds.DBCluster, error) { //nolint:unparam +func waitDBClusterUpdated(ctx context.Context, conn *rds.RDS, id string, waitNoPendingModifiedValues bool, timeout time.Duration) (*rds.DBCluster, error) { //nolint:unparam + pendingStatuses := []string{ + clusterStatusBackingUp, + clusterStatusConfiguringIAMDatabaseAuth, + clusterStatusModifying, + clusterStatusRenaming, + clusterStatusResettingMasterCredentials, + clusterStatusScalingCompute, + clusterStatusUpgrading, + } + if waitNoPendingModifiedValues { + pendingStatuses = append(pendingStatuses, clusterStatusAvailableWithPendingModifiedValues) + } + stateConf := &retry.StateChangeConf{ - Pending: []string{ - ClusterStatusBackingUp, - ClusterStatusConfiguringIAMDatabaseAuth, - ClusterStatusModifying, - ClusterStatusRenaming, - ClusterStatusResettingMasterCredentials, - ClusterStatusScalingCompute, - ClusterStatusUpgrading, - }, - Target: []string{ClusterStatusAvailable}, - Refresh: statusDBCluster(ctx, conn, id), + Pending: pendingStatuses, + Target: []string{clusterStatusAvailable}, + Refresh: statusDBCluster(ctx, conn, id, waitNoPendingModifiedValues), Timeout: timeout, MinTimeout: 10 * time.Second, Delay: 30 * time.Second, @@ -1858,15 +1871,15 @@ func waitDBClusterUpdated(ctx context.Context, conn *rds.RDS, id string, timeout func waitDBClusterDeleted(ctx context.Context, conn *rds.RDS, id string, timeout time.Duration) (*rds.DBCluster, error) { stateConf := &retry.StateChangeConf{ Pending: []string{ - ClusterStatusAvailable, - ClusterStatusBackingUp, - ClusterStatusDeleting, - ClusterStatusModifying, - ClusterStatusPromoting, - ClusterStatusScalingCompute, + clusterStatusAvailable, + clusterStatusBackingUp, + clusterStatusDeleting, + clusterStatusModifying, + clusterStatusPromoting, + clusterStatusScalingCompute, }, Target: []string{}, - Refresh: statusDBCluster(ctx, conn, id), + Refresh: statusDBCluster(ctx, conn, id, false), Timeout: timeout, MinTimeout: 10 * time.Second, Delay: 30 * time.Second, diff --git a/internal/service/rds/cluster_snapshot.go b/internal/service/rds/cluster_snapshot.go index 188112ec12f..724d228e8cd 100644 --- a/internal/service/rds/cluster_snapshot.go +++ b/internal/service/rds/cluster_snapshot.go @@ -296,8 +296,8 @@ func statusDBClusterSnapshot(ctx context.Context, conn *rds.RDS, id string) retr func waitDBClusterSnapshotCreated(ctx context.Context, conn *rds.RDS, id string, timeout time.Duration) (*rds.DBClusterSnapshot, error) { stateConf := &retry.StateChangeConf{ - Pending: []string{ClusterSnapshotStatusCreating}, - Target: []string{ClusterSnapshotStatusAvailable}, + Pending: []string{clusterSnapshotStatusCreating}, + Target: []string{clusterSnapshotStatusAvailable}, Refresh: statusDBClusterSnapshot(ctx, conn, id), Timeout: timeout, MinTimeout: 10 * time.Second, diff --git a/internal/service/rds/consts.go b/internal/service/rds/consts.go index fe7e5365271..7bad796afc2 100644 --- a/internal/service/rds/consts.go +++ b/internal/service/rds/consts.go @@ -10,31 +10,34 @@ import ( ) const ( - ClusterRoleStatusActive = "ACTIVE" - ClusterRoleStatusDeleted = "DELETED" - ClusterRoleStatusPending = "PENDING" + clusterRoleStatusActive = "ACTIVE" + clusterRoleStatusDeleted = "DELETED" + clusterRoleStatusPending = "PENDING" ) const ( - ClusterStatusAvailable = "available" - ClusterStatusBackingUp = "backing-up" - ClusterStatusConfiguringIAMDatabaseAuth = "configuring-iam-database-auth" - ClusterStatusCreating = "creating" - ClusterStatusDeleting = "deleting" - ClusterStatusMigrating = "migrating" - ClusterStatusModifying = "modifying" - ClusterStatusPreparingDataMigration = "preparing-data-migration" - ClusterStatusPromoting = "promoting" - ClusterStatusRebooting = "rebooting" - ClusterStatusRenaming = "renaming" - ClusterStatusResettingMasterCredentials = "resetting-master-credentials" - ClusterStatusScalingCompute = "scaling-compute" - ClusterStatusUpgrading = "upgrading" + clusterStatusAvailable = "available" + clusterStatusBackingUp = "backing-up" + clusterStatusConfiguringIAMDatabaseAuth = "configuring-iam-database-auth" + clusterStatusCreating = "creating" + clusterStatusDeleting = "deleting" + clusterStatusMigrating = "migrating" + clusterStatusModifying = "modifying" + clusterStatusPreparingDataMigration = "preparing-data-migration" + clusterStatusPromoting = "promoting" + clusterStatusRebooting = "rebooting" + clusterStatusRenaming = "renaming" + clusterStatusResettingMasterCredentials = "resetting-master-credentials" + clusterStatusScalingCompute = "scaling-compute" + clusterStatusUpgrading = "upgrading" + + // Non-standard status values. + clusterStatusAvailableWithPendingModifiedValues = "tf-available-with-pending-modified-values" ) const ( - ClusterSnapshotStatusAvailable = "available" - ClusterSnapshotStatusCreating = "creating" + clusterSnapshotStatusAvailable = "available" + clusterSnapshotStatusCreating = "creating" ) const ( diff --git a/internal/service/rds/exports_test.go b/internal/service/rds/exports_test.go index 2b042e2d7c5..2c5698543e7 100644 --- a/internal/service/rds/exports_test.go +++ b/internal/service/rds/exports_test.go @@ -6,6 +6,7 @@ package rds // Exports for use in tests only. var ( ResourceCertificate = resourceCertificate + ResourceCluster = resourceCluster ResourceEventSubscription = resourceEventSubscription ResourceProxy = resourceProxy ResourceProxyDefaultTargetGroup = resourceProxyDefaultTargetGroup diff --git a/internal/service/rds/find.go b/internal/service/rds/find.go index a314ca691a8..44401bb7ef4 100644 --- a/internal/service/rds/find.go +++ b/internal/service/rds/find.go @@ -21,7 +21,7 @@ func FindDBClusterRoleByDBClusterIDAndRoleARN(ctx context.Context, conn *rds.RDS for _, associatedRole := range dbCluster.AssociatedRoles { if aws.StringValue(associatedRole.RoleArn) == roleARN { - if status := aws.StringValue(associatedRole.Status); status == ClusterRoleStatusDeleted { + if status := aws.StringValue(associatedRole.Status); status == clusterRoleStatusDeleted { return nil, &retry.NotFoundError{ Message: status, } diff --git a/internal/service/rds/service_package_gen.go b/internal/service/rds/service_package_gen.go index ce0328607ae..1c43f4ec6e5 100644 --- a/internal/service/rds/service_package_gen.go +++ b/internal/service/rds/service_package_gen.go @@ -205,7 +205,7 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka Name: "Default Certificate", }, { - Factory: ResourceCluster, + Factory: resourceCluster, TypeName: "aws_rds_cluster", Name: "Cluster", Tags: &types.ServicePackageResourceTags{ diff --git a/internal/service/rds/sweep.go b/internal/service/rds/sweep.go index 9a88e4e6609..5e1ec5bf94c 100644 --- a/internal/service/rds/sweep.go +++ b/internal/service/rds/sweep.go @@ -229,7 +229,7 @@ func sweepClusters(region string) error { for _, v := range page.DBClusters { arn := aws.StringValue(v.DBClusterArn) id := aws.StringValue(v.DBClusterIdentifier) - r := ResourceCluster() + r := resourceCluster() d := r.Data(nil) d.SetId(id) d.Set(names.AttrApplyImmediately, true) diff --git a/internal/service/rds/wait.go b/internal/service/rds/wait.go index e4bc79d9bdf..1932090e361 100644 --- a/internal/service/rds/wait.go +++ b/internal/service/rds/wait.go @@ -13,8 +13,8 @@ import ( func waitDBClusterRoleAssociationCreated(ctx context.Context, conn *rds.RDS, dbClusterID, roleARN string, timeout time.Duration) (*rds.DBClusterRole, error) { stateConf := &retry.StateChangeConf{ - Pending: []string{ClusterRoleStatusPending}, - Target: []string{ClusterRoleStatusActive}, + Pending: []string{clusterRoleStatusPending}, + Target: []string{clusterRoleStatusActive}, Refresh: statusDBClusterRole(ctx, conn, dbClusterID, roleARN), Timeout: timeout, MinTimeout: 10 * time.Second, @@ -32,7 +32,7 @@ func waitDBClusterRoleAssociationCreated(ctx context.Context, conn *rds.RDS, dbC func waitDBClusterRoleAssociationDeleted(ctx context.Context, conn *rds.RDS, dbClusterID, roleARN string, timeout time.Duration) (*rds.DBClusterRole, error) { stateConf := &retry.StateChangeConf{ - Pending: []string{ClusterRoleStatusActive, ClusterRoleStatusPending}, + Pending: []string{clusterRoleStatusActive, clusterRoleStatusPending}, Target: []string{}, Refresh: statusDBClusterRole(ctx, conn, dbClusterID, roleARN), Timeout: timeout, From 3aeebdc2953f020c60ff98dc48679d10a84ada23 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 19 Jul 2024 11:03:24 -0400 Subject: [PATCH 2/4] Acceptance test output: % make testacc TESTARGS='-run=TestAccRDSCluster_allowMajorVersionUpgrade' PKG=rds make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.22.5 test ./internal/service/rds/... -v -count 1 -parallel 20 -run=TestAccRDSCluster_allowMajorVersionUpgrade -timeout 360m === RUN TestAccRDSCluster_allowMajorVersionUpgrade === PAUSE TestAccRDSCluster_allowMajorVersionUpgrade === RUN TestAccRDSCluster_allowMajorVersionUpgradeNoApplyImmediately === PAUSE TestAccRDSCluster_allowMajorVersionUpgradeNoApplyImmediately === RUN TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParametersApplyImm === PAUSE TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParametersApplyImm === RUN TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParameters === PAUSE TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParameters === CONT TestAccRDSCluster_allowMajorVersionUpgrade === CONT TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParametersApplyImm === CONT TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParameters === CONT TestAccRDSCluster_allowMajorVersionUpgradeNoApplyImmediately --- PASS: TestAccRDSCluster_allowMajorVersionUpgradeNoApplyImmediately (1283.81s) --- PASS: TestAccRDSCluster_allowMajorVersionUpgrade (1924.24s) --- PASS: TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParametersApplyImm (2250.30s) --- PASS: TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParameters (2315.92s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/rds 2320.945s From ac8c89e7ad9a179b9fbb62f96ce01917619eb017 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 22 Jul 2024 07:48:54 -0400 Subject: [PATCH 3/4] r/aws_rds_cluster: Mark `ca_certificate_identifier` as Computed. --- internal/service/rds/cluster.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/rds/cluster.go b/internal/service/rds/cluster.go index e514926f2c8..70bb734c71f 100644 --- a/internal/service/rds/cluster.go +++ b/internal/service/rds/cluster.go @@ -117,6 +117,7 @@ func resourceCluster() *schema.Resource { "ca_certificate_identifier": { Type: schema.TypeString, Optional: true, + Computed: true, }, "ca_certificate_valid_till": { Type: schema.TypeString, From 5810bc42d388fcb7b33913a6663f487fa8455766 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 22 Jul 2024 15:14:30 -0400 Subject: [PATCH 4/4] Add CHANGELOG entry. --- .changelog/38437.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/38437.txt diff --git a/.changelog/38437.txt b/.changelog/38437.txt new file mode 100644 index 00000000000..94c25499c8e --- /dev/null +++ b/.changelog/38437.txt @@ -0,0 +1,7 @@ +```release-note:bug +resource/aws_rds_cluster: Mark `ca_certificate_identifier` as Computed +``` + +```release-note:bug +resource/aws_rds_cluster: Wait for no pending modified values on Update if `apply_immediately` is `true`. This fixes `InvalidParameterCombination` errors when updating `engine_version` +``` \ No newline at end of file