Skip to content

Commit

Permalink
Merge pull request #24021 from hashicorp/repgroup-tag-needs-available
Browse files Browse the repository at this point in the history 
resource/aws_elasticache_replication_group: Must be in `available` state before modifying tags
  • Loading branch information
@gdavison
gdavison authored Apr 7, 2022
2 parents b23e004 + 0de7505 commit 831b4ed
Show file tree
Hide file tree
Showing 4 changed files with 224 additions and 11 deletions.
3 changes: 3 additions & 0 deletions .changelog/24021.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_elasticache_replication_group: Waits for available state before updating tags
```
144 changes: 144 additions & 0 deletions internal/service/elasticache/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,96 @@ func TestAccElastiCacheCluster_Engine_Redis_LogDeliveryConfigurations(t *testing
})
}

func TestAccElastiCacheCluster_tags(t *testing.T) {
var cluster elasticache.CacheCluster
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_elasticache_cluster.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, elasticache.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckClusterDestroy,
Steps: []resource.TestStep{
{
Config: testAccClusterConfigTags1(rName, "key1", "value1"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckClusterExists(resourceName, &cluster),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags_all.key1", "value1"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"apply_immediately"}, //not in the API
},
{
Config: testAccClusterConfigTags2(rName, "key1", "value1updated", "key2", "value2"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckClusterExists(resourceName, &cluster),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags_all.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags_all.key2", "value2"),
),
},
{
Config: testAccClusterConfigTags1(rName, "key2", "value2"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckClusterExists(resourceName, &cluster),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags_all.key2", "value2"),
),
},
},
})
}

func TestAccElastiCacheCluster_tagWithOtherModification(t *testing.T) {
var cluster elasticache.CacheCluster
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_elasticache_cluster.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, elasticache.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckClusterDestroy,
Steps: []resource.TestStep{
{
Config: testAccClusterVersionAndTagConfig(rName, "5.0.4", "key1", "value1"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckClusterExists(resourceName, &cluster),
resource.TestCheckResourceAttr(resourceName, "engine_version", "5.0.4"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags_all.key1", "value1"),
),
},
{
Config: testAccClusterVersionAndTagConfig(rName, "5.0.6", "key1", "value1updated"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckClusterExists(resourceName, &cluster),
resource.TestCheckResourceAttr(resourceName, "engine_version", "5.0.6"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags_all.key1", "value1updated"),
),
},
},
})
}

func testAccCheckClusterAttributes(v *elasticache.CacheCluster) resource.TestCheckFunc {
return func(s *terraform.State) error {
if v.NotificationConfiguration == nil {
Expand Down Expand Up @@ -1646,3 +1736,57 @@ data "aws_elasticache_cluster" "test" {
`, rName, slowLogDeliveryEnabled, slowDeliveryDestination, slowDeliveryFormat, engineLogDeliveryEnabled, engineDeliveryDestination, engineLogDeliveryFormat)

}

func testAccClusterConfigTags1(rName, tag1Key, tag1Value string) string {
return acctest.ConfigCompose(
acctest.ConfigAvailableAZsNoOptIn(),
fmt.Sprintf(`
resource "aws_elasticache_cluster" "test" {
cluster_id = %[1]q
engine = "memcached"
node_type = "cache.t3.small"
num_cache_nodes = 1
tags = {
%[2]q = %[3]q
}
}
`, rName, tag1Key, tag1Value))
}

func testAccClusterConfigTags2(rName, tag1Key, tag1Value, tag2Key, tag2Value string) string {
return acctest.ConfigCompose(
acctest.ConfigAvailableAZsNoOptIn(),
fmt.Sprintf(`
resource "aws_elasticache_cluster" "test" {
cluster_id = %[1]q
engine = "memcached"
node_type = "cache.t3.small"
num_cache_nodes = 1
tags = {
%[2]q = %[3]q
%[4]q = %[5]q
}
}
`, rName, tag1Key, tag1Value, tag2Key, tag2Value))
}

func testAccClusterVersionAndTagConfig(rName, version, tagKey1, tagValue1 string) string {
return acctest.ConfigCompose(
fmt.Sprintf(`
resource "aws_elasticache_cluster" "test" {
cluster_id = %[1]q
node_type = "cache.t3.small"
num_cache_nodes = 1
engine = "redis"
engine_version = %[2]q
apply_immediately = true
tags = {
%[3]q = %[4]q
}
}
`, rName, version, tagKey1, tagValue1),
)
}
21 changes: 13 additions & 8 deletions internal/service/elasticache/replication_group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func ResourceReplicationGroup() *schema.Resource {
Optional: true,
Computed: true,
StateFunc: func(val interface{}) string {
// Elasticache always changes the maintenance to lowercase
// ElastiCache always changes the maintenance to lowercase
return strings.ToLower(val.(string))
},
ValidateFunc: verify.ValidOnceAWeekWindowFormat,
Expand Down Expand Up @@ -684,7 +684,7 @@ func resourceReplicationGroupRead(d *schema.ResourceData, meta interface{}) erro

// tags not supported in all partitions
if err != nil {
log.Printf("[WARN] failed listing tags for Elasticache Replication Group (%s): %s", aws.StringValue(rgp.ARN), err)
log.Printf("[WARN] failed listing tags for ElastiCache Replication Group (%s): %s", aws.StringValue(rgp.ARN), err)
}

if tags != nil {
Expand Down Expand Up @@ -913,6 +913,11 @@ func resourceReplicationGroupUpdate(d *schema.ResourceData, meta interface{}) er
if err != nil {
return fmt.Errorf("error updating ElastiCache Replication Group (%s): %w", d.Id(), err)
}

_, err = WaitReplicationGroupAvailable(conn, d.Id(), d.Timeout(schema.TimeoutUpdate))
if err != nil {
return fmt.Errorf("error waiting for ElastiCache Replication Group (%s) to update: %w", d.Id(), err)
}
}

if d.HasChange("auth_token") {
Expand All @@ -925,7 +930,12 @@ func resourceReplicationGroupUpdate(d *schema.ResourceData, meta interface{}) er

_, err := conn.ModifyReplicationGroup(params)
if err != nil {
return fmt.Errorf("error changing auth_token for Elasticache Replication Group (%s): %w", d.Id(), err)
return fmt.Errorf("error changing auth_token for ElastiCache Replication Group (%s): %w", d.Id(), err)
}

_, err = WaitReplicationGroupAvailable(conn, d.Id(), d.Timeout(schema.TimeoutUpdate))
if err != nil {
return fmt.Errorf("error waiting for ElastiCache Replication Group (%s) auth_token change: %w", d.Id(), err)
}
}

Expand All @@ -944,11 +954,6 @@ func resourceReplicationGroupUpdate(d *schema.ResourceData, meta interface{}) er
}
}

_, err := WaitReplicationGroupAvailable(conn, d.Id(), d.Timeout(schema.TimeoutUpdate))
if err != nil {
return fmt.Errorf("error waiting for modification: %w", err)
}

return resourceReplicationGroupRead(d, meta)
}

Expand Down
67 changes: 64 additions & 3 deletions internal/service/elasticache/replication_group_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1715,7 +1715,7 @@ func TestAccElastiCacheReplicationGroup_tags(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccReplicationGroupTags1Config(rName, "key1", "value1"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckReplicationGroupExists(resourceName, &rg),
testAccReplicationGroupCheckMemberClusterTags(resourceName, clusterDataSourcePrefix, 2, []kvp{
{"key1", "value1"},
Expand All @@ -1730,7 +1730,7 @@ func TestAccElastiCacheReplicationGroup_tags(t *testing.T) {
},
{
Config: testAccReplicationGroupTags2Config(rName, "key1", "value1updated", "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckReplicationGroupExists(resourceName, &rg),
testAccReplicationGroupCheckMemberClusterTags(resourceName, clusterDataSourcePrefix, 2, []kvp{
{"key1", "value1updated"},
Expand All @@ -1740,7 +1740,7 @@ func TestAccElastiCacheReplicationGroup_tags(t *testing.T) {
},
{
Config: testAccReplicationGroupTags1Config(rName, "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckReplicationGroupExists(resourceName, &rg),
testAccReplicationGroupCheckMemberClusterTags(resourceName, clusterDataSourcePrefix, 2, []kvp{
{"key2", "value2"},
Expand All @@ -1751,6 +1751,46 @@ func TestAccElastiCacheReplicationGroup_tags(t *testing.T) {
})
}

func TestAccElastiCacheReplicationGroup_tagWithOtherModification(t *testing.T) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

var rg elasticache.ReplicationGroup
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_elasticache_replication_group.test"
clusterDataSourcePrefix := "data.aws_elasticache_cluster.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, elasticache.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckReplicationDestroy,
Steps: []resource.TestStep{
{
Config: testAccReplicationGroupVersionAndTagConfig(rName, "5.0.4", "key1", "value1"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckReplicationGroupExists(resourceName, &rg),
resource.TestCheckResourceAttr(resourceName, "engine_version", "5.0.4"),
testAccReplicationGroupCheckMemberClusterTags(resourceName, clusterDataSourcePrefix, 2, []kvp{
{"key1", "value1"},
}),
),
},
{
Config: testAccReplicationGroupVersionAndTagConfig(rName, "5.0.6", "key1", "value1updated"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckReplicationGroupExists(resourceName, &rg),
resource.TestCheckResourceAttr(resourceName, "engine_version", "5.0.6"),
testAccReplicationGroupCheckMemberClusterTags(resourceName, clusterDataSourcePrefix, 2, []kvp{
{"key1", "value1updated"},
}),
),
},
},
})
}

func TestAccElastiCacheReplicationGroup_finalSnapshot(t *testing.T) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
Expand Down Expand Up @@ -3112,6 +3152,27 @@ resource "aws_elasticache_replication_group" "test" {
)
}

func testAccReplicationGroupVersionAndTagConfig(rName, version, tagKey1, tagValue1 string) string {
const clusterCount = 2
return acctest.ConfigCompose(
testAccReplicationGroupClusterData(clusterCount),
fmt.Sprintf(`
resource "aws_elasticache_replication_group" "test" {
replication_group_id = %[1]q
replication_group_description = "test description"
node_type = "cache.t3.small"
number_cache_clusters = %[2]d
apply_immediately = true
engine_version = %[3]q
tags = {
%[4]q = %[5]q
}
}
`, rName, clusterCount, version, tagKey1, tagValue1),
)
}

func testAccReplicationGroupClusterData(count int) string {
return fmt.Sprintf(`
data "aws_elasticache_cluster" "test" {
Expand Down

0 comments on commit 831b4ed

Please sign in to comment.