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

Add support for "StepScaling" autoscaling policies. #4277

Merged
merged 3 commits into from
Mar 31, 2016
Merged

Add support for "StepScaling" autoscaling policies. #4277

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
305a450
aws_autoscaling_policy: Add support for StepScaling policies.
ctiwald Dec 11, 2015
2cf6afa
aws_autoscaling_policy: Add tests for StepScaling policies.
ctiwald Dec 11, 2015
f5c898e
aws_autoscaling_policy: Add documentation for StepScaling policies.
ctiwald Dec 11, 2015
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
131 changes: 123 additions & 8 deletions builtin/providers/aws/resource_aws_autoscaling_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package aws

import (
"bytes"
"fmt"
"log"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/autoscaling"
"github.com/hashicorp/terraform/helper/hashcode"
"github.com/hashicorp/terraform/helper/schema"
)

Expand Down Expand Up @@ -35,17 +37,59 @@ func resourceAwsAutoscalingPolicy() *schema.Resource {
Required: true,
ForceNew: true,
},
"policy_type": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Default: "SimpleScaling", // preserve AWS's default to make validation easier.
},
"cooldown": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
},
"min_adjustment_step": &schema.Schema{
"estimated_instance_warmup": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
},
"scaling_adjustment": &schema.Schema{
"metric_aggregation_type": &schema.Schema{
Type: schema.TypeString,
Optional: true,
},
"min_adjustment_magnitude": &schema.Schema{
Type: schema.TypeInt,
Required: true,
Optional: true,
},
"min_adjustment_step": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
Deprecated: "Use min_adjustment_magnitude instead.",
ConflictsWith: []string{"min_adjustment_magnitude"},
},
"scaling_adjustment": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
ConflictsWith: []string{"step_adjustment"},
},
"step_adjustment": &schema.Schema{
Type: schema.TypeSet,
Optional: true,
ConflictsWith: []string{"scaling_adjustment"},
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"metric_interval_lower_bound": &schema.Schema{
Type: schema.TypeString,
Optional: true,
},
"metric_interval_upper_bound": &schema.Schema{
Type: schema.TypeString,
Optional: true,
},
"scaling_adjustment": &schema.Schema{
Type: schema.TypeInt,
Required: true,
},
},
},
Set: resourceAwsAutoscalingScalingAdjustmentHash,
},
},
}
Expand All @@ -54,7 +98,10 @@ func resourceAwsAutoscalingPolicy() *schema.Resource {
func resourceAwsAutoscalingPolicyCreate(d *schema.ResourceData, meta interface{}) error {
autoscalingconn := meta.(*AWSClient).autoscalingconn

params := getAwsAutoscalingPutScalingPolicyInput(d)
params, err := getAwsAutoscalingPutScalingPolicyInput(d)
if err != nil {
return err
}

log.Printf("[DEBUG] AutoScaling PutScalingPolicy: %#v", params)
resp, err := autoscalingconn.PutScalingPolicy(&params)
Expand Down Expand Up @@ -84,18 +131,26 @@ func resourceAwsAutoscalingPolicyRead(d *schema.ResourceData, meta interface{})
d.Set("adjustment_type", p.AdjustmentType)
d.Set("autoscaling_group_name", p.AutoScalingGroupName)
d.Set("cooldown", p.Cooldown)
d.Set("estimated_instance_warmup", p.EstimatedInstanceWarmup)
d.Set("metric_aggregation_type", p.MetricAggregationType)
d.Set("policy_type", p.PolicyType)
d.Set("min_adjustment_magnitude", p.MinAdjustmentMagnitude)
d.Set("min_adjustment_step", p.MinAdjustmentStep)
d.Set("arn", p.PolicyARN)
d.Set("name", p.PolicyName)
d.Set("scaling_adjustment", p.ScalingAdjustment)
d.Set("step_adjustment", flattenStepAdjustments(p.StepAdjustments))

return nil
}

func resourceAwsAutoscalingPolicyUpdate(d *schema.ResourceData, meta interface{}) error {
autoscalingconn := meta.(*AWSClient).autoscalingconn

params := getAwsAutoscalingPutScalingPolicyInput(d)
params, inputErr := getAwsAutoscalingPutScalingPolicyInput(d)
if inputErr != nil {
return inputErr
}

log.Printf("[DEBUG] Autoscaling Update Scaling Policy: %#v", params)
_, err := autoscalingconn.PutScalingPolicy(&params)
Expand Down Expand Up @@ -128,8 +183,10 @@ func resourceAwsAutoscalingPolicyDelete(d *schema.ResourceData, meta interface{}
return nil
}

// PutScalingPolicy seems to require all params to be resent, so create and update can share this common function
func getAwsAutoscalingPutScalingPolicyInput(d *schema.ResourceData) autoscaling.PutScalingPolicyInput {
// PutScalingPolicy can safely resend all parameters without destroying the
// resource, so create and update can share this common function. It will error
// if certain mutually exclusive values are set.
func getAwsAutoscalingPutScalingPolicyInput(d *schema.ResourceData) (autoscaling.PutScalingPolicyInput, error) {
var params = autoscaling.PutScalingPolicyInput{
AutoScalingGroupName: aws.String(d.Get("autoscaling_group_name").(string)),
PolicyName: aws.String(d.Get("name").(string)),
Expand All @@ -143,15 +200,59 @@ func getAwsAutoscalingPutScalingPolicyInput(d *schema.ResourceData) autoscaling.
params.Cooldown = aws.Int64(int64(v.(int)))
}

if v, ok := d.GetOk("estimated_instance_warmup"); ok {
params.EstimatedInstanceWarmup = aws.Int64(int64(v.(int)))
}

if v, ok := d.GetOk("metric_aggregation_type"); ok {
params.MetricAggregationType = aws.String(v.(string))
}

if v, ok := d.GetOk("policy_type"); ok {
params.PolicyType = aws.String(v.(string))
}

if v, ok := d.GetOk("scaling_adjustment"); ok {
params.ScalingAdjustment = aws.Int64(int64(v.(int)))
}

if v, ok := d.GetOk("step_adjustment"); ok {
steps, err := expandStepAdjustments(v.(*schema.Set).List())
if err != nil {
return params, fmt.Errorf("metric_interval_lower_bound and metric_interval_upper_bound must be strings!")
}
params.StepAdjustments = steps
}

if v, ok := d.GetOk("min_adjustment_magnitude"); ok {
params.MinAdjustmentMagnitude = aws.Int64(int64(v.(int)))
}

if v, ok := d.GetOk("min_adjustment_step"); ok {
params.MinAdjustmentStep = aws.Int64(int64(v.(int)))
}

return params
// Validate our final input to confirm it won't error when sent to AWS.
// First, SimpleScaling policy types...
if *params.PolicyType == "SimpleScaling" && params.StepAdjustments != nil {
return params, fmt.Errorf("SimpleScaling policy types cannot use step_adjustments!")
}
if *params.PolicyType == "SimpleScaling" && params.MetricAggregationType != nil {
return params, fmt.Errorf("SimpleScaling policy types cannot use metric_aggregation_type!")
}
if *params.PolicyType == "SimpleScaling" && params.EstimatedInstanceWarmup != nil {
return params, fmt.Errorf("SimpleScaling policy types cannot use estimated_instance_warmup!")
}

// Second, StepScaling policy types...
if *params.PolicyType == "StepScaling" && params.ScalingAdjustment != nil {
return params, fmt.Errorf("StepScaling policy types cannot use scaling_adjustment!")
}
if *params.PolicyType == "StepScaling" && params.Cooldown != nil {
return params, fmt.Errorf("StepScaling policy types cannot use cooldown!")
}

return params, nil
}

func getAwsAutoscalingPolicy(d *schema.ResourceData, meta interface{}) (*autoscaling.ScalingPolicy, error) {
Expand Down Expand Up @@ -179,3 +280,17 @@ func getAwsAutoscalingPolicy(d *schema.ResourceData, meta interface{}) (*autosca
// policy not found
return nil, nil
}

func resourceAwsAutoscalingScalingAdjustmentHash(v interface{}) int {
var buf bytes.Buffer
m := v.(map[string]interface{})
if v, ok := m["metric_interval_lower_bound"]; ok {
buf.WriteString(fmt.Sprintf("%f-", v))
}
if v, ok := m["metric_interval_upper_bound"]; ok {
buf.WriteString(fmt.Sprintf("%f-", v))
}
buf.WriteString(fmt.Sprintf("%d-", m["scaling_adjustment"].(int)))

return hashcode.String(buf.String())
}
77 changes: 51 additions & 26 deletions builtin/providers/aws/resource_aws_autoscaling_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,20 @@ func TestAccAWSAutoscalingPolicy_basic(t *testing.T) {
resource.TestStep{
Config: testAccAWSAutoscalingPolicyConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar", &policy),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar", "adjustment_type", "ChangeInCapacity"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar", "cooldown", "300"),
testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "adjustment_type", "ChangeInCapacity"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "policy_type", "SimpleScaling"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "cooldown", "300"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "name", "foobar_simple"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "scaling_adjustment", "2"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "autoscaling_group_name", "terraform-test-foobar5"),
testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_step", &policy),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "adjustment_type", "ChangeInCapacity"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "policy_type", "StepScaling"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "name", "foobar_step"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "metric_aggregation_type", "Minimum"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "estimated_instance_warmup", "200"),
resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "autoscaling_group_name", "terraform-test-foobar5"),
),
},
},
Expand Down Expand Up @@ -82,33 +93,47 @@ func testAccCheckAWSAutoscalingPolicyDestroy(s *terraform.State) error {

var testAccAWSAutoscalingPolicyConfig = fmt.Sprintf(`
resource "aws_launch_configuration" "foobar" {
name = "terraform-test-foobar5"
image_id = "ami-21f78e11"
instance_type = "t1.micro"
name = "terraform-test-foobar5"
image_id = "ami-21f78e11"
instance_type = "t1.micro"
}

resource "aws_autoscaling_group" "foobar" {
availability_zones = ["us-west-2a"]
name = "terraform-test-foobar5"
max_size = 5
min_size = 2
health_check_grace_period = 300
health_check_type = "ELB"
force_delete = true
termination_policies = ["OldestInstance"]
launch_configuration = "${aws_launch_configuration.foobar.name}"
tag {
key = "Foo"
value = "foo-bar"
propagate_at_launch = true
}
availability_zones = ["us-west-2a"]
name = "terraform-test-foobar5"
max_size = 5
min_size = 2
health_check_grace_period = 300
health_check_type = "ELB"
force_delete = true
termination_policies = ["OldestInstance"]
launch_configuration = "${aws_launch_configuration.foobar.name}"
tag {
key = "Foo"
value = "foo-bar"
propagate_at_launch = true
}
}

resource "aws_autoscaling_policy" "foobar_simple" {
name = "foobar_simple"
adjustment_type = "ChangeInCapacity"
cooldown = 300
policy_type = "SimpleScaling"
scaling_adjustment = 2
autoscaling_group_name = "${aws_autoscaling_group.foobar.name}"
}

resource "aws_autoscaling_policy" "foobar" {
name = "foobar"
scaling_adjustment = 4
adjustment_type = "ChangeInCapacity"
cooldown = 300
autoscaling_group_name = "${aws_autoscaling_group.foobar.name}"
resource "aws_autoscaling_policy" "foobar_step" {
name = "foobar_step"
adjustment_type = "ChangeInCapacity"
policy_type = "StepScaling"
estimated_instance_warmup = 200
metric_aggregation_type = "Minimum"
step_adjustment {
scaling_adjustment = 1
metric_interval_lower_bound = 2.0
}
autoscaling_group_name = "${aws_autoscaling_group.foobar.name}"
}
`)
71 changes: 71 additions & 0 deletions builtin/providers/aws/structure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"sort"
"strconv"
"strings"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -305,6 +306,58 @@ func flattenAccessLog(l *elb.AccessLog) []map[string]interface{} {
return result
}

// Takes the result of flatmap.Expand for an array of step adjustments and
// returns a []*autoscaling.StepAdjustment.
func expandStepAdjustments(configured []interface{}) ([]*autoscaling.StepAdjustment, error) {
var adjustments []*autoscaling.StepAdjustment

// Loop over our configured step adjustments and create an array
// of aws-sdk-go compatible objects. We're forced to convert strings
// to floats here because there's no way to detect whether or not
// an uninitialized, optional schema element is "0.0" deliberately.
// With strings, we can test for "", which is definitely an empty
// struct value.
for _, raw := range configured {
data := raw.(map[string]interface{})
a := &autoscaling.StepAdjustment{
ScalingAdjustment: aws.Int64(int64(data["scaling_adjustment"].(int))),
}
if data["metric_interval_lower_bound"] != "" {
bound := data["metric_interval_lower_bound"]
switch bound := bound.(type) {
case string:
f, err := strconv.ParseFloat(bound, 64)
if err != nil {
return nil, fmt.Errorf(
"metric_interval_lower_bound must be a float value represented as a string")
}
a.MetricIntervalLowerBound = aws.Float64(f)
default:
return nil, fmt.Errorf(
"metric_interval_lower_bound isn't a string. This is a bug. Please file an issue.")
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record, I really hate this code. As a schema.Set, we lose the ability to distinguish between an uninitialized float (i.e. 0) and a user-declared value of 0 in the state file for TypeFloat elements. But the StepScaling policies critically depend on absent boundaries -- AWS treats missing boundaries as positive and negative infinity, which apply to a ton of use cases.

Hence I made them strings... which have to be parsed into floats... I hate it too. I'm open to any suggestions to rework this function more sanely, but I can't find any other way to distinguish between a legitimate 0.0 from a user and an empty boundary value.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is agreed! currently, 0's are really tough to work with in Terraform :( The only way around this would be to set default values of 0 and then test for those and them treat as excluded

if data["metric_interval_upper_bound"] != "" {
bound := data["metric_interval_upper_bound"]
switch bound := bound.(type) {
case string:
f, err := strconv.ParseFloat(bound, 64)
if err != nil {
return nil, fmt.Errorf(
"metric_interval_upper_bound must be a float value represented as a string")
}
a.MetricIntervalUpperBound = aws.Float64(f)
default:
return nil, fmt.Errorf(
"metric_interval_upper_bound isn't a string. This is a bug. Please file an issue.")
}
}
adjustments = append(adjustments, a)
}

return adjustments, nil
}

// Flattens a health check into something that flatmap.Flatten()
// can handle
func flattenHealthCheck(check *elb.HealthCheck) []map[string]interface{} {
Expand Down Expand Up @@ -564,6 +617,24 @@ func flattenAttachment(a *ec2.NetworkInterfaceAttachment) map[string]interface{}
return att
}

// Flattens step adjustments into a list of map[string]interface.
func flattenStepAdjustments(adjustments []*autoscaling.StepAdjustment) []map[string]interface{} {
result := make([]map[string]interface{}, 0, len(adjustments))
for _, raw := range adjustments {
a := map[string]interface{}{
"scaling_adjustment": *raw.ScalingAdjustment,
}
if raw.MetricIntervalUpperBound != nil {
a["metric_interval_upper_bound"] = *raw.MetricIntervalUpperBound
}
if raw.MetricIntervalLowerBound != nil {
a["metric_interval_lower_bound"] = *raw.MetricIntervalLowerBound
}
result = append(result, a)
}
return result
}

func flattenResourceRecords(recs []*route53.ResourceRecord) []string {
strs := make([]string, 0, len(recs))
for _, r := range recs {
Expand Down
Loading