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.

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

Terraform assumes aws_appautoscaling_policy names are globally unique #751

Closed
hashibot opened this issue Jun 13, 2017 · 7 comments · Fixed by #3012
Closed

Terraform assumes aws_appautoscaling_policy names are globally unique #751

hashibot opened this issue Jun 13, 2017 · 7 comments · Fixed by #3012
Labels
bug Addresses a defect in current functionality.
Milestone

Comments

@hashibot
Copy link

This issue was originally opened by @cbroglie as hashicorp/terraform#14302. It was migrated here as part of the provider split. The original body of the issue is below.


AWS only requires the policy name to be unique given a resource id, service namespace, and scalable dimension, but Terraform assumes the policy names are globally unique. This makes the behavior of aws_appautoscaling_policy non-deterministic when the same policy name is reused for different resources.

Terraform Version

v0.9.4

Affected Resource(s)

Please list the resources as a list, for example:

  • aws_appautoscaling_policy

Terraform Configuration Files

Our configuration leverages a module named ecs_autoscaling, and its contents are here:

variable "service_name" {}
variable "cluster_name" {}
variable "min" {}
variable "max" {}
variable "iam_role" {}

variable "cooldown" {
  default = 300
}

variable "metric_aggregation_type" {
  default = "Average"
}

variable "scale_up_adjustment" {
  default = 1
}

variable "scale_down_adjustment" {
  default = -1
}

resource "aws_appautoscaling_target" "ecs_target" {
  min_capacity = "${var.min}"
  max_capacity = "${var.max}"
  resource_id = "service/${var.cluster_name}/${var.service_name}"
  role_arn = "${var.iam_role}"
  scalable_dimension = "ecs:service:DesiredCount"
  service_namespace  = "ecs"
}

resource "aws_appautoscaling_policy" "ecs_policy_up" {
  depends_on = ["aws_appautoscaling_target.ecs_target"]

  adjustment_type = "ChangeInCapacity"
  cooldown = "${var.cooldown}"
  metric_aggregation_type = "${var.metric_aggregation_type}"
  name = "scale-up"
  resource_id = "service/${var.cluster_name}/${var.service_name}"
  scalable_dimension = "ecs:service:DesiredCount"
  service_namespace = "ecs"

  step_adjustment {
    metric_interval_lower_bound = 0
    scaling_adjustment = "${var.scale_up_adjustment}"
  }
}

resource "aws_appautoscaling_policy" "ecs_policy_down" {
  depends_on = ["aws_appautoscaling_target.ecs_target"]

  adjustment_type = "ChangeInCapacity"
  cooldown = "${var.cooldown}"
  metric_aggregation_type = "${var.metric_aggregation_type}"
  name = "scale-down"
  resource_id = "service/${var.cluster_name}/${var.service_name}"
  scalable_dimension = "ecs:service:DesiredCount"
  service_namespace = "ecs"

  step_adjustment {
    metric_interval_lower_bound = 0
    scaling_adjustment = "${var.scale_down_adjustment}"
  }
}

output "target_resource_id" {
  value = "${aws_appautoscaling_target.ecs_target.resource_id}"
}

output "scale_up_policy_arn" {
  value = "${aws_appautoscaling_policy.ecs_policy_up.arn}"
}

output "scale_down_policy_arn" {
  value = "${aws_appautoscaling_policy.ecs_policy_down.arn}"
}

Then the main configuration file leverages it like this:

module "ecs_autoscaling_example1" {
  source  = "modules/ecs_autoscaling"
  service_name = "example1"
  cluster_name = "${aws_ecs_cluster.cluster.name}"
  min = "${var.min}"
  max = "${var.max}"
  iam_role = "${var.ecs_iam_role}"
}

module "ecs_autoscaling_example2" {
  source  = "modules/ecs_autoscaling"
  service_name = "example2"
  cluster_name = "${aws_ecs_cluster.cluster.name}"
  min = "${var.min}"
  max = "${var.max}"
  iam_role = "${var.ecs_iam_role}"
}

Actual Behavior

Since Terraform assumes the names are globally unique, the behavior depends on the order of results from the call to applicationautoscaling.DescribeScalingPolicies.

Steps to Reproduce

Execute terraform apply multiple times and observe that changes are made continuously.

Important Factoids

The issue is getAwsAppautoscalingPolicy only passes the policy name and service namespace parameters to applicationautoscaling.DescribeScalingPolicies, and uses the first result which matches the policy name. It should also pass the resource id and scalable dimension parameters, and assert that it gets at most 1 result back.

@hashibot hashibot added the bug Addresses a defect in current functionality. label Jun 13, 2017
@bflad
Copy link
Contributor

bflad commented Nov 14, 2017

@cbroglie is this still an issue? I believe this might have been fixed in #1808 and released in provider version 1.1.0 (October 16, 2017).

@cbroglie
Copy link
Contributor

Unfortunately I've switched companies, and am no longer setup to test.

But from looking back through my description above, I think #1808 is only a partial fix, as it still does not include the resource id.

@mksm
Copy link

mksm commented Jan 16, 2018

This is still an issue. Having scaling policies with same name and attributes but for different resource_id causes terraform to keep wanting to update the aws_appautoscaling_policy's resource_id.

@bflad
Copy link
Contributor

bflad commented Jan 16, 2018

I have submitted a PR to address this: #3012

@bflad
Copy link
Contributor

bflad commented Jan 17, 2018

This has landed in master and will ship with v1.7.1 set to release later this week. Happy Terraform'ing! 🎉

@bflad
Copy link
Contributor

bflad commented Jan 22, 2018

This has been released in terraform-provider-aws version 1.7.1. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@ghost
Copy link

ghost commented Apr 8, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality.
Projects
None yet
4 participants