Fixing global rds by allowing for optional parameters on cluster #7213

bculberson · 2019-01-19T00:18:34Z

TBD: Add global acc test

Changes proposed in this pull request:

Change 1
Allow username and password to be optional when using a global cluster identifier
Change 2
Allow source region and global cluster identifier to be set for secondary region to come online

Output from acceptance testing:

make testacc TESTARGS='-run=TestAccAWSRDSCluster_GlobalClusterIdentifier'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSRDSCluster_GlobalClusterIdentifier -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSRDSCluster_GlobalClusterIdentifier
=== PAUSE TestAccAWSRDSCluster_GlobalClusterIdentifier
=== RUN   TestAccAWSRDSCluster_GlobalClusterIdentifier_Add
=== PAUSE TestAccAWSRDSCluster_GlobalClusterIdentifier_Add
=== RUN   TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove
=== PAUSE TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove
=== RUN   TestAccAWSRDSCluster_GlobalClusterIdentifier_Update
=== PAUSE TestAccAWSRDSCluster_GlobalClusterIdentifier_Update
=== CONT  TestAccAWSRDSCluster_GlobalClusterIdentifier
=== CONT  TestAccAWSRDSCluster_GlobalClusterIdentifier_Update
=== CONT  TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove
=== CONT  TestAccAWSRDSCluster_GlobalClusterIdentifier_Add
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Add (218.57s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Update (218.81s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier (243.67s)
--- PASS: TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove (250.19s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	250.852s

...

… rds

adamscharf · 2019-02-15T18:08:47Z

I'd really like to see this merged as I'm facing the issues outlined in #7212

Edit: Fixed typo

bflad

Hi @bculberson 👋 Thank you so much for fixing this. I left a few notes below, but we'll definitely do the merge for the resource and documentation updates as they are good to go with the very minor adjustment to prevent a Terraform crash. 🚀

bflad · 2019-02-22T07:16:20Z

aws/resource_aws_rds_cluster.go

-			return fmt.Errorf(`provider.aws: aws_rds_cluster: %s: "master_username": required field is not set`, d.Get("database_name").(string))
+		if _, ok := d.GetOk("global_cluster_identifier"); !ok {
+			if _, ok := d.GetOk("master_password"); !ok {
+				return fmt.Errorf(`provider.aws: aws_db_instance: %s: "master_password": required field is not set`, d.Get("name").(string))


name is not an attribute in this resource's schema, so can cause a panic as found by the acceptance testing:

=== CONT TestAccAWSRDSCluster_missingUserNameCausesError ------- Stderr: ------- panic: interface conversion: interface {} is nil, not string goroutine 137 [running]: github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsRDSClusterCreate(0xc000349260, 0x3c28320, 0xc0001adc00, 0xc000349260, 0x0) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_rds_cluster.go:718 +0x6d9d

Will fix this on merge.

bflad · 2019-02-22T07:17:48Z

aws/resource_aws_rds_cluster_test.go

+%s
+
+resource "aws_rds_cluster" "test" {
+  depends_on		   = ["aws_db_subnet_group.dbsubnet"]


Instead of using depends_on here, a direct reference can be used below instead:

db_subnet_group_name = "${aws_db_subnet_group.dbsubnet.name}"

bflad · 2019-02-22T07:18:36Z

aws/resource_aws_rds_cluster_test.go

 resource "aws_rds_global_cluster" "test" {
  global_cluster_identifier = %q
 }

 resource "aws_rds_cluster" "test" {
+  depends_on				= ["aws_db_subnet_group.dbsubnet"]


depends_on is extraneous here as there is already a direct reference below.

bflad · 2019-02-22T07:24:43Z

aws/resource_aws_rds_cluster_test.go

+resource "aws_vpc" "vpc" {
+  cidr_block = "10.0.0.0/16"
+  tags = {
+  	Name = "terraform-acctest-rds-cluster-global-cross-region"


Setting up cross-region acceptance testing is a pretty complicated undertaking (see also: TestAccAWSRDSCluster_EncryptedCrossRegionReplication). 😅 Since the implementation in 9b7a9b0 is only halfway there and has a few other little issues, opting to merge this pull request, but revert that commit until we can get a full, new test setup, preferably separate from the existing ones.

See also: #7213 (comment)

…or master_username Reference: #7213 (comment) Previous output from acceptance testing: ``` === CONT TestAccAWSRDSCluster_missingUserNameCausesError ------- Stderr: ------- panic: interface conversion: interface {} is nil, not string goroutine 137 [running]: github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsRDSClusterCreate(0xc000349260, 0x3c28320, 0xc0001adc00, 0xc000349260, 0x0) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_rds_cluster.go:718 +0x6d9d ``` Output from acceptance testing: ``` --- PASS: TestAccAWSRDSCluster_missingUserNameCausesError (3.24s) ```

bflad · 2019-02-22T08:03:38Z

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

typez · 2019-10-03T19:08:54Z

Unfortunately, it looks like this change hadn't actually fixed the problem. I had no luck at all trying to create a global RDS cluster via Terraform. There is a new related issue with all the details - #10188

diprochatterjee · 2019-11-01T13:50:10Z

Unfortunately this is still an issue. I have the same issue with creating a rds global cluster. The rds global cluster gets created but when I try to create an rds cluster as the primary cluster for the global cluster, it fails with error - * aws_rds_cluster.primary: error creating RDS cluster: InvalidParameterValue: The engine mode global you requested is currently unavailable. I am using Terraform version 0.11.11

ghost · 2019-11-01T14:50:16Z

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!

ghost added size/XS Managed by automation to categorize the size of a PR. service/rds Issues and PRs that pertain to the rds service. documentation Introduces or discusses updates to documentation. labels Jan 19, 2019

bculberson changed the title ~~GH-7212 allow for optional parameters on cluster with global rds~~ Fixing global rds by allowing for optional parameters on cluster Jan 19, 2019

bculberson closed this Jan 21, 2019

bculberson reopened this Jan 22, 2019

bculberson mentioned this pull request Jan 23, 2019

Unable to create rds global database in aws using Terraform #7020

Closed

bculberson force-pushed the GH-7212 branch from 9bfa4da to 64ac7f8 Compare January 23, 2019 21:46

hashicorpGH-7212 allow for optional parameters on cluster with global…

eccd2ec

… rds

bculberson force-pushed the GH-7212 branch from 64ac7f8 to eccd2ec Compare January 23, 2019 21:47

ghost added size/S Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. labels Jan 23, 2019

ghost added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Feb 11, 2019

bculberson force-pushed the GH-7212 branch 2 times, most recently from 322fc8d to 116a681 Compare February 12, 2019 01:09

ghost added size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Feb 12, 2019

bculberson force-pushed the GH-7212 branch 6 times, most recently from 39872e7 to 8fe010b Compare February 12, 2019 15:20

add test coverage

9b7a9b0

bculberson force-pushed the GH-7212 branch from 8fe010b to 9b7a9b0 Compare February 12, 2019 15:44

bflad added the bug Addresses a defect in current functionality. label Feb 22, 2019

bflad approved these changes Feb 22, 2019

View reviewed changes

bflad added this to the v1.60.0 milestone Feb 22, 2019

bflad merged commit 9b7a9b0 into hashicorp:master Feb 22, 2019

bflad added a commit that referenced this pull request Feb 22, 2019

Revert 9b7a9b0

8ebb174

See also: #7213 (comment)

bflad added a commit that referenced this pull request Feb 22, 2019

Update CHANGELOG for #7213

fb3524b

ghost locked and limited conversation to collaborators Nov 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing global rds by allowing for optional parameters on cluster #7213

Fixing global rds by allowing for optional parameters on cluster #7213

bculberson commented Jan 19, 2019 •

edited

Loading

adamscharf commented Feb 15, 2019 •

edited

Loading

bflad left a comment

bflad Feb 22, 2019

bflad Feb 22, 2019

bflad Feb 22, 2019

bflad Feb 22, 2019

bflad commented Feb 22, 2019

typez commented Oct 3, 2019 •

edited

Loading

diprochatterjee commented Nov 1, 2019

ghost commented Nov 1, 2019

Fixing global rds by allowing for optional parameters on cluster #7213

Fixing global rds by allowing for optional parameters on cluster #7213

Conversation

bculberson commented Jan 19, 2019 • edited Loading

adamscharf commented Feb 15, 2019 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

bflad Feb 22, 2019

Choose a reason for hiding this comment

bflad Feb 22, 2019

Choose a reason for hiding this comment

bflad Feb 22, 2019

Choose a reason for hiding this comment

bflad Feb 22, 2019

Choose a reason for hiding this comment

bflad commented Feb 22, 2019

typez commented Oct 3, 2019 • edited Loading

diprochatterjee commented Nov 1, 2019

ghost commented Nov 1, 2019

bculberson commented Jan 19, 2019 •

edited

Loading

adamscharf commented Feb 15, 2019 •

edited

Loading

typez commented Oct 3, 2019 •

edited

Loading