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

resource/aws_appautoscaling_policy: Recreate resource for resource_id updates and ignore ObjectNotFoundException on deletion #7982

Merged
merged 1 commit into from
Mar 20, 2019
Merged

resource/aws_appautoscaling_policy: Recreate resource for resource_id updates and ignore ObjectNotFoundException on deletion #7982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 16 additions & 3 deletions aws/resource_aws_appautoscaling_policy.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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