-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
resource/aws_elasticache_parameter_group: Handle API reset issues with reserved-memory parameter #6752
resource/aws_elasticache_parameter_group: Handle API reset issues with reserved-memory parameter #6752
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,6 +188,100 @@ 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. | ||
|
||
// 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" { | ||
break | ||
} | ||
|
||
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove this bit. I only see it becoming false on line 230 and we break there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That |
||
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) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a log here or I'm even tempted to error out if this value has changed while using this family
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a warning log in there 👍