service/rds: Initial support for RDS Aurora Global Databases #6861

bflad · 2018-12-14T22:11:59Z

Changes:

resource/aws_rds_cluster: Allow global in engine_mode validation
resource/aws_rds_cluster: Add global_cluster_identifier argument
resource/aws_rds_cluster: Automatically remove cluster from global cluster on delete
New Resource: aws_rds_global_cluster

--- PASS: TestAccAWSRDSCluster_BacktrackWindow (185.53s)
--- PASS: TestAccAWSRDSCluster_backupsUpdate (206.64s)
--- PASS: TestAccAWSRDSCluster_basic (135.45s)
--- PASS: TestAccAWSRDSCluster_DeletionProtection (165.47s)
--- PASS: TestAccAWSRDSCluster_encrypted (126.42s)
--- PASS: TestAccAWSRDSCluster_EncryptedCrossRegionReplication (1609.46s)
--- PASS: TestAccAWSRDSCluster_EngineMode (344.98s)
--- PASS: TestAccAWSRDSCluster_EngineMode_Global (118.41s)
--- PASS: TestAccAWSRDSCluster_EngineMode_ParallelQuery (117.81s)
--- PASS: TestAccAWSRDSCluster_EngineVersion (122.85s)
--- PASS: TestAccAWSRDSCluster_EngineVersionWithPrimaryInstance (1022.02s)
--- PASS: TestAccAWSRDSCluster_generatedName (176.05s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier (125.53s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Add (132.58s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove (140.14s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Update (124.50s)
--- PASS: TestAccAWSRDSCluster_iamAuth (115.26s)
--- PASS: TestAccAWSRDSCluster_importBasic (116.22s)
--- PASS: TestAccAWSRDSCluster_kmsKey (144.50s)
--- PASS: TestAccAWSRDSCluster_missingUserNameCausesError (2.31s)
--- PASS: TestAccAWSRDSCluster_namePrefix (145.07s)
--- PASS: TestAccAWSRDSCluster_Port (252.80s)
--- PASS: TestAccAWSRDSCluster_s3Restore (1474.31s)
--- PASS: TestAccAWSRDSCluster_ScalingConfiguration (270.01s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier (438.26s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_DeletionProtection (374.71s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EncryptedRestore (331.91s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineMode_ParallelQuery (353.25s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineMode_Provisioned (355.21s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineVersion_Different (358.34s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineVersion_Equal (394.42s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_PreferredBackupWindow (380.13s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_PreferredMaintenanceWindow (334.25s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_Tags (354.72s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_VpcSecurityGroupIds (255.20s)
--- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_VpcSecurityGroupIds_Tags (271.42s)
--- PASS: TestAccAWSRDSCluster_takeFinalSnapshot (93.01s)
--- PASS: TestAccAWSRDSCluster_updateCloudwatchLogsExports (158.10s)
--- PASS: TestAccAWSRDSCluster_updateIamRoles (122.85s)
--- PASS: TestAccAWSRDSCluster_updateTags (153.91s)

--- PASS: TestAccAWSRdsGlobalCluster_basic (14.24s)
--- PASS: TestAccAWSRdsGlobalCluster_DatabaseName (24.06s)
--- PASS: TestAccAWSRdsGlobalCluster_DeletionProtection (22.31s)
--- PASS: TestAccAWSRdsGlobalCluster_disappears (11.16s)
--- PASS: TestAccAWSRdsGlobalCluster_Engine_Aurora (14.41s)
--- PASS: TestAccAWSRdsGlobalCluster_EngineVersion_Aurora (14.24s)
--- PASS: TestAccAWSRdsGlobalCluster_StorageEncrypted (25.60s)

Changes: * resource/aws_rds_cluster: Add `global_cluster_identifier` argument * resource/aws_rds_cluster: Allow `global` in `engine_mode` validation * resource/aws_rds_cluster: Automatically remove cluster from global cluster on delete * New Resource: `aws_rds_global_cluster` ``` --- PASS: TestAccAWSRDSCluster_BacktrackWindow (185.53s) --- PASS: TestAccAWSRDSCluster_backupsUpdate (206.64s) --- PASS: TestAccAWSRDSCluster_basic (135.45s) --- PASS: TestAccAWSRDSCluster_DeletionProtection (165.47s) --- PASS: TestAccAWSRDSCluster_encrypted (126.42s) --- PASS: TestAccAWSRDSCluster_EncryptedCrossRegionReplication (1609.46s) --- PASS: TestAccAWSRDSCluster_EngineMode (344.98s) --- PASS: TestAccAWSRDSCluster_EngineMode_Global (118.41s) --- PASS: TestAccAWSRDSCluster_EngineMode_ParallelQuery (117.81s) --- PASS: TestAccAWSRDSCluster_EngineVersion (122.85s) --- PASS: TestAccAWSRDSCluster_EngineVersionWithPrimaryInstance (1022.02s) --- PASS: TestAccAWSRDSCluster_generatedName (176.05s) --- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier (125.53s) --- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Add (132.58s) --- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove (140.14s) --- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Update (124.50s) --- PASS: TestAccAWSRDSCluster_iamAuth (115.26s) --- PASS: TestAccAWSRDSCluster_importBasic (116.22s) --- PASS: TestAccAWSRDSCluster_kmsKey (144.50s) --- PASS: TestAccAWSRDSCluster_missingUserNameCausesError (2.31s) --- PASS: TestAccAWSRDSCluster_namePrefix (145.07s) --- PASS: TestAccAWSRDSCluster_Port (252.80s) --- PASS: TestAccAWSRDSCluster_s3Restore (1474.31s) --- PASS: TestAccAWSRDSCluster_ScalingConfiguration (270.01s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier (438.26s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_DeletionProtection (374.71s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EncryptedRestore (331.91s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineMode_ParallelQuery (353.25s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineMode_Provisioned (355.21s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineVersion_Different (358.34s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_EngineVersion_Equal (394.42s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_PreferredBackupWindow (380.13s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_PreferredMaintenanceWindow (334.25s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_Tags (354.72s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_VpcSecurityGroupIds (255.20s) --- PASS: TestAccAWSRDSCluster_SnapshotIdentifier_VpcSecurityGroupIds_Tags (271.42s) --- PASS: TestAccAWSRDSCluster_takeFinalSnapshot (93.01s) --- PASS: TestAccAWSRDSCluster_updateCloudwatchLogsExports (158.10s) --- PASS: TestAccAWSRDSCluster_updateIamRoles (122.85s) --- PASS: TestAccAWSRDSCluster_updateTags (153.91s) --- PASS: TestAccAWSRdsGlobalCluster_basic (14.24s) --- PASS: TestAccAWSRdsGlobalCluster_DatabaseName (24.06s) --- PASS: TestAccAWSRdsGlobalCluster_DeletionProtection (22.31s) --- PASS: TestAccAWSRdsGlobalCluster_disappears (11.16s) --- PASS: TestAccAWSRdsGlobalCluster_Engine_Aurora (14.41s) --- PASS: TestAccAWSRdsGlobalCluster_EngineVersion_Aurora (14.24s) --- PASS: TestAccAWSRdsGlobalCluster_StorageEncrypted (25.60s) ```

mbfrahry

LGTM with some minor comments

aws/resource_aws_rds_cluster.go

mbfrahry · 2018-12-19T06:28:57Z

aws/resource_aws_rds_cluster.go

+	// Automatically remove from global cluster to bypass this error on deletion:
+	// InvalidDBClusterStateFault: This cluster is a part of a global cluster, please remove it from globalcluster first
+	if d.Get("global_cluster_identifier").(string) != "" {
+		input := &rds.RemoveFromGlobalClusterInput{


Since this same chunk of code is used in update, I'd suggest having it in it's own function

The error handling is slightly different. 😄 I'm not sure if the indirection is worthwhile to fill in a two value struct, print a log line, and call the API, but we can certainly discuss this out of band! 👍

That's good with me!

mbfrahry · 2018-12-19T06:34:36Z

aws/resource_aws_rds_global_cluster.go

+				Type:     schema.TypeString,
+				Computed: true,
+			},
+			"source_db_cluster_identifier": {


I don't see this being used anywhere. Should we remove it or was it missed?

Good find and probably something I should've mentioned in the commit message/PR comment!

The RDS API gives you this second option for creating Global Clusters, based on an existing Cluster with global engine mode, instead of creating an "empty" Global Cluster. I think there were two reasons for omitting this to start.

When playing with this, the workflow wound up being very tedious because of operation ordering requirements within the RDS API and Terraform references. If I recall correctly, since the Cluster becomes a child of the Global Cluster in the dependency graph and Global Clusters, it wasn't easy to setup the resources to properly destroy. I may have inadvertently fixed this issue with the aws_rds_cluster resource Delete function update though. 😅

The other workflow issue is that engine mode cannot be currently updated for existing Clusters, so essentially you have to create new Clusters anyways to get Global Cluster support. I didn't think we needed to introduce all the complexity of supporting both creation methods initially, since operators can just adjust their initial Global Cluster configuration to match our supported workflow. I may have missed something in my thinking there though.

TL;DR I'll remove this attribute for now since its not used. If there are use cases that require it, we can add it (and all its complexity) later. 👍

Now that I'm thinking about it more -- it was that because of the referencing (Global Cluster being dependent on Cluster), we'd try to destroy the Global Cluster first and it would complain that it still had members. We'd have to introduce a flag in aws_rds_global_cluster to allow you to force remove all members first. Not unsurmountable, but definitely additional complexity which we might not need.

Ouch, that extra flag seems tricky but I'm totally onboard with seeing this come in later if it's requested.

Co-Authored-By: bflad <bflad417@gmail.com>

…e_db_cluster_identifier attribute

bflad · 2018-12-19T17:31:20Z

TravisCI is green and re-verified the acceptance testing around the adjusted conditional, merging!

--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Update (118.41s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Add (125.06s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove (141.67s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier (166.14s)

bflad · 2018-12-20T20:18:10Z

This has been released in version 1.53.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-01T17:44:28Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

bflad added enhancement Requests to existing resources that expand the functionality or scope. new-resource Introduces a new resource. service/rds Issues and PRs that pertain to the rds service. labels Dec 14, 2018

bflad requested a review from a team December 14, 2018 22:12

bflad mentioned this pull request Dec 14, 2018

Feature Request: Add engine mode for global database to aws_rds_cluster #6846

Closed

mbfrahry approved these changes Dec 19, 2018

View reviewed changes

mbfrahry and others added 2 commits December 19, 2018 11:58

resource/aws_rds_cluster: Remove extraneous conditional

fd69e2d

Co-Authored-By: bflad <bflad417@gmail.com>

resource/aws_rds_global_cluster: Remove currently unimplemented sourc…

2754910

…e_db_cluster_identifier attribute

bflad added this to the v1.53.0 milestone Dec 19, 2018

bflad merged commit 659b02d into master Dec 19, 2018

bflad deleted the f-aws_rds_global_cluster branch December 19, 2018 17:31

bflad added a commit that referenced this pull request Dec 19, 2018

Update CHANGELOG for #6861

f8c623d

ghost locked and limited conversation to collaborators Apr 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

service/rds: Initial support for RDS Aurora Global Databases #6861

service/rds: Initial support for RDS Aurora Global Databases #6861

bflad commented Dec 14, 2018

mbfrahry left a comment

mbfrahry Dec 19, 2018

bflad Dec 19, 2018

mbfrahry Dec 19, 2018

mbfrahry Dec 19, 2018

bflad Dec 19, 2018

bflad Dec 19, 2018

mbfrahry Dec 19, 2018

bflad commented Dec 19, 2018

bflad commented Dec 20, 2018

ghost commented Apr 1, 2020

service/rds: Initial support for RDS Aurora Global Databases #6861

service/rds: Initial support for RDS Aurora Global Databases #6861

Conversation

bflad commented Dec 14, 2018

mbfrahry left a comment

Choose a reason for hiding this comment

mbfrahry Dec 19, 2018

Choose a reason for hiding this comment

bflad Dec 19, 2018

Choose a reason for hiding this comment

mbfrahry Dec 19, 2018

Choose a reason for hiding this comment

mbfrahry Dec 19, 2018

Choose a reason for hiding this comment

bflad Dec 19, 2018

Choose a reason for hiding this comment

bflad Dec 19, 2018

Choose a reason for hiding this comment

mbfrahry Dec 19, 2018

Choose a reason for hiding this comment

bflad commented Dec 19, 2018

bflad commented Dec 20, 2018

ghost commented Apr 1, 2020