Skip to content

Commit

Permalink
Merge pull request #7982 from terraform-providers/b-aws_appautoscalin…
Browse files Browse the repository at this point in the history 
…g_policy-resource_id-ForceNew

resource/aws_appautoscaling_policy: Recreate resource for `resource_id` updates and ignore `ObjectNotFoundException` on deletion
  • Loading branch information
@bflad
bflad committed Mar 20, 2019
2 parents b8b3fcf + fce53d8 commit fd2dc0d
Show file tree
Hide file tree
Showing 3 changed files with 237 additions and 10 deletions.
19 changes: 16 additions & 3 deletions aws/resource_aws_appautoscaling_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func resourceAwsAppautoscalingPolicy() *schema.Resource {
"resource_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"scalable_dimension": {
Type: schema.TypeString,
Expand Down Expand Up @@ -368,17 +369,29 @@ func resourceAwsAppautoscalingPolicyDelete(d *schema.ResourceData, meta interfac
log.Printf("[DEBUG] Deleting Application AutoScaling Policy opts: %#v", params)
err = resource.Retry(2*time.Minute, func() *resource.RetryError {
_, err = conn.DeleteScalingPolicy(&params)

if isAWSErr(err, applicationautoscaling.ErrCodeFailedResourceAccessException, "") {
return resource.RetryableError(err)
}

if isAWSErr(err, applicationautoscaling.ErrCodeObjectNotFoundException, "") {
return nil
}

if err != nil {
if isAWSErr(err, applicationautoscaling.ErrCodeFailedResourceAccessException, "") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}
return nil
})

if isResourceTimeoutError(err) {
_, err = conn.DeleteScalingPolicy(&params)
}

if err != nil {
return fmt.Errorf("Failed to delete scaling policy: %s", err)
}

return nil
}

Expand Down
220 changes: 218 additions & 2 deletions aws/resource_aws_appautoscaling_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,28 @@ func TestAccAWSAppautoScalingPolicy_basic(t *testing.T) {
})
}

func TestAccAWSAppautoScalingPolicy_disappears(t *testing.T) {
var policy applicationautoscaling.ScalingPolicy
resourceName := "aws_appautoscaling_policy.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSAppautoscalingPolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSAppautoscalingPolicyConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAppautoscalingPolicyExists(resourceName, &policy),
testAccCheckAWSAppautoscalingPolicyDisappears(&policy),
),
ExpectNonEmptyPlan: true,
},
},
})
}

func TestAccAWSAppautoScalingPolicy_scaleOutAndIn(t *testing.T) {
var policy applicationautoscaling.ScalingPolicy

Expand Down Expand Up @@ -212,6 +234,40 @@ func TestAccAWSAppautoScalingPolicy_multiplePoliciesSameResource(t *testing.T) {
})
}

// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/7963
func TestAccAWSAppautoScalingPolicy_ResourceId_ForceNew(t *testing.T) {
var policy applicationautoscaling.ScalingPolicy
appAutoscalingTargetResourceName := "aws_appautoscaling_target.test"
resourceName := "aws_appautoscaling_policy.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSAppautoscalingPolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSAppautoscalingPolicyConfigResourceIdForceNew1(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAppautoscalingPolicyExists(resourceName, &policy),
resource.TestCheckResourceAttrPair(resourceName, "resource_id", appAutoscalingTargetResourceName, "resource_id"),
resource.TestCheckResourceAttrPair(resourceName, "scalable_dimension", appAutoscalingTargetResourceName, "scalable_dimension"),
resource.TestCheckResourceAttrPair(resourceName, "service_namespace", appAutoscalingTargetResourceName, "service_namespace"),
),
},
{
Config: testAccAWSAppautoscalingPolicyConfigResourceIdForceNew2(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAppautoscalingPolicyExists(resourceName, &policy),
resource.TestCheckResourceAttrPair(resourceName, "resource_id", appAutoscalingTargetResourceName, "resource_id"),
resource.TestCheckResourceAttrPair(resourceName, "scalable_dimension", appAutoscalingTargetResourceName, "scalable_dimension"),
resource.TestCheckResourceAttrPair(resourceName, "service_namespace", appAutoscalingTargetResourceName, "service_namespace"),
),
},
},
})
}

func testAccCheckAWSAppautoscalingPolicyExists(n string, policy *applicationautoscaling.ScalingPolicy) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
Expand All @@ -221,8 +277,10 @@ func testAccCheckAWSAppautoscalingPolicyExists(n string, policy *applicationauto

conn := testAccProvider.Meta().(*AWSClient).appautoscalingconn
params := &applicationautoscaling.DescribeScalingPoliciesInput{
ServiceNamespace: aws.String(rs.Primary.Attributes["service_namespace"]),
PolicyNames: []*string{aws.String(rs.Primary.ID)},
PolicyNames: []*string{aws.String(rs.Primary.ID)},
ResourceId: aws.String(rs.Primary.Attributes["resource_id"]),
ScalableDimension: aws.String(rs.Primary.Attributes["scalable_dimension"]),
ServiceNamespace: aws.String(rs.Primary.Attributes["service_namespace"]),
}
resp, err := conn.DescribeScalingPolicies(params)
if err != nil {
Expand All @@ -232,6 +290,8 @@ func testAccCheckAWSAppautoscalingPolicyExists(n string, policy *applicationauto
return fmt.Errorf("ScalingPolicy %s not found", rs.Primary.ID)
}

*policy = *resp.ScalingPolicies[0]

return nil
}
}
Expand All @@ -258,6 +318,23 @@ func testAccCheckAWSAppautoscalingPolicyDestroy(s *terraform.State) error {
return nil
}

func testAccCheckAWSAppautoscalingPolicyDisappears(policy *applicationautoscaling.ScalingPolicy) resource.TestCheckFunc {
return func(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).appautoscalingconn

input := &applicationautoscaling.DeleteScalingPolicyInput{
PolicyName: policy.PolicyName,
ResourceId: policy.ResourceId,
ScalableDimension: policy.ScalableDimension,
ServiceNamespace: policy.ServiceNamespace,
}

_, err := conn.DeleteScalingPolicy(input)

return err
}
}

func testAccAWSAppautoscalingPolicyConfig(rName string) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "test" {
Expand Down Expand Up @@ -676,3 +753,142 @@ resource "aws_appautoscaling_policy" "foobar_in" {
}
`, randClusterName, randPolicyNamePrefix, randPolicyNamePrefix)
}

func testAccAWSAppautoscalingPolicyConfigResourceIdForceNewBase(rName string) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "test" {
name = %[1]q
}
resource "aws_ecs_task_definition" "test" {
family = %[1]q
container_definitions = <<EOF
[
{
"name": "busybox",
"image": "busybox:latest",
"cpu": 10,
"memory": 128,
"essential": true
}
]
EOF
}
resource "aws_ecs_service" "test1" {
cluster = "${aws_ecs_cluster.test.id}"
deployment_maximum_percent = 200
deployment_minimum_healthy_percent = 50
desired_count = 0
name = "%[1]s-1"
task_definition = "${aws_ecs_task_definition.test.arn}"
}
resource "aws_ecs_service" "test2" {
cluster = "${aws_ecs_cluster.test.id}"
deployment_maximum_percent = 200
deployment_minimum_healthy_percent = 50
desired_count = 0
name = "%[1]s-2"
task_definition = "${aws_ecs_task_definition.test.arn}"
}
`, rName)
}

func testAccAWSAppautoscalingPolicyConfigResourceIdForceNew1(rName string) string {
return testAccAWSAppautoscalingPolicyConfigResourceIdForceNewBase(rName) + fmt.Sprintf(`
resource "aws_appautoscaling_target" "test" {
max_capacity = 4
min_capacity = 0
resource_id = "service/${aws_ecs_cluster.test.name}/${aws_ecs_service.test1.name}"
scalable_dimension = "ecs:service:DesiredCount"
service_namespace = "ecs"
}
resource "aws_appautoscaling_policy" "test" {
# The usage of depends_on here is intentional as this used to be a documented example
depends_on = ["aws_appautoscaling_target.test"]
name = %[1]q
resource_id = "service/${aws_ecs_cluster.test.name}/${aws_ecs_service.test1.name}"
scalable_dimension = "ecs:service:DesiredCount"
service_namespace = "ecs"
step_scaling_policy_configuration {
adjustment_type = "ChangeInCapacity"
cooldown = 60
metric_aggregation_type = "Average"
step_adjustment {
metric_interval_lower_bound = 0
scaling_adjustment = 1
}
}
}
resource "aws_cloudwatch_metric_alarm" "test" {
alarm_actions = ["${aws_appautoscaling_policy.test.arn}"]
alarm_name = %[1]q
comparison_operator = "LessThanOrEqualToThreshold"
evaluation_periods = "5"
metric_name = "CPUReservation"
namespace = "AWS/ECS"
period = "60"
statistic = "Average"
threshold = "0"
dimensions = {
ClusterName = "${aws_ecs_cluster.test.name}"
}
}
`, rName)
}

func testAccAWSAppautoscalingPolicyConfigResourceIdForceNew2(rName string) string {
return testAccAWSAppautoscalingPolicyConfigResourceIdForceNewBase(rName) + fmt.Sprintf(`
resource "aws_appautoscaling_target" "test" {
max_capacity = 4
min_capacity = 0
resource_id = "service/${aws_ecs_cluster.test.name}/${aws_ecs_service.test2.name}"
scalable_dimension = "ecs:service:DesiredCount"
service_namespace = "ecs"
}
resource "aws_appautoscaling_policy" "test" {
# The usage of depends_on here is intentional as this used to be a documented example
depends_on = ["aws_appautoscaling_target.test"]
name = %[1]q
resource_id = "service/${aws_ecs_cluster.test.name}/${aws_ecs_service.test2.name}"
scalable_dimension = "ecs:service:DesiredCount"
service_namespace = "ecs"
step_scaling_policy_configuration {
adjustment_type = "ChangeInCapacity"
cooldown = 60
metric_aggregation_type = "Average"
step_adjustment {
metric_interval_lower_bound = 0
scaling_adjustment = 1
}
}
}
resource "aws_cloudwatch_metric_alarm" "test" {
alarm_actions = ["${aws_appautoscaling_policy.test.arn}"]
alarm_name = %[1]q
comparison_operator = "LessThanOrEqualToThreshold"
evaluation_periods = "5"
metric_name = "CPUReservation"
namespace = "AWS/ECS"
period = "60"
statistic = "Average"
threshold = "0"
dimensions = {
ClusterName = "${aws_ecs_cluster.test.name}"
}
}
`, rName)
}
8 changes: 3 additions & 5 deletions website/docs/r/appautoscaling_policy.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ resource "aws_appautoscaling_target" "ecs_target" {
resource "aws_appautoscaling_policy" "ecs_policy" {
name = "scale-down"
policy_type = "StepScaling"
resource_id = "service/clusterName/serviceName"
scalable_dimension = "ecs:service:DesiredCount"
service_namespace = "ecs"
resource_id = "${aws_appautoscaling_target.ecs_target.resource_id}"
scalable_dimension = "${aws_appautoscaling_target.ecs_target.scalable_dimension}"
service_namespace = "${aws_appautoscaling_target.ecs_target.service_namespace}"
step_scaling_policy_configuration {
adjustment_type = "ChangeInCapacity"
Expand All @@ -70,8 +70,6 @@ resource "aws_appautoscaling_policy" "ecs_policy" {
scaling_adjustment = -1
}
}
depends_on = ["aws_appautoscaling_target.ecs_target"]
}
```

Expand Down

0 comments on commit fd2dc0d

Please sign in to comment.