Ignoring `load_balancer` is not always what one want for CD

[ohmer]

Issue related to: https://github.com/terraform-aws-modules/terraform-aws-ecs/blob/ccc0d3a0786de98d53857b08663dedf11833d789/modules/service/main.tf#L394C7-L394C20

As noted in https://github.com/terraform-aws-modules/terraform-aws-ecs/tree/master/docs#service-1 and changed by #81, calling the module with ignore_task_definition_changes for a service also ignores change related to the load_balancer attribute. The goal was to support blue/green deployment via CodePipeline.

I would like to support deployment via a simple service update. I use GitHub Actions with https://github.com/aws-actions/amazon-ecs-deploy-task-definition and manage the load balancer and service resources via Terraform.

In the description, it was considered to create an ignore_task_definition_changes variable as well as ignore_load_balancer_changes. In #81 (comment), it was dropped to keep things simple. To support my use case, I think the 4 cases (ignore 0, ignore lb, ignore task def, ignore both) should be implemented.

Happy to submit a PR if maintainers would like this done.

[bryantbiggs]

it is highly unlikely that we will support that because the number of resources would start to explode since the lifecycle block does not support variables

[ohmer]

I see 4 aws_ecs_service resources instead of 2, do you see more @bryantbiggs?

[bryantbiggs]

We have 2 currently, but the more permutations we add, this number will go up drastically. Having 4 even doesn't make sense

[ohmer]

OK, will tailor to my need via a fork. Thanks for looking into this @bryantbiggs.

[andrewedstrom]

Just want to +1 that addressing this would be extremely helpful.

I want to update my task definition with GitHub actions but am really not sure how to set up the load balancer if I use ignore_task_definition_changes, so I've had to use the workaround from the design doc instead.

[rkubik-hostersi]

@andrewedstrom what's the workaround?
From my side, also +1 to this issue as the load balancer config is unmanageable for now :(

[andrewedstrom]

@rkubik-hostersi The workaround (this is cribbed from the Service section of the design design doc) is to set ignore_task_definition_changes = false in your service, then dynamically pull the image tag from the AWS SSM Parameter Store.

Something like this:

data "aws_ssm_parameter" "app_image_tag" {
  name = "/my-app/image-tag"
}

data "aws_ecr_repository" "app" {
  name = "my-app-repository"
}

data "aws_ecr_image" "app" {
  repository_name = data.aws_ecr_repository.app.name
  image_tag       = data.aws_ssm_parameter.app_image_tag.value
}

module "ecs_service" {
  source = "terraform-aws-modules/ecs/aws//modules/service"

  # ... omitted for brevity

  container_definitions = {
    default = {
      name  = data.aws_ecr_repository.app.name
      image = "${data.aws_ecr_repository.app.repository_url}@${data.aws_ecr_image.app.id}"
      # ...
    }
  }

  # This is the default, but just to clarify on this example that we are NOT ignoring
  # task definition changes, and still allowing an external party to modify the image/tag
  # without conflicting with Terraform
  ignore_task_definition_changes = false
}

In order for this to work, your GitHub action to deploy must not only deploy an updated task definition, but also must write the image tag to the parameter store.

The one major downside of this workaround is that any time you make a change to your terraform, even if it doesn't touch the ECS service at all, terraform will overwrite your container task definitions. It doesn't cause any downtime or issues, but it is extra noise.

[ohmer]

How is that a workaround since there is still drift? Ok drift is less dangerous but still a drift in the state...

And you have to duplicate your task definition to make the drift not dangerous. My approach is to Terraform a dummy task definition and let devs update it since it lives with the application code in another repo.

I understand the maintainer preference to keep things simple and agree that static lifecycle blocks are annoying.
My preference remains a trivial fork to avoid of potentially unpleasant side effects and unnecessary mental burden.

[andrewedstrom]

@bryantbiggs You should probably see this discussion.

@ohmer I agree with you

[bryantbiggs]

I am following, but not 100% certain what the issue is. I think its less about the LB and more about deploying new image versions through means outside of Terraform and not having conflicts? Is that correct?

I am anxiously waiting for this weeks release for hashicorp/terraform-provider-aws#30154 which I hope will alleviate that. We are tracking that for v6.0 of this module

[andrewedstrom]

@bryantbiggs Thanks for the reply!

I just opened an issue about the problems we're having with the workaround, which hopefully can provide more clarity: #165. Basically, once we've run a deploy from GitHub actions, we see conflicts on every terraform plan, even when nothing has changed in the container_definitions. It sounds like hashicorp/terraform-provider-aws#30154 might help us avoid this?

We would just use ignore_task_definition_changes, however that's undesirable for us because we're not sure how to hook the load balancer up.

[bryantbiggs]

thank you - lets see what we can do with this update in the provider. its a 💩 situation that I wish ECS would handle a bit better on their end - everything we do here and in the provider is just hacks to work around the current API structure

On that note 😅 - please please PLEASE talk with your AWS account team to have them add PFR influence for this internally within AWS (PFR == product feature request). Thats the best way to get these things on the service teams radar

[andrewedstrom]

@bryantbiggs Thanks for the reply! I'm curious—what parts of this issue I'm describing are AWS's API, and what parts are terraform-aws-ecs? Just trying to understand what specifically I should bring up with our account team.

For instance, does the coupling between ignore_task_definition_changes and load balancing come from AWS's API?