Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/rds_cluster: update delete timeout and add additional retry condition #14420

Merged
merged 2 commits into from
Jul 31, 2020
Merged

resource/rds_cluster: update delete timeout and add additional retry condition #14420

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5cb638d
use new provider incantation
anGie44 Jul 31, 2020
54497cc
update test configs with resource refs
anGie44 Jul 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion aws/resource_aws_rds_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
const (
rdsClusterScalingConfiguration_DefaultMinCapacity = 1
rdsClusterScalingConfiguration_DefaultMaxCapacity = 16
rdsClusterTimeoutDelete = 2 * time.Minute
)

func resourceAwsRDSCluster() *schema.Resource {
Expand Down Expand Up @@ -1283,12 +1284,15 @@ func resourceAwsRDSClusterDelete(d *schema.ResourceData, meta interface{}) error

log.Printf("[DEBUG] RDS Cluster delete options: %s", deleteOpts)

err := resource.Retry(1*time.Minute, func() *resource.RetryError {
err := resource.Retry(rdsClusterTimeoutDelete, func() *resource.RetryError {
_, err := conn.DeleteDBCluster(&deleteOpts)
if err != nil {
if isAWSErr(err, rds.ErrCodeInvalidDBClusterStateFault, "is not currently in the available state") {
return resource.RetryableError(err)
}
if isAWSErr(err, rds.ErrCodeInvalidDBClusterStateFault, "cluster is a part of a global cluster") {
return resource.RetryableError(err)
}
if isAWSErr(err, rds.ErrCodeDBClusterNotFoundFault, "") {
return nil
}
Expand Down
125 changes: 125 additions & 0 deletions aws/resource_aws_rds_cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2181,6 +2181,34 @@ func TestAccAWSRDSCluster_EnableHttpEndpoint(t *testing.T) {
})
}

// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/13126
func TestAccAWSRDSCluster_MultipleClustersInGlobalCluster(t *testing.T) {
var providers []*schema.Provider
var primaryDbCluster, secondaryDbCluster rds.DBCluster

rNameGlobal := acctest.RandomWithPrefix("tf-acc-test-global")
rNamePrimary := acctest.RandomWithPrefix("tf-acc-test-primary")
rNameSecondary := acctest.RandomWithPrefix("tf-acc-test-secondary")

resourceNamePrimary := "aws_rds_cluster.primary"
resourceNameSecondary := "aws_rds_cluster.secondary"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: testAccProviderFactories(&providers),
CheckDestroy: testAccCheckAWSClusterDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSRDSClusterConfig_MultipleClustersInGlobalCluster(rNameGlobal, rNamePrimary, rNameSecondary),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSClusterExistsWithProvider(resourceNamePrimary, &primaryDbCluster, testAccAwsRegionProviderFunc(testAccGetRegion(), &providers)),
testAccCheckAWSClusterExistsWithProvider(resourceNameSecondary, &secondaryDbCluster, testAccAwsRegionProviderFunc(testAccGetAlternateRegion(), &providers)),
),
},
},
})
}

func testAccAWSClusterConfig(rName string) string {
return fmt.Sprintf(`
resource "aws_rds_cluster" "test" {
Expand Down Expand Up @@ -3518,3 +3546,100 @@ resource "aws_rds_cluster" "test" {
}
`, rName, enableHttpEndpoint)
}

func testAccAWSRDSClusterConfig_MultipleClustersInGlobalCluster(rNameGlobal, rNamePrimary, rNameSecondary string) string {
return composeConfig(
testAccMultipleRegionProviderConfig(2),
fmt.Sprintf(`
data "aws_region" "current" {}

data "aws_availability_zones" "alternate" {
provider = "awsalternate"
state = "available"

filter {
name = "opt-in-status"
values = ["opt-in-not-required"]
}
}

resource "aws_rds_global_cluster" "test" {
global_cluster_identifier = "%[1]s"
engine = "aurora-mysql"
engine_version = "5.7.mysql_aurora.2.07.1"
}

resource "aws_rds_cluster" "primary" {
cluster_identifier = "%[2]s"
database_name = "mydb"
master_username = "foo"
master_password = "barbarbar"
skip_final_snapshot = true
global_cluster_identifier = aws_rds_global_cluster.test.id
engine = "aurora-mysql"
engine_version = "5.7.mysql_aurora.2.07.1"
}

resource "aws_rds_cluster_instance" "primary" {
identifier = "%[2]s"
cluster_identifier = aws_rds_cluster.primary.id
instance_class = "db.r4.large" # only db.r4 or db.r5 are valid for Aurora global db
engine = "aurora-mysql"
engine_version = "5.7.mysql_aurora.2.07.1"
}

resource "aws_vpc" "alternate" {
provider = "awsalternate"
cidr_block = "10.0.0.0/16"

tags = {
Name = "%[3]s"
}
}

resource "aws_subnet" "alternate" {
provider = "awsalternate"
count = 3
vpc_id = aws_vpc.alternate.id
availability_zone = data.aws_availability_zones.alternate.names[count.index]
cidr_block = "10.0.${count.index}.0/24"

tags = {
Name = "%[3]s"
}
}

resource "aws_db_subnet_group" "alternate" {
provider = "awsalternate"
name = "%[3]s"
subnet_ids = aws_subnet.alternate[*].id
}

resource "aws_rds_cluster" "secondary" {
provider = "awsalternate"
cluster_identifier = "%[3]s"
db_subnet_group_name = aws_db_subnet_group.alternate.name
skip_final_snapshot = true
source_region = data.aws_region.current.name
global_cluster_identifier = aws_rds_global_cluster.test.id
engine = "aurora-mysql"
engine_version = "5.7.mysql_aurora.2.07.1"
depends_on = [aws_rds_cluster_instance.primary]

lifecycle {
ignore_changes = [
replication_source_identifier,
]
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about this, is the API automatically adding it? 😄 If so, we may want to mark it as Computed: true in a separate change.

Copy link
Contributor Author

@anGie44 anGie44 Jul 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is in this case -- the rds_cluster that get's the Secondary role get's this field populated with the identifier/ARN of the Primary cluster. similarly, if you have 1 global cluster , 1 primary cluster , and 1 regional cluster (reading from the primary cluster e.g. the "secondary" cluster here would be configured without global_cluster_identifier but with replication_source_identifier of the "primary" rds_cluster), the API will send back the global_cluster_identifier also creating the non-empty plan

Copy link
Contributor Author

@anGie44 anGie44 Jul 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the original issue's description, i didn't see a workaround to this behavior so i thought maybe the test configuration is a little off or it was just omitted for brevity? took me a bit of experimenting to get the right combination of primary/secondary rds_clusters 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! For what its worth, I think we're going to mark global_cluster_identifier as Computed: true anyways for #10965

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, that issue def looks similar to what i ran into here

Copy link
Contributor Author

@anGie44 anGie44 Jul 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah looks like there's already an issue related to this param: #10150 .. not sure about the timeout though

}

resource "aws_rds_cluster_instance" "secondary" {
provider = "awsalternate"
identifier = "%[3]s"
cluster_identifier = aws_rds_cluster.secondary.id
instance_class = "db.r4.large" # only db.r4 or db.r5 are valid for Aurora global db
engine = "aurora-mysql"
engine_version = "5.7.mysql_aurora.2.07.1"
}
`, rNameGlobal, rNamePrimary, rNameSecondary))
}