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

Jump to bottom

resource/aws_elasticache_replication_group: Must be in available state before modifying tags #24021

Merged
merged 3 commits into from
Apr 7, 2022
Merged

resource/aws_elasticache_replication_group: Must be in available state before modifying tags #24021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86bd0e7
ElastiCache replication group now waits for modification before updat…
gdavison Apr 4, 2022
0687c04
Adds tests to ElastiCache cluster for waiting before updating tags
gdavison Apr 4, 2022
0de7505
Adds CHANGELOG entry
gdavison Apr 4, 2022
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
3 changes: 3 additions & 0 deletions .changelog/24021.txt
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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