From 3330e2a469bf86d7f50aba9e5b7a9c430445c449 Mon Sep 17 00:00:00 2001 From: "xiaowei.wang" Date: Sun, 11 Mar 2018 15:53:38 +0100 Subject: [PATCH 1/5] resource/autoscaling_policy: cleanup code --- aws/resource_aws_autoscaling_policy.go | 58 ++++++++++----------- aws/resource_aws_autoscaling_policy_test.go | 16 +++--- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/aws/resource_aws_autoscaling_policy.go b/aws/resource_aws_autoscaling_policy.go index 303c863b179..a9d7b8d630a 100644 --- a/aws/resource_aws_autoscaling_policy.go +++ b/aws/resource_aws_autoscaling_policy.go @@ -21,73 +21,73 @@ func resourceAwsAutoscalingPolicy() *schema.Resource { Delete: resourceAwsAutoscalingPolicyDelete, Schema: map[string]*schema.Schema{ - "arn": &schema.Schema{ + "arn": { Type: schema.TypeString, Computed: true, }, - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "adjustment_type": &schema.Schema{ + "adjustment_type": { Type: schema.TypeString, Optional: true, }, - "autoscaling_group_name": &schema.Schema{ + "autoscaling_group_name": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "policy_type": &schema.Schema{ + "policy_type": { Type: schema.TypeString, Optional: true, Default: "SimpleScaling", // preserve AWS's default to make validation easier. }, - "cooldown": &schema.Schema{ + "cooldown": { Type: schema.TypeInt, Optional: true, }, - "estimated_instance_warmup": &schema.Schema{ + "estimated_instance_warmup": { Type: schema.TypeInt, Optional: true, }, - "metric_aggregation_type": &schema.Schema{ + "metric_aggregation_type": { Type: schema.TypeString, Optional: true, Computed: true, }, - "min_adjustment_magnitude": &schema.Schema{ + "min_adjustment_magnitude": { Type: schema.TypeInt, Optional: true, ValidateFunc: validation.IntAtLeast(1), }, - "min_adjustment_step": &schema.Schema{ + "min_adjustment_step": { Type: schema.TypeInt, Optional: true, Deprecated: "Use min_adjustment_magnitude instead, otherwise you may see a perpetual diff on this resource.", ConflictsWith: []string{"min_adjustment_magnitude"}, }, - "scaling_adjustment": &schema.Schema{ + "scaling_adjustment": { Type: schema.TypeInt, Optional: true, ConflictsWith: []string{"step_adjustment"}, }, - "step_adjustment": &schema.Schema{ + "step_adjustment": { Type: schema.TypeSet, Optional: true, ConflictsWith: []string{"scaling_adjustment"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "metric_interval_lower_bound": &schema.Schema{ + "metric_interval_lower_bound": { Type: schema.TypeString, Optional: true, }, - "metric_interval_upper_bound": &schema.Schema{ + "metric_interval_upper_bound": { Type: schema.TypeString, Optional: true, }, - "scaling_adjustment": &schema.Schema{ + "scaling_adjustment": { Type: schema.TypeInt, Required: true, }, @@ -95,77 +95,77 @@ func resourceAwsAutoscalingPolicy() *schema.Resource { }, Set: resourceAwsAutoscalingScalingAdjustmentHash, }, - "target_tracking_configuration": &schema.Schema{ + "target_tracking_configuration": { Type: schema.TypeList, Optional: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "predefined_metric_specification": &schema.Schema{ + "predefined_metric_specification": { Type: schema.TypeList, Optional: true, MaxItems: 1, ConflictsWith: []string{"target_tracking_configuration.0.customized_metric_specification"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "predefined_metric_type": &schema.Schema{ + "predefined_metric_type": { Type: schema.TypeString, Required: true, }, - "resource_label": &schema.Schema{ + "resource_label": { Type: schema.TypeString, Optional: true, }, }, }, }, - "customized_metric_specification": &schema.Schema{ + "customized_metric_specification": { Type: schema.TypeList, Optional: true, MaxItems: 1, ConflictsWith: []string{"target_tracking_configuration.0.predefined_metric_specification"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "metric_dimension": &schema.Schema{ + "metric_dimension": { Type: schema.TypeList, Optional: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, }, - "value": &schema.Schema{ + "value": { Type: schema.TypeString, Required: true, }, }, }, }, - "metric_name": &schema.Schema{ + "metric_name": { Type: schema.TypeString, Required: true, }, - "namespace": &schema.Schema{ + "namespace": { Type: schema.TypeString, Required: true, }, - "statistic": &schema.Schema{ + "statistic": { Type: schema.TypeString, Required: true, }, - "unit": &schema.Schema{ + "unit": { Type: schema.TypeString, Optional: true, }, }, }, }, - "target_value": &schema.Schema{ + "target_value": { Type: schema.TypeFloat, Required: true, }, - "disable_scale_in": &schema.Schema{ + "disable_scale_in": { Type: schema.TypeBool, Optional: true, Default: false, diff --git a/aws/resource_aws_autoscaling_policy_test.go b/aws/resource_aws_autoscaling_policy_test.go index 3e4ad5d3cf9..61df8caeee3 100644 --- a/aws/resource_aws_autoscaling_policy_test.go +++ b/aws/resource_aws_autoscaling_policy_test.go @@ -25,7 +25,7 @@ func TestAccAWSAutoscalingPolicy_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSAutoscalingPolicyConfig(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), @@ -58,7 +58,7 @@ func TestAccAWSAutoscalingPolicy_disappears(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSAutoscalingPolicyConfig(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), @@ -118,7 +118,7 @@ func TestAccAWSAutoscalingPolicy_upgrade(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSAutoscalingPolicyConfig_upgrade_614(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), @@ -128,7 +128,7 @@ func TestAccAWSAutoscalingPolicy_upgrade(t *testing.T) { ExpectNonEmptyPlan: true, }, - resource.TestStep{ + { Config: testAccAWSAutoscalingPolicyConfig_upgrade_615(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), @@ -150,7 +150,7 @@ func TestAccAWSAutoscalingPolicy_TargetTrack_Predefined(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAwsAutoscalingPolicyConfig_TargetTracking_Predefined(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.test", &policy), @@ -170,7 +170,7 @@ func TestAccAWSAutoscalingPolicy_TargetTrack_Custom(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAwsAutoscalingPolicyConfig_TargetTracking_Custom(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.test", &policy), @@ -189,7 +189,7 @@ func TestAccAWSAutoscalingPolicy_zerovalue(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSAutoscalingPolicyConfig_zerovalue(acctest.RandString(5)), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &simplepolicy), @@ -391,7 +391,7 @@ func TestAccAWSAutoscalingPolicy_SimpleScalingStepAdjustment(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSAutoscalingPolicyConfig_SimpleScalingStepAdjustment(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), From a3d379e72d55f1b1d3261e5554be121abe784e48 Mon Sep 17 00:00:00 2001 From: "xiaowei.wang" Date: Sun, 11 Mar 2018 20:55:20 +0100 Subject: [PATCH 2/5] resource/autoscaling_policy: cleanup acceptance test --- aws/resource_aws_autoscaling_policy_test.go | 392 ++++++++------------ 1 file changed, 154 insertions(+), 238 deletions(-) diff --git a/aws/resource_aws_autoscaling_policy_test.go b/aws/resource_aws_autoscaling_policy_test.go index 61df8caeee3..78e9a32a66e 100644 --- a/aws/resource_aws_autoscaling_policy_test.go +++ b/aws/resource_aws_autoscaling_policy_test.go @@ -18,7 +18,7 @@ import ( func TestAccAWSAutoscalingPolicy_basic(t *testing.T) { var policy autoscaling.ScalingPolicy - name := fmt.Sprintf("terraform-test-foobar-%s", acctest.RandString(5)) + name := fmt.Sprintf("terraform-testacc-asp-%s", acctest.RandString(5)) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -26,22 +26,32 @@ func TestAccAWSAutoscalingPolicy_basic(t *testing.T) { CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSAutoscalingPolicyConfig(name), + Config: testAccAWSAutoscalingPolicyConfig_basic(name), Check: resource.ComposeTestCheckFunc( 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", "name", name+"-foobar_simple"), resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "scaling_adjustment", "2"), resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "autoscaling_group_name", name), 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", "name", 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", name), + testAccCheckScalingPolicyExists("aws_autoscaling_policy.ecs_general_purpose_scale_out", &policy), + resource.TestCheckResourceAttr("aws_autoscaling_policy.ecs_general_purpose_scale_out", "name", "memory-reservation-high"), + ), + }, + { + Config: testAccAWSAutoscalingPolicyConfig_basicUpdate(name), + Check: resource.ComposeTestCheckFunc( + testAccCheckScalingPolicyExists("aws_autoscaling_policy.ecs_general_purpose_scale_out", &policy), + resource.TestCheckResourceAttr("aws_autoscaling_policy.ecs_general_purpose_scale_out", "name", "memory-reservation-high"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.ecs_general_purpose_scale_out", "step_adjustment.#", "1"), ), }, }, @@ -51,7 +61,7 @@ func TestAccAWSAutoscalingPolicy_basic(t *testing.T) { func TestAccAWSAutoscalingPolicy_disappears(t *testing.T) { var policy autoscaling.ScalingPolicy - name := fmt.Sprintf("terraform-test-foobar-%s", acctest.RandString(5)) + name := fmt.Sprintf("terraform-testacc-asp-%s", acctest.RandString(5)) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -59,7 +69,7 @@ func TestAccAWSAutoscalingPolicy_disappears(t *testing.T) { CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSAutoscalingPolicyConfig(name), + Config: testAccAWSAutoscalingPolicyConfig_basic(name), Check: resource.ComposeTestCheckFunc( testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), testAccCheckScalingPolicyDisappears(&policy), @@ -111,7 +121,7 @@ func testAccCheckScalingPolicyDisappears(conf *autoscaling.ScalingPolicy) resour func TestAccAWSAutoscalingPolicy_upgrade(t *testing.T) { var policy autoscaling.ScalingPolicy - name := acctest.RandString(5) + name := fmt.Sprintf("terraform-testacc-asp-%s", acctest.RandString(5)) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -127,7 +137,6 @@ func TestAccAWSAutoscalingPolicy_upgrade(t *testing.T) { ), ExpectNonEmptyPlan: true, }, - { Config: testAccAWSAutoscalingPolicyConfig_upgrade_615(name), Check: resource.ComposeTestCheckFunc( @@ -140,10 +149,32 @@ func TestAccAWSAutoscalingPolicy_upgrade(t *testing.T) { }) } +func TestAccAWSAutoscalingPolicy_SimpleScalingStepAdjustment(t *testing.T) { + var policy autoscaling.ScalingPolicy + + name := fmt.Sprintf("terraform-testacc-asp-%s", acctest.RandString(5)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSAutoscalingPolicyConfig_SimpleScalingStepAdjustment(name), + Check: resource.ComposeTestCheckFunc( + testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "adjustment_type", "ExactCapacity"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "scaling_adjustment", "0"), + ), + }, + }, + }) +} + func TestAccAWSAutoscalingPolicy_TargetTrack_Predefined(t *testing.T) { var policy autoscaling.ScalingPolicy - name := acctest.RandString(5) + name := fmt.Sprintf("terraform-testacc-asp-%s", acctest.RandString(5)) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -163,7 +194,7 @@ func TestAccAWSAutoscalingPolicy_TargetTrack_Predefined(t *testing.T) { func TestAccAWSAutoscalingPolicy_TargetTrack_Custom(t *testing.T) { var policy autoscaling.ScalingPolicy - name := acctest.RandString(5) + name := fmt.Sprintf("terraform-testacc-asp-%s", acctest.RandString(5)) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -256,42 +287,50 @@ func testAccCheckAWSAutoscalingPolicyDestroy(s *terraform.State) error { return nil } -func testAccAWSAutoscalingPolicyConfig(name string) string { +func testAccAWSAutoscalingPolicyConfig_base(name string) string { return fmt.Sprintf(` -resource "aws_launch_configuration" "foobar" { +data "aws_ami" "amzn" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn2-ami-hvm-*"] + } +} + +data "aws_availability_zones" "available" {} + +resource "aws_launch_configuration" "test" { name = "%s" - image_id = "ami-21f78e11" - instance_type = "t1.micro" + image_id = "${data.aws_ami.amzn.id}" + instance_type = "t2.micro" } -resource "aws_autoscaling_group" "foobar" { - availability_zones = ["us-west-2a"] +resource "aws_autoscaling_group" "test" { + availability_zones = ["${data.aws_availability_zones.available.names}"] name = "%s" - max_size = 5 - min_size = 2 - health_check_grace_period = 300 - health_check_type = "ELB" + max_size = 0 + min_size = 0 force_delete = true - termination_policies = ["OldestInstance"] - launch_configuration = "${aws_launch_configuration.foobar.name}" - tag { - key = "Foo" - value = "foo-bar" - propagate_at_launch = true - } + launch_configuration = "${aws_launch_configuration.test.name}" +} +`, name, name) } +func testAccAWSAutoscalingPolicyConfig_basic(name string) string { + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "foobar_simple" + name = "%s-foobar_simple" adjustment_type = "ChangeInCapacity" cooldown = 300 policy_type = "SimpleScaling" scaling_adjustment = 2 - autoscaling_group_name = "${aws_autoscaling_group.foobar.name}" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } resource "aws_autoscaling_policy" "foobar_step" { - name = "foobar_step" + name = "%s-foobar_step" adjustment_type = "ChangeInCapacity" policy_type = "StepScaling" estimated_instance_warmup = 200 @@ -300,180 +339,116 @@ resource "aws_autoscaling_policy" "foobar_step" { scaling_adjustment = 1 metric_interval_lower_bound = 2.0 } - autoscaling_group_name = "${aws_autoscaling_group.foobar.name}" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" +} + +resource "aws_autoscaling_policy" "ecs_general_purpose_scale_out" { + name = "memory-reservation-high" + + autoscaling_group_name = "${aws_autoscaling_group.test.name}" + estimated_instance_warmup = 60 + + adjustment_type = "PercentChangeInCapacity" + policy_type = "StepScaling" + + metric_aggregation_type = "Maximum" + + step_adjustment { + scaling_adjustment = 100 + metric_interval_lower_bound = 0 + } } `, name, name) } -func testAccAWSAutoscalingPolicyConfig_upgrade_614(name string) string { - return fmt.Sprintf(` -resource "aws_launch_configuration" "foobar" { - name = "tf-test-%s" - image_id = "ami-21f78e11" - instance_type = "t1.micro" -} - -resource "aws_autoscaling_group" "foobar" { - availability_zones = ["us-west-2a"] - name = "terraform-test-%s" - max_size = 5 - min_size = 1 - 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 +func testAccAWSAutoscalingPolicyConfig_basicUpdate(name string) string { + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` +resource "aws_autoscaling_policy" "foobar_simple" { + name = "%s-foobar_simple" + adjustment_type = "ChangeInCapacity" + cooldown = 300 + policy_type = "SimpleScaling" + scaling_adjustment = 2 + autoscaling_group_name = "${aws_autoscaling_group.test.name}" +} + +resource "aws_autoscaling_policy" "foobar_step" { + name = "%s-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.test.name}" +} + +resource "aws_autoscaling_policy" "ecs_general_purpose_scale_out" { + name = "memory-reservation-high" + + autoscaling_group_name = "${aws_autoscaling_group.test.name}" + estimated_instance_warmup = 60 + + adjustment_type = "PercentChangeInCapacity" + policy_type = "StepScaling" + + metric_aggregation_type = "Maximum" + + step_adjustment { + scaling_adjustment = 10 + metric_interval_lower_bound = 0 } } +`, name, name) +} +func testAccAWSAutoscalingPolicyConfig_upgrade_614(name string) string { + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "foobar_simple_%s" + name = "%s-foobar_simple" adjustment_type = "PercentChangeInCapacity" cooldown = 300 policy_type = "SimpleScaling" scaling_adjustment = 2 min_adjustment_step = 1 - autoscaling_group_name = "${aws_autoscaling_group.foobar.name}" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } -`, name, name, name) +`, name) } func testAccAWSAutoscalingPolicyConfig_upgrade_615(name string) string { - return fmt.Sprintf(` -resource "aws_launch_configuration" "foobar" { - name = "tf-test-%s" - image_id = "ami-21f78e11" - instance_type = "t1.micro" -} - -resource "aws_autoscaling_group" "foobar" { - availability_zones = ["us-west-2a"] - name = "terraform-test-%s" - max_size = 5 - min_size = 1 - 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 - } -} - + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "foobar_simple_%s" + name = "%s-foobar_simple" adjustment_type = "PercentChangeInCapacity" cooldown = 300 policy_type = "SimpleScaling" scaling_adjustment = 2 min_adjustment_magnitude = 1 - autoscaling_group_name = "${aws_autoscaling_group.foobar.name}" -} -`, name, name, name) + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } - -func TestAccAWSAutoscalingPolicy_SimpleScalingStepAdjustment(t *testing.T) { - var policy autoscaling.ScalingPolicy - - name := acctest.RandString(5) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSAutoscalingPolicyDestroy, - Steps: []resource.TestStep{ - { - Config: testAccAWSAutoscalingPolicyConfig_SimpleScalingStepAdjustment(name), - Check: resource.ComposeTestCheckFunc( - testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), - resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "adjustment_type", "ExactCapacity"), - resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "scaling_adjustment", "0"), - ), - }, - }, - }) +`, name) } func testAccAWSAutoscalingPolicyConfig_SimpleScalingStepAdjustment(name string) string { - return fmt.Sprintf(` -resource "aws_launch_configuration" "foobar" { - name = "tf-test-%s" - image_id = "ami-21f78e11" - instance_type = "t1.micro" -} - -resource "aws_autoscaling_group" "foobar" { - availability_zones = ["us-west-2a"] - name = "terraform-test-%s" - max_size = 5 - min_size = 0 - 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 - } -} - + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "foobar_simple_%s" + name = "%s-foobar_simple" adjustment_type = "ExactCapacity" cooldown = 300 policy_type = "SimpleScaling" scaling_adjustment = 0 - autoscaling_group_name = "${aws_autoscaling_group.foobar.name}" -} -`, name, name, name) -} - -func testAccAwsAutoscalingPolicyConfig_TargetTracking_Predefined(rName string) string { - return fmt.Sprintf(` -data "aws_ami" "amzn" { - most_recent = true - owners = ["amazon"] - - filter { - name = "name" - values = ["amzn-ami-hvm-*-x86_64-gp2"] - } -} - -data "aws_availability_zones" "test" {} - -resource "aws_launch_configuration" "test" { - name = "tf-test-%s" - image_id = "${data.aws_ami.amzn.id}" - instance_type = "t2.micro" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } - -resource "aws_autoscaling_group" "test" { - availability_zones = ["${data.aws_availability_zones.test.names[0]}"] - name = "tf-test-%s" - max_size = 5 - min_size = 0 - health_check_grace_period = 300 - health_check_type = "ELB" - force_delete = true - termination_policies = ["OldestInstance"] - launch_configuration = "${aws_launch_configuration.test.name}" +`, name) } +func testAccAwsAutoscalingPolicyConfig_TargetTracking_Predefined(name string) string { + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "test" { - name = "tf-as-%s" + name = "%s-test" policy_type = "TargetTrackingScaling" autoscaling_group_name = "${aws_autoscaling_group.test.name}" @@ -484,43 +459,13 @@ resource "aws_autoscaling_policy" "test" { target_value = 40.0 } } -`, rName, rName, rName) -} - -func testAccAwsAutoscalingPolicyConfig_TargetTracking_Custom(rName string) string { - return fmt.Sprintf(` -data "aws_ami" "amzn" { - most_recent = true - owners = ["amazon"] - - filter { - name = "name" - values = ["amzn-ami-hvm-*-x86_64-gp2"] - } -} - -data "aws_availability_zones" "test" {} - -resource "aws_launch_configuration" "test" { - name = "tf-test-%s" - image_id = "${data.aws_ami.amzn.id}" - instance_type = "t2.micro" -} - -resource "aws_autoscaling_group" "test" { - availability_zones = ["${data.aws_availability_zones.test.names[0]}"] - name = "tf-test-%s" - max_size = 5 - min_size = 0 - health_check_grace_period = 300 - health_check_type = "ELB" - force_delete = true - termination_policies = ["OldestInstance"] - launch_configuration = "${aws_launch_configuration.test.name}" +`, name) } +func testAccAwsAutoscalingPolicyConfig_TargetTracking_Custom(name string) string { + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "test" { - name = "tf-as-%s" + name = "%s-test" policy_type = "TargetTrackingScaling" autoscaling_group_name = "${aws_autoscaling_group.test.name}" @@ -537,51 +482,22 @@ resource "aws_autoscaling_policy" "test" { target_value = 40.0 } } -`, rName, rName, rName) +`, name) } func testAccAWSAutoscalingPolicyConfig_zerovalue(name string) string { - return fmt.Sprintf(` -data "aws_ami" "amzn" { - most_recent = true - owners = ["amazon"] - filter { - name = "name" - values = ["amzn-ami-hvm-*-x86_64-gp2"] - } -} - -resource "aws_launch_configuration" "foobar" { - name = "tf-test-%s" - image_id = "${data.aws_ami.amzn.id}" - instance_type = "t2.micro" -} - -data "aws_availability_zones" "test" {} - -resource "aws_autoscaling_group" "foobar" { - availability_zones = ["${data.aws_availability_zones.test.names[0]}"] - name = "terraform-test-%s" - max_size = 5 - min_size = 0 - health_check_grace_period = 300 - health_check_type = "ELB" - force_delete = true - termination_policies = ["OldestInstance"] - launch_configuration = "${aws_launch_configuration.foobar.name}" -} - + return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "foobar_simple_%s" + name = "%s-foobar_simple" adjustment_type = "ExactCapacity" cooldown = 0 policy_type = "SimpleScaling" scaling_adjustment = 0 - autoscaling_group_name = "${aws_autoscaling_group.foobar.name}" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } resource "aws_autoscaling_policy" "foobar_step" { - name = "foobar_step_%s" + name = "%s-foobar_step" adjustment_type = "PercentChangeInCapacity" policy_type = "StepScaling" estimated_instance_warmup = 0 @@ -591,7 +507,7 @@ resource "aws_autoscaling_policy" "foobar_step" { metric_interval_lower_bound = 2.0 } min_adjustment_magnitude = 1 - autoscaling_group_name = "${aws_autoscaling_group.foobar.name}" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } -`, name, name, name, name) +`, name, name) } From 738e6ae6eea70db4f847790f895b8f87870a73e3 Mon Sep 17 00:00:00 2001 From: "xiaowei.wang" Date: Sun, 11 Mar 2018 21:18:32 +0100 Subject: [PATCH 3/5] resource/autoscaling_policy: set parameters based on policy type --- aws/resource_aws_autoscaling_policy.go | 104 +++++------ aws/resource_aws_autoscaling_policy_test.go | 182 +++++++++----------- 2 files changed, 121 insertions(+), 165 deletions(-) diff --git a/aws/resource_aws_autoscaling_policy.go b/aws/resource_aws_autoscaling_policy.go index a9d7b8d630a..8c4f2159678 100644 --- a/aws/resource_aws_autoscaling_policy.go +++ b/aws/resource_aws_autoscaling_policy.go @@ -43,6 +43,11 @@ func resourceAwsAutoscalingPolicy() *schema.Resource { Type: schema.TypeString, Optional: true, Default: "SimpleScaling", // preserve AWS's default to make validation easier. + ValidateFunc: validation.StringInSlice([]string{ + "SimpleScaling", + "StepScaling", + "TargetTrackingScaling", + }, false), }, "cooldown": { Type: schema.TypeInt, @@ -281,32 +286,54 @@ func getAwsAutoscalingPutScalingPolicyInput(d *schema.ResourceData) (autoscaling PolicyName: aws.String(d.Get("name").(string)), } - if v, ok := d.GetOk("adjustment_type"); ok { + // get policy_type first as parameter support depends on policy type + policyType := d.Get("policy_type") + params.PolicyType = aws.String(policyType.(string)) + + // This parameter is supported if the policy type is SimpleScaling or StepScaling. + if v, ok := d.GetOk("adjustment_type"); ok && (policyType == "SimpleScaling" || policyType == "StepScaling") { params.AdjustmentType = aws.String(v.(string)) } - if v, ok := d.GetOkExists("cooldown"); ok { + // This parameter is supported if the policy type is SimpleScaling. + if v, ok := d.GetOkExists("cooldown"); ok && policyType == "SimpleScaling" { params.Cooldown = aws.Int64(int64(v.(int))) } - if v, ok := d.GetOkExists("estimated_instance_warmup"); ok { + // This parameter is supported if the policy type is StepScaling or TargetTrackingScaling. + if v, ok := d.GetOkExists("estimated_instance_warmup"); ok && (policyType == "StepScaling" || policyType == "TargetTrackingScaling") { params.EstimatedInstanceWarmup = aws.Int64(int64(v.(int))) } - if v, ok := d.GetOk("metric_aggregation_type"); ok { + // This parameter is supported if the policy type is StepScaling. + if v, ok := d.GetOk("metric_aggregation_type"); ok && policyType == "StepScaling" { params.MetricAggregationType = aws.String(v.(string)) } - if v, ok := d.GetOk("policy_type"); ok { - params.PolicyType = aws.String(v.(string)) + // MinAdjustmentMagnitude is supported if the policy type is SimpleScaling or StepScaling. + // MinAdjustmentStep is available for backward compatibility. Use MinAdjustmentMagnitude instead. + if v, ok := d.GetOkExists("min_adjustment_magnitude"); ok && v.(int) != 0 && (policyType == "SimpleScaling" || policyType == "StepScaling") { + params.MinAdjustmentMagnitude = aws.Int64(int64(v.(int))) + } else if v, ok := d.GetOkExists("min_adjustment_step"); ok && v.(int) != 0 && (policyType == "SimpleScaling" || policyType == "StepScaling") { + params.MinAdjustmentStep = aws.Int64(int64(v.(int))) } + // This parameter is required if the policy type is SimpleScaling and not supported otherwise. //if policy_type=="SimpleScaling" then scaling_adjustment is required and 0 is allowed - if v, ok := d.GetOkExists("scaling_adjustment"); ok || *params.PolicyType == "SimpleScaling" { + if policyType == "SimpleScaling" { + v, ok := d.GetOkExists("scaling_adjustment") + if !ok { + return params, fmt.Errorf("scaling_adjustment is required for policy type SimpleScaling") + } params.ScalingAdjustment = aws.Int64(int64(v.(int))) } - if v, ok := d.GetOk("step_adjustment"); ok { + // This parameter is required if the policy type is StepScaling and not supported otherwise. + if policyType == "StepScaling" { + v, ok := d.GetOk("step_adjustment") + if !ok { + return params, fmt.Errorf("step_adjustment is required for policy type StepScaling") + } 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!") @@ -314,64 +341,15 @@ func getAwsAutoscalingPutScalingPolicyInput(d *schema.ResourceData) (autoscaling params.StepAdjustments = steps } - if v, ok := d.GetOkExists("min_adjustment_magnitude"); ok { - // params.MinAdjustmentMagnitude = aws.Int64(int64(d.Get("min_adjustment_magnitude").(int))) - params.MinAdjustmentMagnitude = aws.Int64(int64(v.(int))) - } else if v, ok := d.GetOkExists("min_adjustment_step"); ok { - // params.MinAdjustmentStep = aws.Int64(int64(d.Get("min_adjustment_step").(int))) - params.MinAdjustmentStep = aws.Int64(int64(v.(int))) - } - - if v, ok := d.GetOk("target_tracking_configuration"); ok { + // This parameter is required if the policy type is TargetTrackingScaling and not supported otherwise. + if policyType == "TargetTrackingScaling" { + v, ok := d.GetOk("target_tracking_configuration") + if !ok { + return params, fmt.Errorf("target_tracking_configuration is required for policy type TargetTrackingScaling") + } params.TargetTrackingConfiguration = expandTargetTrackingConfiguration(v.([]interface{})) } - // 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!") - } - if *params.PolicyType == "SimpleScaling" && params.TargetTrackingConfiguration != nil { - return params, fmt.Errorf("SimpleScaling policy types cannot use target_tracking_configuration!") - } - - // 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!") - } - if *params.PolicyType == "StepScaling" && params.TargetTrackingConfiguration != nil { - return params, fmt.Errorf("StepScaling policy types cannot use target_tracking_configuration!") - } - - // Third, TargetTrackingScaling policy types... - if *params.PolicyType == "TargetTrackingScaling" && params.AdjustmentType != nil { - return params, fmt.Errorf("TargetTrackingScaling policy types cannot use adjustment_type!") - } - if *params.PolicyType == "TargetTrackingScaling" && params.Cooldown != nil { - return params, fmt.Errorf("TargetTrackingScaling policy types cannot use cooldown!") - } - if *params.PolicyType == "TargetTrackingScaling" && params.MetricAggregationType != nil { - return params, fmt.Errorf("TargetTrackingScaling policy types cannot use metric_aggregation_type!") - } - if *params.PolicyType == "TargetTrackingScaling" && params.MinAdjustmentMagnitude != nil { - return params, fmt.Errorf("TargetTrackingScaling policy types cannot use min_adjustment_magnitude!") - } - if *params.PolicyType == "TargetTrackingScaling" && params.ScalingAdjustment != nil { - return params, fmt.Errorf("TargetTrackingScaling policy types cannot use scaling_adjustment!") - } - if *params.PolicyType == "TargetTrackingScaling" && params.StepAdjustments != nil { - return params, fmt.Errorf("TargetTrackingScaling policy types cannot use step_adjustments!") - } - return params, nil } diff --git a/aws/resource_aws_autoscaling_policy_test.go b/aws/resource_aws_autoscaling_policy_test.go index 78e9a32a66e..6989ca4ec22 100644 --- a/aws/resource_aws_autoscaling_policy_test.go +++ b/aws/resource_aws_autoscaling_policy_test.go @@ -42,16 +42,19 @@ func TestAccAWSAutoscalingPolicy_basic(t *testing.T) { 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", name), - testAccCheckScalingPolicyExists("aws_autoscaling_policy.ecs_general_purpose_scale_out", &policy), - resource.TestCheckResourceAttr("aws_autoscaling_policy.ecs_general_purpose_scale_out", "name", "memory-reservation-high"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "step_adjustment.2042107634.scaling_adjustment", "1"), ), }, { Config: testAccAWSAutoscalingPolicyConfig_basicUpdate(name), Check: resource.ComposeTestCheckFunc( - testAccCheckScalingPolicyExists("aws_autoscaling_policy.ecs_general_purpose_scale_out", &policy), - resource.TestCheckResourceAttr("aws_autoscaling_policy.ecs_general_purpose_scale_out", "name", "memory-reservation-high"), - resource.TestCheckResourceAttr("aws_autoscaling_policy.ecs_general_purpose_scale_out", "step_adjustment.#", "1"), + testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_simple", &policy), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "policy_type", "SimpleScaling"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_simple", "cooldown", "30"), + testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_step", &policy), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "policy_type", "StepScaling"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "estimated_instance_warmup", "20"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "step_adjustment.997979330.scaling_adjustment", "10"), ), }, }, @@ -302,18 +305,18 @@ data "aws_ami" "amzn" { data "aws_availability_zones" "available" {} resource "aws_launch_configuration" "test" { - name = "%s" - image_id = "${data.aws_ami.amzn.id}" - instance_type = "t2.micro" + name = "%s" + image_id = "${data.aws_ami.amzn.id}" + instance_type = "t2.micro" } resource "aws_autoscaling_group" "test" { - availability_zones = ["${data.aws_availability_zones.available.names}"] - name = "%s" - max_size = 0 - min_size = 0 - force_delete = true - launch_configuration = "${aws_launch_configuration.test.name}" + availability_zones = ["${data.aws_availability_zones.available.names}"] + name = "%s" + max_size = 0 + min_size = 0 + force_delete = true + launch_configuration = "${aws_launch_configuration.test.name}" } `, name, name) } @@ -321,42 +324,27 @@ resource "aws_autoscaling_group" "test" { func testAccAWSAutoscalingPolicyConfig_basic(name string) string { return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "%s-foobar_simple" - adjustment_type = "ChangeInCapacity" - cooldown = 300 - policy_type = "SimpleScaling" - scaling_adjustment = 2 - autoscaling_group_name = "${aws_autoscaling_group.test.name}" + name = "%s-foobar_simple" + adjustment_type = "ChangeInCapacity" + cooldown = 300 + policy_type = "SimpleScaling" + scaling_adjustment = 2 + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } resource "aws_autoscaling_policy" "foobar_step" { - name = "%s-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.test.name}" -} - -resource "aws_autoscaling_policy" "ecs_general_purpose_scale_out" { - name = "memory-reservation-high" - - autoscaling_group_name = "${aws_autoscaling_group.test.name}" - estimated_instance_warmup = 60 - - adjustment_type = "PercentChangeInCapacity" - policy_type = "StepScaling" - - metric_aggregation_type = "Maximum" + name = "%s-foobar_step" + adjustment_type = "ChangeInCapacity" + policy_type = "StepScaling" + estimated_instance_warmup = 200 + metric_aggregation_type = "Minimum" step_adjustment { - scaling_adjustment = 100 - metric_interval_lower_bound = 0 + scaling_adjustment = 1 + metric_interval_lower_bound = 2.0 } + + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } `, name, name) } @@ -364,42 +352,27 @@ resource "aws_autoscaling_policy" "ecs_general_purpose_scale_out" { func testAccAWSAutoscalingPolicyConfig_basicUpdate(name string) string { return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "%s-foobar_simple" - adjustment_type = "ChangeInCapacity" - cooldown = 300 - policy_type = "SimpleScaling" - scaling_adjustment = 2 - autoscaling_group_name = "${aws_autoscaling_group.test.name}" + name = "%s-foobar_simple" + adjustment_type = "ChangeInCapacity" + cooldown = 30 + policy_type = "SimpleScaling" + scaling_adjustment = 2 + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } resource "aws_autoscaling_policy" "foobar_step" { - name = "%s-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.test.name}" -} - -resource "aws_autoscaling_policy" "ecs_general_purpose_scale_out" { - name = "memory-reservation-high" - - autoscaling_group_name = "${aws_autoscaling_group.test.name}" - estimated_instance_warmup = 60 - - adjustment_type = "PercentChangeInCapacity" - policy_type = "StepScaling" - - metric_aggregation_type = "Maximum" + name = "%s-foobar_step" + adjustment_type = "ChangeInCapacity" + policy_type = "StepScaling" + estimated_instance_warmup = 20 + metric_aggregation_type = "Minimum" step_adjustment { scaling_adjustment = 10 - metric_interval_lower_bound = 0 + metric_interval_lower_bound = 2.0 } + + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } `, name, name) } @@ -435,12 +408,12 @@ resource "aws_autoscaling_policy" "foobar_simple" { func testAccAWSAutoscalingPolicyConfig_SimpleScalingStepAdjustment(name string) string { return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "%s-foobar_simple" - adjustment_type = "ExactCapacity" - cooldown = 300 - policy_type = "SimpleScaling" - scaling_adjustment = 0 - autoscaling_group_name = "${aws_autoscaling_group.test.name}" + name = "%s-foobar_simple" + adjustment_type = "ExactCapacity" + cooldown = 300 + policy_type = "SimpleScaling" + scaling_adjustment = 0 + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } `, name) } @@ -448,14 +421,15 @@ resource "aws_autoscaling_policy" "foobar_simple" { func testAccAwsAutoscalingPolicyConfig_TargetTracking_Predefined(name string) string { return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "test" { - name = "%s-test" - policy_type = "TargetTrackingScaling" - autoscaling_group_name = "${aws_autoscaling_group.test.name}" + name = "%s-test" + policy_type = "TargetTrackingScaling" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" target_tracking_configuration { predefined_metric_specification { predefined_metric_type = "ASGAverageCPUUtilization" } + target_value = 40.0 } } @@ -465,20 +439,22 @@ resource "aws_autoscaling_policy" "test" { func testAccAwsAutoscalingPolicyConfig_TargetTracking_Custom(name string) string { return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "test" { - name = "%s-test" - policy_type = "TargetTrackingScaling" - autoscaling_group_name = "${aws_autoscaling_group.test.name}" + name = "%s-test" + policy_type = "TargetTrackingScaling" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" target_tracking_configuration { customized_metric_specification { metric_dimension { - name = "fuga" + name = "fuga" value = "fuga" } + metric_name = "hoge" - namespace = "hoge" - statistic = "Average" + namespace = "hoge" + statistic = "Average" } + target_value = 40.0 } } @@ -488,26 +464,28 @@ resource "aws_autoscaling_policy" "test" { func testAccAWSAutoscalingPolicyConfig_zerovalue(name string) string { return testAccAWSAutoscalingPolicyConfig_base(name) + fmt.Sprintf(` resource "aws_autoscaling_policy" "foobar_simple" { - name = "%s-foobar_simple" - adjustment_type = "ExactCapacity" - cooldown = 0 - policy_type = "SimpleScaling" - scaling_adjustment = 0 - autoscaling_group_name = "${aws_autoscaling_group.test.name}" + name = "%s-foobar_simple" + adjustment_type = "ExactCapacity" + cooldown = 0 + policy_type = "SimpleScaling" + scaling_adjustment = 0 + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } resource "aws_autoscaling_policy" "foobar_step" { - name = "%s-foobar_step" - adjustment_type = "PercentChangeInCapacity" - policy_type = "StepScaling" + name = "%s-foobar_step" + adjustment_type = "PercentChangeInCapacity" + policy_type = "StepScaling" estimated_instance_warmup = 0 - metric_aggregation_type = "Minimum" - step_adjustment { - scaling_adjustment = 1 + metric_aggregation_type = "Minimum" + + step_adjustment { + scaling_adjustment = 1 metric_interval_lower_bound = 2.0 - } + } + min_adjustment_magnitude = 1 - autoscaling_group_name = "${aws_autoscaling_group.test.name}" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" } `, name, name) } From eb6a4e3f739f421785c5cb6d6c23e98693de2671 Mon Sep 17 00:00:00 2001 From: "xiaowei.wang" Date: Mon, 12 Mar 2018 13:48:40 +0100 Subject: [PATCH 4/5] resource/autoscaling_policy: raise error on unsupported arguments --- aws/resource_aws_autoscaling_policy.go | 56 +++++++++++++++++--------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/aws/resource_aws_autoscaling_policy.go b/aws/resource_aws_autoscaling_policy.go index 8c4f2159678..1289667cbc8 100644 --- a/aws/resource_aws_autoscaling_policy.go +++ b/aws/resource_aws_autoscaling_policy.go @@ -186,11 +186,11 @@ func resourceAwsAutoscalingPolicyCreate(d *schema.ResourceData, meta interface{} autoscalingconn := meta.(*AWSClient).autoscalingconn params, err := getAwsAutoscalingPutScalingPolicyInput(d) + log.Printf("[DEBUG] AutoScaling PutScalingPolicy on Create: %#v", params) if err != nil { return err } - log.Printf("[DEBUG] AutoScaling PutScalingPolicy: %#v", params) resp, err := autoscalingconn.PutScalingPolicy(¶ms) if err != nil { return fmt.Errorf("Error putting scaling policy: %s", err) @@ -241,11 +241,11 @@ func resourceAwsAutoscalingPolicyUpdate(d *schema.ResourceData, meta interface{} autoscalingconn := meta.(*AWSClient).autoscalingconn params, inputErr := getAwsAutoscalingPutScalingPolicyInput(d) + log.Printf("[DEBUG] AutoScaling PutScalingPolicy on Update: %#v", params) if inputErr != nil { return inputErr } - log.Printf("[DEBUG] Autoscaling Update Scaling Policy: %#v", params) _, err := autoscalingconn.PutScalingPolicy(¶ms) if err != nil { return err @@ -296,13 +296,23 @@ func getAwsAutoscalingPutScalingPolicyInput(d *schema.ResourceData) (autoscaling } // This parameter is supported if the policy type is SimpleScaling. - if v, ok := d.GetOkExists("cooldown"); ok && policyType == "SimpleScaling" { + if v, ok := d.GetOkExists("cooldown"); ok { + // 0 is allowed as placeholder even if policyType is not supported params.Cooldown = aws.Int64(int64(v.(int))) + if v.(int) != 0 && policyType != "SimpleScaling" { + return params, fmt.Errorf("cooldown is only supported for policy type SimpleScaling") + } } // This parameter is supported if the policy type is StepScaling or TargetTrackingScaling. - if v, ok := d.GetOkExists("estimated_instance_warmup"); ok && (policyType == "StepScaling" || policyType == "TargetTrackingScaling") { - params.EstimatedInstanceWarmup = aws.Int64(int64(v.(int))) + if v, ok := d.GetOkExists("estimated_instance_warmup"); ok { + // 0 is NOT allowed as placeholder if policyType is not supported + if policyType == "StepScaling" || policyType == "TargetTrackingScaling" { + params.EstimatedInstanceWarmup = aws.Int64(int64(v.(int))) + } + if v.(int) != 0 && policyType != "StepScaling" && policyType != "TargetTrackingScaling" { + return params, fmt.Errorf("estimated_instance_warmup is only supported for policy type StepScaling and TargetTrackingScaling") + } } // This parameter is supported if the policy type is StepScaling. @@ -320,34 +330,40 @@ func getAwsAutoscalingPutScalingPolicyInput(d *schema.ResourceData) (autoscaling // This parameter is required if the policy type is SimpleScaling and not supported otherwise. //if policy_type=="SimpleScaling" then scaling_adjustment is required and 0 is allowed - if policyType == "SimpleScaling" { - v, ok := d.GetOkExists("scaling_adjustment") - if !ok { - return params, fmt.Errorf("scaling_adjustment is required for policy type SimpleScaling") + if v, ok := d.GetOkExists("scaling_adjustment"); ok { + // 0 is NOT allowed as placeholder if policyType is not supported + if policyType == "SimpleScaling" { + params.ScalingAdjustment = aws.Int64(int64(v.(int))) + } + if v.(int) != 0 && policyType != "SimpleScaling" { + return params, fmt.Errorf("scaling_adjustment is only supported for policy type SimpleScaling") } - params.ScalingAdjustment = aws.Int64(int64(v.(int))) + } else if !ok && policyType == "SimpleScaling" { + return params, fmt.Errorf("scaling_adjustment is required for policy type SimpleScaling") } // This parameter is required if the policy type is StepScaling and not supported otherwise. - if policyType == "StepScaling" { - v, ok := d.GetOk("step_adjustment") - if !ok { - return params, fmt.Errorf("step_adjustment is required for policy type StepScaling") - } + 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 len(steps) != 0 && policyType != "StepScaling" { + return params, fmt.Errorf("step_adjustment is only supported for policy type StepScaling") + } + } else if !ok && policyType == "StepScaling" { + return params, fmt.Errorf("step_adjustment is required for policy type StepScaling") } // This parameter is required if the policy type is TargetTrackingScaling and not supported otherwise. - if policyType == "TargetTrackingScaling" { - v, ok := d.GetOk("target_tracking_configuration") - if !ok { - return params, fmt.Errorf("target_tracking_configuration is required for policy type TargetTrackingScaling") - } + if v, ok := d.GetOk("target_tracking_configuration"); ok { params.TargetTrackingConfiguration = expandTargetTrackingConfiguration(v.([]interface{})) + if policyType != "TargetTrackingScaling" { + return params, fmt.Errorf("target_tracking_configuration is only supported for policy type TargetTrackingScaling") + } + } else if !ok && policyType == "TargetTrackingScaling" { + return params, fmt.Errorf("target_tracking_configuration is required for policy type TargetTrackingScaling") } return params, nil From f7486935aa24380c2800942cf7e9e9d9f312e351 Mon Sep 17 00:00:00 2001 From: "xiaowei.wang" Date: Mon, 12 Mar 2018 13:52:26 +0100 Subject: [PATCH 5/5] resource/autoscaling_policy: add test cases for #3689 --- aws/resource_aws_autoscaling_policy_test.go | 55 ++++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_autoscaling_policy_test.go b/aws/resource_aws_autoscaling_policy_test.go index 6989ca4ec22..6333cf96a54 100644 --- a/aws/resource_aws_autoscaling_policy_test.go +++ b/aws/resource_aws_autoscaling_policy_test.go @@ -43,6 +43,15 @@ func TestAccAWSAutoscalingPolicy_basic(t *testing.T) { resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "estimated_instance_warmup", "200"), resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "autoscaling_group_name", name), resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "step_adjustment.2042107634.scaling_adjustment", "1"), + testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_target_tracking", &policy), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "policy_type", "TargetTrackingScaling"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "name", name+"-foobar_target_tracking"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "autoscaling_group_name", name), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.#", "1"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.customized_metric_specification.#", "0"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.predefined_metric_specification.#", "1"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.predefined_metric_specification.0.predefined_metric_type", "ASGAverageCPUUtilization"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.target_value", "40"), ), }, { @@ -55,6 +64,13 @@ func TestAccAWSAutoscalingPolicy_basic(t *testing.T) { resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "policy_type", "StepScaling"), resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "estimated_instance_warmup", "20"), resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_step", "step_adjustment.997979330.scaling_adjustment", "10"), + testAccCheckScalingPolicyExists("aws_autoscaling_policy.foobar_target_tracking", &policy), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "policy_type", "TargetTrackingScaling"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.#", "1"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.customized_metric_specification.#", "1"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.customized_metric_specification.0.statistic", "Average"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.predefined_metric_specification.#", "0"), + resource.TestCheckResourceAttr("aws_autoscaling_policy.foobar_target_tracking", "target_tracking_configuration.0.target_value", "70"), ), }, }, @@ -346,7 +362,21 @@ resource "aws_autoscaling_policy" "foobar_step" { autoscaling_group_name = "${aws_autoscaling_group.test.name}" } -`, name, name) + +resource "aws_autoscaling_policy" "foobar_target_tracking" { + name = "%s-foobar_target_tracking" + policy_type = "TargetTrackingScaling" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" + + target_tracking_configuration { + predefined_metric_specification { + predefined_metric_type = "ASGAverageCPUUtilization" + } + + target_value = 40.0 + } +} +`, name, name, name) } func testAccAWSAutoscalingPolicyConfig_basicUpdate(name string) string { @@ -374,7 +404,28 @@ resource "aws_autoscaling_policy" "foobar_step" { autoscaling_group_name = "${aws_autoscaling_group.test.name}" } -`, name, name) + +resource "aws_autoscaling_policy" "foobar_target_tracking" { + name = "%s-foobar_target_tracking" + policy_type = "TargetTrackingScaling" + autoscaling_group_name = "${aws_autoscaling_group.test.name}" + + target_tracking_configuration { + customized_metric_specification { + metric_dimension { + name = "fuga" + value = "fuga" + } + + metric_name = "hoge" + namespace = "hoge" + statistic = "Average" + } + + target_value = 70.0 + } +} +`, name, name, name) } func testAccAWSAutoscalingPolicyConfig_upgrade_614(name string) string {