diff --git a/aws/resource_aws_elasticache_parameter_group.go b/aws/resource_aws_elasticache_parameter_group.go index 3b7b2f4caf9e..e8b57cf6bec5 100644 --- a/aws/resource_aws_elasticache_parameter_group.go +++ b/aws/resource_aws_elasticache_parameter_group.go @@ -188,6 +188,101 @@ func resourceAwsElasticacheParameterGroupUpdate(d *schema.ResourceData, meta int } return nil }) + + // When attempting to reset the reserved-memory parameter, the API + // can return the below 500 error, which causes the AWS Go SDK to + // automatically retry and hence timeout resource.Retry(): + // InternalFailure: An internal error has occurred. Please try your query again at a later time. + // Instead of hardcoding the reserved-memory parameter removal + // above, which may become out of date, here we add logic to + // workaround this API behavior + + if isResourceTimeoutError(err) { + for i, paramToModify := range paramsToModify { + if aws.StringValue(paramToModify.ParameterName) != "reserved-memory" { + continue + } + + // Always reset the top level error and remove the reset for reserved-memory + err = nil + paramsToModify = append(paramsToModify[:i], paramsToModify[i+1:]...) + + // If we are only trying to remove reserved-memory and not perform + // an update to reserved-memory or reserved-memory-percentage, we + // can attempt to workaround the API issue by switching it to + // reserved-memory-percentage first then reset that temporary parameter. + + tryReservedMemoryPercentageWorkaround := true + + allConfiguredParameters, err := expandElastiCacheParameters(d.Get("parameter").(*schema.Set).List()) + if err != nil { + return fmt.Errorf("error expanding parameter configuration: %s", err) + } + + for _, configuredParameter := range allConfiguredParameters { + if aws.StringValue(configuredParameter.ParameterName) == "reserved-memory" || aws.StringValue(configuredParameter.ParameterName) == "reserved-memory-percentage" { + tryReservedMemoryPercentageWorkaround = false + break + } + } + + if !tryReservedMemoryPercentageWorkaround { + break + } + + // The reserved-memory-percentage parameter does not exist in redis2.6 and redis2.8 + family := d.Get("family").(string) + if family == "redis2.6" || family == "redis2.8" { + log.Printf("[WARN] Cannot reset Elasticache Parameter Group (%s) reserved-memory parameter with %s family", d.Id(), family) + break + } + + modifyInput := &elasticache.ModifyCacheParameterGroupInput{ + CacheParameterGroupName: aws.String(d.Get("name").(string)), + ParameterNameValues: []*elasticache.ParameterNameValue{ + { + ParameterName: aws.String("reserved-memory-percentage"), + ParameterValue: aws.String("0"), + }, + }, + } + _, err = conn.ModifyCacheParameterGroup(modifyInput) + + if err != nil { + log.Printf("[WARN] Error attempting reserved-memory workaround to switch to reserved-memory-percentage: %s", err) + break + } + + resetInput := &elasticache.ResetCacheParameterGroupInput{ + CacheParameterGroupName: aws.String(d.Get("name").(string)), + ParameterNameValues: []*elasticache.ParameterNameValue{ + { + ParameterName: aws.String("reserved-memory-percentage"), + ParameterValue: aws.String("0"), + }, + }, + } + + _, err = conn.ResetCacheParameterGroup(resetInput) + + if err != nil { + log.Printf("[WARN] Error attempting reserved-memory workaround to reset reserved-memory-percentage: %s", err) + } + + break + } + + // Retry any remaining parameter resets with reserved-memory potentially removed + if len(paramsToModify) > 0 { + resetOpts = elasticache.ResetCacheParameterGroupInput{ + CacheParameterGroupName: aws.String(d.Get("name").(string)), + ParameterNameValues: paramsToModify, + } + // Reset top level error with potentially any new errors + _, err = conn.ResetCacheParameterGroup(&resetOpts) + } + } + if err != nil { return fmt.Errorf("Error resetting Cache Parameter Group: %s", err) } diff --git a/aws/resource_aws_elasticache_parameter_group_test.go b/aws/resource_aws_elasticache_parameter_group_test.go index e3c992bea786..d71b326ca3a0 100644 --- a/aws/resource_aws_elasticache_parameter_group_test.go +++ b/aws/resource_aws_elasticache_parameter_group_test.go @@ -12,7 +12,8 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestAccAWSElasticacheParameterGroup_importBasic(t *testing.T) { +func TestAccAWSElasticacheParameterGroup_basic(t *testing.T) { + var v elasticache.CacheParameterGroup resourceName := "aws_elasticache_parameter_group.bar" rName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) @@ -23,8 +24,14 @@ func TestAccAWSElasticacheParameterGroup_importBasic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccAWSElasticacheParameterGroupConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &v), + testAccCheckAWSElasticacheParameterGroupAttributes(&v, rName), + resource.TestCheckResourceAttr(resourceName, "description", "Managed by Terraform"), + resource.TestCheckResourceAttr(resourceName, "family", "redis2.8"), + resource.TestCheckResourceAttr(resourceName, "name", rName), + ), }, - { ResourceName: resourceName, ImportState: true, @@ -34,7 +41,7 @@ func TestAccAWSElasticacheParameterGroup_importBasic(t *testing.T) { }) } -func TestAccAWSElasticacheParameterGroup_basic(t *testing.T) { +func TestAccAWSElasticacheParameterGroup_addParameter(t *testing.T) { var v elasticache.CacheParameterGroup rName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) @@ -44,16 +51,10 @@ func TestAccAWSElasticacheParameterGroup_basic(t *testing.T) { CheckDestroy: testAccCheckAWSElasticacheParameterGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSElasticacheParameterGroupConfig(rName), + Config: testAccAWSElasticacheParameterGroupConfigParameter1(rName, "redis2.8", "appendonly", "yes"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheParameterGroupExists("aws_elasticache_parameter_group.bar", &v), - testAccCheckAWSElasticacheParameterGroupAttributes(&v, rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "name", rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "family", "redis2.8"), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "description", "Managed by Terraform"), + resource.TestCheckResourceAttr("aws_elasticache_parameter_group.bar", "parameter.#", "1"), resource.TestCheckResourceAttr( "aws_elasticache_parameter_group.bar", "parameter.283487565.name", "appendonly"), resource.TestCheckResourceAttr( @@ -61,16 +62,15 @@ func TestAccAWSElasticacheParameterGroup_basic(t *testing.T) { ), }, { - Config: testAccAWSElasticacheParameterGroupAddParametersConfig(rName), + ResourceName: "aws_elasticache_parameter_group.bar", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSElasticacheParameterGroupConfigParameter2(rName, "redis2.8", "appendonly", "yes", "appendfsync", "always"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheParameterGroupExists("aws_elasticache_parameter_group.bar", &v), - testAccCheckAWSElasticacheParameterGroupAttributes(&v, rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "name", rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "family", "redis2.8"), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "description", "Test parameter group for terraform"), + resource.TestCheckResourceAttr("aws_elasticache_parameter_group.bar", "parameter.#", "2"), resource.TestCheckResourceAttr( "aws_elasticache_parameter_group.bar", "parameter.283487565.name", "appendonly"), resource.TestCheckResourceAttr( @@ -85,7 +85,8 @@ func TestAccAWSElasticacheParameterGroup_basic(t *testing.T) { }) } -func TestAccAWSElasticacheParameterGroup_only(t *testing.T) { +// Regression for https://github.com/terraform-providers/terraform-provider-aws/issues/116 +func TestAccAWSElasticacheParameterGroup_removeAllParameters(t *testing.T) { var v elasticache.CacheParameterGroup rName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) @@ -95,23 +96,28 @@ func TestAccAWSElasticacheParameterGroup_only(t *testing.T) { CheckDestroy: testAccCheckAWSElasticacheParameterGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSElasticacheParameterGroupOnlyConfig(rName), + Config: testAccAWSElasticacheParameterGroupConfigParameter2(rName, "redis2.8", "appendonly", "yes", "appendfsync", "always"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheParameterGroupExists("aws_elasticache_parameter_group.bar", &v), - testAccCheckAWSElasticacheParameterGroupAttributes(&v, rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "name", rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "family", "redis2.8"), + resource.TestCheckResourceAttr("aws_elasticache_parameter_group.bar", "parameter.#", "2"), + ), + }, + { + Config: testAccAWSElasticacheParameterGroupConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheParameterGroupExists("aws_elasticache_parameter_group.bar", &v), + resource.TestCheckResourceAttr("aws_elasticache_parameter_group.bar", "parameter.#", "0"), ), }, }, }) } -// Regression for https://github.com/terraform-providers/terraform-provider-aws/issues/116 -func TestAccAWSElasticacheParameterGroup_removeParam(t *testing.T) { - var v elasticache.CacheParameterGroup +// The API throws 500 errors when attempting to reset the reserved-memory parameter. +// This covers our custom logic handling for this situation. +func TestAccAWSElasticacheParameterGroup_removeReservedMemoryParameter(t *testing.T) { + var cacheParameterGroup1 elasticache.CacheParameterGroup + resourceName := "aws_elasticache_parameter_group.bar" rName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) resource.ParallelTest(t, resource.TestCase{ @@ -120,27 +126,94 @@ func TestAccAWSElasticacheParameterGroup_removeParam(t *testing.T) { CheckDestroy: testAccCheckAWSElasticacheParameterGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSElasticacheParameterGroupAddParametersConfig(rName), + Config: testAccAWSElasticacheParameterGroupConfigParameter1(rName, "redis3.2", "reserved-memory", "0"), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSElasticacheParameterGroupExists("aws_elasticache_parameter_group.bar", &v), - testAccCheckAWSElasticacheParameterGroupAttributes(&v, rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "name", rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "family", "redis2.8"), + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &cacheParameterGroup1), + resource.TestCheckResourceAttr(resourceName, "parameter.#", "1"), ), }, { - Config: testAccAWSElasticacheParameterGroupOnlyConfig(rName), + Config: testAccAWSElasticacheParameterGroupConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSElasticacheParameterGroupExists("aws_elasticache_parameter_group.bar", &v), - testAccCheckAWSElasticacheParameterGroupAttributes(&v, rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "name", rName), - resource.TestCheckResourceAttr( - "aws_elasticache_parameter_group.bar", "family", "redis2.8"), + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &cacheParameterGroup1), + resource.TestCheckResourceAttr(resourceName, "parameter.#", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +// The API throws 500 errors when attempting to reset the reserved-memory parameter. +// This covers our custom logic handling for this situation. +func TestAccAWSElasticacheParameterGroup_switchReservedMemoryParameter(t *testing.T) { + var cacheParameterGroup1 elasticache.CacheParameterGroup + resourceName := "aws_elasticache_parameter_group.bar" + rName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSElasticacheParameterGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheParameterGroupConfigParameter1(rName, "redis3.2", "reserved-memory", "0"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &cacheParameterGroup1), + resource.TestCheckResourceAttr(resourceName, "parameter.#", "1"), + ), + }, + { + Config: testAccAWSElasticacheParameterGroupConfigParameter1(rName, "redis3.2", "reserved-memory-percent", "25"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &cacheParameterGroup1), + resource.TestCheckResourceAttr(resourceName, "parameter.#", "1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +// The API throws 500 errors when attempting to reset the reserved-memory parameter. +// This covers our custom logic handling for this situation. +func TestAccAWSElasticacheParameterGroup_updateReservedMemoryParameter(t *testing.T) { + var cacheParameterGroup1 elasticache.CacheParameterGroup + resourceName := "aws_elasticache_parameter_group.bar" + rName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSElasticacheParameterGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheParameterGroupConfigParameter1(rName, "redis2.8", "reserved-memory", "0"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &cacheParameterGroup1), + resource.TestCheckResourceAttr(resourceName, "parameter.#", "1"), + ), + }, + { + Config: testAccAWSElasticacheParameterGroupConfigParameter1(rName, "redis2.8", "reserved-memory", "1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &cacheParameterGroup1), + resource.TestCheckResourceAttr(resourceName, "parameter.#", "1"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -156,13 +229,44 @@ func TestAccAWSElasticacheParameterGroup_UppercaseName(t *testing.T) { CheckDestroy: testAccCheckAWSElasticacheParameterGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSElasticacheParameterGroupConfig_UppercaseName(rName), + Config: testAccAWSElasticacheParameterGroupConfigParameter1(rName, "redis2.8", "appendonly", "yes"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheParameterGroupExists("aws_elasticache_parameter_group.bar", &v), resource.TestCheckResourceAttr( "aws_elasticache_parameter_group.bar", "name", fmt.Sprintf("tf-elastipg-%d", rInt)), ), }, + { + ResourceName: "aws_elasticache_parameter_group.bar", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAWSElasticacheParameterGroup_Description(t *testing.T) { + var v elasticache.CacheParameterGroup + resourceName := "aws_elasticache_parameter_group.bar" + rName := fmt.Sprintf("parameter-group-test-terraform-%d", acctest.RandInt()) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSElasticacheParameterGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheParameterGroupConfigDescription(rName, "description1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheParameterGroupExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "description", "description1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -253,49 +357,47 @@ func testAccCheckAWSElasticacheParameterGroupExists(n string, v *elasticache.Cac func testAccAWSElasticacheParameterGroupConfig(rName string) string { return fmt.Sprintf(` resource "aws_elasticache_parameter_group" "bar" { - name = "%s" - family = "redis2.8" - parameter { - name = "appendonly" - value = "yes" - } + family = "redis2.8" + name = %q }`, rName) } -func testAccAWSElasticacheParameterGroupAddParametersConfig(rName string) string { +func testAccAWSElasticacheParameterGroupConfigDescription(rName, description string) string { return fmt.Sprintf(` resource "aws_elasticache_parameter_group" "bar" { - name = "%s" - family = "redis2.8" - description = "Test parameter group for terraform" - parameter { - name = "appendonly" - value = "yes" - } - parameter { - name = "appendfsync" - value = "always" - } -}`, rName) + description = %q + family = "redis2.8" + name = %q +}`, description, rName) } -func testAccAWSElasticacheParameterGroupOnlyConfig(rName string) string { +func testAccAWSElasticacheParameterGroupConfigParameter1(rName, family, parameterName1, parameterValue1 string) string { return fmt.Sprintf(` resource "aws_elasticache_parameter_group" "bar" { - name = "%s" - family = "redis2.8" - description = "Test parameter group for terraform" -}`, rName) + family = %q + name = %q + + parameter { + name = %q + value = %q + } +}`, family, rName, parameterName1, parameterValue1) } -func testAccAWSElasticacheParameterGroupConfig_UppercaseName(rName string) string { +func testAccAWSElasticacheParameterGroupConfigParameter2(rName, family, parameterName1, parameterValue1, parameterName2, parameterValue2 string) string { return fmt.Sprintf(` resource "aws_elasticache_parameter_group" "bar" { - name = "%s" - family = "redis2.8" + family = %q + name = %q + parameter { - name = "appendonly" - value = "yes" + name = %q + value = %q } -}`, rName) + + parameter { + name = %q + value = %q + } +}`, family, rName, parameterName1, parameterValue1, parameterName2, parameterValue2) } diff --git a/website/docs/r/elasticache_parameter_group.html.markdown b/website/docs/r/elasticache_parameter_group.html.markdown index 826c49676c8b..d688730d62f1 100644 --- a/website/docs/r/elasticache_parameter_group.html.markdown +++ b/website/docs/r/elasticache_parameter_group.html.markdown @@ -10,6 +10,8 @@ description: |- Provides an ElastiCache parameter group resource. +~> **NOTE:** Attempting to remove the `reserved-memory` parameter when `family` is set to `redis2.6` or `redis2.8` may show a perpetual difference in Terraform due to an Elasticache API limitation. Leave that parameter configured with any value to workaround the issue. + ## Example Usage ```hcl