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.

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

Allow adding member Replication Group to ElastiCache Global Replication Group #17725

Merged
merged 10 commits into from
Mar 11, 2021
6 changes: 6 additions & 0 deletions .semgrep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ rules:
var $CAST *resource.NotFoundError
...
errors.As($ERR, &$CAST)
- pattern-not-inside: |
var $CAST *resource.NotFoundError
...
errors.As($ERR, &$CAST)
...
$CAST.$FIELD
- patterns:
- pattern: |
$X, $Y := $ERR.(*resource.NotFoundError)
Expand Down
5 changes: 3 additions & 2 deletions aws/data_source_aws_elasticache_replication_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ func dataSourceAwsElasticacheReplicationGroup() *schema.Resource {
Read: dataSourceAwsElasticacheReplicationGroupRead,
Schema: map[string]*schema.Schema{
"replication_group_id": {
Type: schema.TypeString,
Required: true,
Type: schema.TypeString,
Required: true,
ValidateFunc: validateReplicationGroupID,
},
"replication_group_description": {
Type: schema.TypeString,
Expand Down
8 changes: 4 additions & 4 deletions aws/internal/service/elasticache/finder/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func ReplicationGroupByID(conn *elasticache.ElastiCache, id string) (*elasticach

if output == nil || len(output.ReplicationGroups) == 0 || output.ReplicationGroups[0] == nil {
return nil, &resource.NotFoundError{
Message: "Empty result",
Message: "empty result",
LastRequest: input,
}
}
Expand All @@ -48,7 +48,7 @@ func ReplicationGroupMemberClustersByID(conn *elasticache.ElastiCache, id string
}
if len(clusters) == 0 {
return clusters, &resource.NotFoundError{
Message: "No Member Clusters found",
Message: fmt.Sprintf("No Member Clusters found in Replication Group (%s)", id),
}
}

Expand Down Expand Up @@ -87,7 +87,7 @@ func CacheCluster(conn *elasticache.ElastiCache, input *elasticache.DescribeCach

if result == nil || len(result.CacheClusters) == 0 || result.CacheClusters[0] == nil {
return nil, &resource.NotFoundError{
Message: "Empty result",
Message: "empty result",
LastRequest: input,
}
}
Expand Down Expand Up @@ -176,6 +176,6 @@ func GlobalReplicationGroupMemberByID(conn *elasticache.ElastiCache, globalRepli
}

return nil, &resource.NotFoundError{
Message: fmt.Sprintf("Replication Group %q not found in Global Replication Group %q", id, globalReplicationGroupID),
Message: fmt.Sprintf("Replication Group (%s) not found in Global Replication Group (%s)", id, globalReplicationGroupID),
}
}
19 changes: 19 additions & 0 deletions aws/internal/service/elasticache/waiter/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,22 @@ func GlobalReplicationGroupStatus(conn *elasticache.ElastiCache, globalReplicati
return grg, aws.StringValue(grg.Status), nil
}
}

const (
GlobalReplicationGroupMemberStatusAssociated = "associated"
)

// GlobalReplicationGroupStatus fetches the Global Replication Group and its Status
func GlobalReplicationGroupMemberStatus(conn *elasticache.ElastiCache, globalReplicationGroupID, id string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
member, err := finder.GlobalReplicationGroupMemberByID(conn, globalReplicationGroupID, id)
if tfresource.NotFound(err) {
return nil, "", nil
}
if err != nil {
return nil, "", err
}

return member, aws.StringValue(member.Status), nil
}
}
28 changes: 28 additions & 0 deletions aws/internal/service/elasticache/waiter/waiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,31 @@ func GlobalReplicationGroupDeleted(conn *elasticache.ElastiCache, globalReplicat
}
return nil, err
}

const (
GlobalReplicationGroupDisassociationRetryTimeout = 45 * time.Minute

globalReplicationGroupDisassociationTimeout = 20 * time.Minute

globalReplicationGroupDisassociationMinTimeout = 10 * time.Second
globalReplicationGroupDisassociationDelay = 30 * time.Second
Comment on lines +204 to +209
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
GlobalReplicationGroupDisassociationRetryTimeout = 45 * time.Minute
globalReplicationGroupDisassociationTimeout = 20 * time.Minute
globalReplicationGroupDisassociationMinTimeout = 10 * time.Second
globalReplicationGroupDisassociationDelay = 30 * time.Second
GlobalReplicationGroupDisassociationRetryTimeout = 45 * time.Minute
GlobalReplicationGroupDisassociationTimeout = 20 * time.Minute
GlobalReplicationGroupDisassociationMinTimeout = 10 * time.Second
GlobalReplicationGroupDisassociationDelay = 30 * time.Second

Small nit: I see no downside in exporting all the constants. With this naming scheme, collisions are unlikely.

)

func GlobalReplicationGroupMemberDetached(conn *elasticache.ElastiCache, globalReplicationGroupID, id string) (*elasticache.GlobalReplicationGroupMember, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{
GlobalReplicationGroupMemberStatusAssociated,
},
Target: []string{},
Refresh: GlobalReplicationGroupMemberStatus(conn, globalReplicationGroupID, id),
Timeout: globalReplicationGroupDisassociationTimeout,
MinTimeout: globalReplicationGroupDisassociationMinTimeout,
Delay: globalReplicationGroupDisassociationDelay,
Comment on lines +219 to +221
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Timeout: globalReplicationGroupDisassociationTimeout,
MinTimeout: globalReplicationGroupDisassociationMinTimeout,
Delay: globalReplicationGroupDisassociationDelay,
Timeout: GlobalReplicationGroupDisassociationTimeout,
MinTimeout: GlobalReplicationGroupDisassociationMinTimeout,
Delay: GlobalReplicationGroupDisassociationDelay,

}

outputRaw, err := stateConf.WaitForState()
if v, ok := outputRaw.(*elasticache.GlobalReplicationGroupMember); ok {
return v, err
}
return nil, err
}
7 changes: 4 additions & 3 deletions aws/resource_aws_elasticache_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,10 @@ func resourceAwsElasticacheCluster() *schema.Resource {
Elem: &schema.Schema{Type: schema.TypeString},
},
"replication_group_id": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validateReplicationGroupID,
ConflictsWith: []string{
"az_mode",
"engine_version",
Expand Down
7 changes: 4 additions & 3 deletions aws/resource_aws_elasticache_global_replication_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ func resourceAwsElasticacheGlobalReplicationGroup() *schema.Resource {
// },
// },
"primary_replication_group_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateReplicationGroupID,
},
"transit_encryption_enabled": {
Type: schema.TypeBool,
Expand Down
175 changes: 174 additions & 1 deletion aws/resource_aws_elasticache_global_replication_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/elasticache/finder"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/elasticache/waiter"
Expand Down Expand Up @@ -170,6 +171,60 @@ func TestAccAWSElasticacheGlobalReplicationGroup_disappears(t *testing.T) {
})
}

func TestAccAWSElasticacheGlobalReplicationGroup_MultipleSecondaries(t *testing.T) {
var providers []*schema.Provider
var globalReplcationGroup elasticache.GlobalReplicationGroup
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_elasticache_global_replication_group.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
testAccPreCheck(t)
testAccMultipleRegionPreCheck(t, 3)
},
ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 3),
CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSElasticacheGlobalReplicationGroupConfig_MultipleSecondaries(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName, &globalReplcationGroup),
),
},
},
})
}

func TestAccAWSElasticacheGlobalReplicationGroup_ReplaceSecondary_DifferentRegion(t *testing.T) {
var providers []*schema.Provider
var globalReplcationGroup elasticache.GlobalReplicationGroup
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_elasticache_global_replication_group.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
testAccPreCheck(t)
testAccMultipleRegionPreCheck(t, 3)
},
ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 3),
CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Setup(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName, &globalReplcationGroup),
),
},
{
Config: testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Move(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName, &globalReplcationGroup),
),
},
},
})
}

func testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName string, v *elasticache.GlobalReplicationGroup) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[resourceName]
Expand All @@ -187,7 +242,7 @@ func testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName string,
return fmt.Errorf("error retrieving ElastiCache Global Replication Group (%s): %w", rs.Primary.ID, err)
}

if aws.StringValue(grg.Status) != waiter.GlobalReplicationGroupStatusAvailable && aws.StringValue(grg.Status) != waiter.GlobalReplicationGroupStatusPrimaryOnly {
if aws.StringValue(grg.Status) == waiter.GlobalReplicationGroupStatusDeleting || aws.StringValue(grg.Status) == waiter.GlobalReplicationGroupStatusDeleted {
return fmt.Errorf("ElastiCache Global Replication Group (%s) exists, but is in a non-available state: %s", rs.Primary.ID, aws.StringValue(grg.Status))
}

Expand Down Expand Up @@ -273,3 +328,121 @@ resource "aws_elasticache_replication_group" "test" {
}
`, rName, primaryReplicationGroupId, description)
}

func testAccAWSElasticacheGlobalReplicationGroupConfig_MultipleSecondaries(rName string) string {
return composeConfig(
testAccMultipleRegionProviderConfig(3),
fmt.Sprintf(`
resource "aws_elasticache_global_replication_group" "test" {
provider = aws

global_replication_group_id_suffix = %[1]q
primary_replication_group_id = aws_elasticache_replication_group.primary.id
}

resource "aws_elasticache_replication_group" "primary" {
provider = aws

replication_group_id = "%[1]s-p"
replication_group_description = "primary"

node_type = "cache.m5.large"

engine = "redis"
engine_version = "5.0.6"
number_cache_clusters = 1
}

resource "aws_elasticache_replication_group" "alternate" {
provider = awsalternate

replication_group_id = "%[1]s-a"
replication_group_description = "alternate"
global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id

number_cache_clusters = 1
}

resource "aws_elasticache_replication_group" "third" {
provider = awsthird

replication_group_id = "%[1]s-t"
replication_group_description = "third"
global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id

number_cache_clusters = 1
}
`, rName))
}

func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Setup(rName string) string {
return composeConfig(
testAccMultipleRegionProviderConfig(3),
fmt.Sprintf(`
resource "aws_elasticache_global_replication_group" "test" {
provider = aws

global_replication_group_id_suffix = %[1]q
primary_replication_group_id = aws_elasticache_replication_group.primary.id
}

resource "aws_elasticache_replication_group" "primary" {
provider = aws

replication_group_id = "%[1]s-p"
replication_group_description = "primary"

node_type = "cache.m5.large"

engine = "redis"
engine_version = "5.0.6"
number_cache_clusters = 1
}

resource "aws_elasticache_replication_group" "secondary" {
provider = awsalternate

replication_group_id = "%[1]s-a"
replication_group_description = "alternate"
global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id

number_cache_clusters = 1
}
`, rName))
}

func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Move(rName string) string {
return composeConfig(
testAccMultipleRegionProviderConfig(3),
fmt.Sprintf(`
resource "aws_elasticache_global_replication_group" "test" {
provider = aws

global_replication_group_id_suffix = %[1]q
primary_replication_group_id = aws_elasticache_replication_group.primary.id
}

resource "aws_elasticache_replication_group" "primary" {
provider = aws

replication_group_id = "%[1]s-p"
replication_group_description = "primary"

node_type = "cache.m5.large"

engine = "redis"
engine_version = "5.0.6"
number_cache_clusters = 1
}

resource "aws_elasticache_replication_group" "third" {
provider = awsthird

replication_group_id = "%[1]s-t"
replication_group_description = "third"
global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id

number_cache_clusters = 1
}
`, rName))
}
Loading