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

Remove AmazonEC2ContainerServiceAutoscaleRole creation #24

Closed
hectcastro opened this issue Apr 17, 2018 · 3 comments
Closed

Remove AmazonEC2ContainerServiceAutoscaleRole creation #24

hectcastro opened this issue Apr 17, 2018 · 3 comments
Assignees

Comments

@hectcastro
Copy link
Contributor

When the role created with AmazonEC2ContainerServiceAutoscaleRole is associated with an application autoscaling target ARN, subsequent Terraform plan/apply cycles flap as AWS overrides the role association with arn:aws:iam::949413123864:role/aws-service-role/ecs.application-autoscaling.amazonaws.com/AWSServiceRoleForApplicationAutoScaling_ECSService.

Consider replacing role creation with something like:

data "aws_iam_role" "ecs_autoscale_role" {
  name = "AWSServiceRoleForApplicationAutoScaling_ECSService"
}

Then output the ARN with data.aws_iam_role.ecs_autoscale_role.arn.

@hectcastro hectcastro added this to the Operations Sprint: 6/8-6/21 milestone Jun 6, 2018
@rbreslow
Copy link
Contributor

rbreslow commented Jun 15, 2018

I think I was able to replicate what you were experiencing:

-/+ module.app_web_service.aws_appautoscaling_target.main (new resource required)
      id: "service/ecsStagingCluster/StagingrockyTesting" => <computed> (forces new resource)
      max_capacity: "2" => "2"
      min_capacity: "1" => "1"
      resource_id: "service/ecsStagingCluster/StagingrockyTesting" => "service/ecsStagingCluster/StagingrockyTesting"
      role_arn: "arn:aws:iam::279682201306:role/aws-service-role/ecs.application-autoscaling.amazonaws.com/AWSServiceRoleForApplicationAutoScaling_ECSService" => "arn:aws:iam::279682201306:role/ecsStagingAutoscaleRole" (forces new resource)
      scalable_dimension: "ecs:service:DesiredCount" => "ecs:service:DesiredCount"
      service_namespace: "ecs" => "ecs"

Every Terraform plan/apply cycle attempts to change role_arn after AWS overrides it.

@rbreslow rbreslow self-assigned this Jun 15, 2018
@rbreslow
Copy link
Contributor

rbreslow commented Jun 15, 2018

Context for why this is happening:

You don't need to manually create the service-linked roles for Application Auto Scaling. Application Auto Scaling creates the appropriate service-linked role for you when you call RegisterScalableTarget or PutScalingPolicy. For example, if you set up automatic scaling for an ECS service, Application Auto Scaling creates the AWSServiceRoleForApplicationAutoScaling_ECSService role.

See:

@rbreslow
Copy link
Contributor

rbreslow commented Jun 15, 2018

hashicorp/terraform-provider-aws#2750 explores this issue and references hashicorp/terraform-provider-aws#2889 which made role_arn optional for aws_appautoscaling_target.

It was introduced in 1.7.0 of the AWS Terraform provider.

@hectcastro, your fix looks like the way to go. I will work on a PR and connect this issue.

Additionally, I'm going to open an issue on azavea/terraform-aws-ecs-web-service to consider this:

diff --git a/main.tf b/main.tf
index 50f75c2..d9dd9ac 100755
--- a/main.tf
+++ b/main.tf
@@ -98,7 +98,6 @@ resource "aws_appautoscaling_target" "main" {
   service_namespace  = "ecs"
   resource_id        = "service/${var.cluster_name}/${aws_ecs_service.main.name}"
   scalable_dimension = "ecs:service:DesiredCount"
-  role_arn           = "${var.ecs_autoscale_role_arn}"
   min_capacity       = "${var.min_count}"
   max_capacity       = "${var.max_count}"

I think this change could be controversial because of the potential fallout from those using outdated AWS providers. We should also start a conversation about bumping the AWS provider version for azavea/operations-app-template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants