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

Fix issue #72 task_definition known after apply #124

Closed
wants to merge 2 commits into from

Conversation

tomwj
Copy link

@tomwj tomwj commented May 13, 2020

Description

Using a data provider means terraform doesn't have the information required until runtime to determine if the task_definition needs to be updated. Subsequently an update is triggered on every apply. It has the information locally already, by removing the indirection of a data provider terraform can correctly reconcile if any updates are required prior to apply being run. Essentially it's making the dependency between the aws_ecs_service and aws_ecs_task_definition explicit rather than implied through the data provider and depends_on

https://www.terraform.io/docs/configuration/data-sources.html#data-resource-dependencies

Motivation and Context

Currently atlantis shows an update everytime a plan or apply is run despite no configuration changes.
#72

Breaking Changes

None that I'm aware of.

How Has This Been Tested?

  • Confirmed that updating the vars that are used in the task_definition trigger an update
  • Manually create a new task_definition revision and confirm that terraform will role back to the

AWS, plan/apply and manual changes

Using a data provider means terraform doesn't have the information required until runtime to determine if the `task_definition` needs to be updated. Subsequently an update is triggered on every `apply`. It has the information locally already, by removing the indirection of a data provider terraform can correctly reconcile if any updates are required prior to `apply` being run. Essentially it's making the dependency between the `aws_ecs_service` and `aws_ecs_task_definition` explicit rather than implied through the `data` provider and `depends_on`

https://www.terraform.io/docs/configuration/data-sources.html#data-resource-dependencies

Currently atlantis shows an update everytime a plan or apply is run despite o configuration changes.
terraform-aws-modules#72

None that I'm aware of.

- Confirmed that updating the vars that are used in the `task_definition` trigger an update
- Manually create a new task_definition revision and confirm that terraform will role back to the

AWS, plan/apply and manual changes
@antonbabenko
Copy link
Member

This PR is not going to work for the situation when a user deploys Atlantis once and then uses something else to update ECS Task Definition (eg, deploy new version of an image).

I remember that I and some other people I know used ecs-deploy or similar tools to do actual deployments. This way we were sure that all infrastructure was managed as code and the image was updating during CI/CD process. I may not remember all details now, but in general, this was the process. I don't remember what is the right approach to avoid what you are experiencing.

/cc @maartenvanderhoef @osterman Do you remember what is the right solution to achieve a clean plan with task_definition every time?

@maartenvanderhoef
Copy link

maartenvanderhoef commented May 13, 2020

Hi, what if you only drop the depends_on from the aws_ecs_task_definition datasource, IIRC depends_on inside datasources causes weird behaviour like this.

One of the workaround I can find here hashicorp/terraform#11806 would be:

data "aws_ecs_task_definition" "atlantis" {		
   task_definition = "${var.name}${replace(aws_ecs_task_definition.atlantis.arn, "/.*/", "")}"
}

or

data "aws_ecs_task_definition" "atlantis" {		
   task_definition = replace(aws_ecs_task_definition.atlantis.arn, "/.*/", var.name)
}

@domcleal
Copy link
Contributor

Hi, what if you only drop the depends_on from the aws_ecs_task_definition datasource, IIRC depends_on inside datasources causes weird behaviour like this.

I think depends_on is needed for the first apply of this module to work, as the data source will fail if the task definition didn't already exist. Unfortunately data sources that depend on resources will always cause this behaviour of showing up in plans.

This PR is not going to work for the situation when a user deploys Atlantis once and then uses something else to update ECS Task Definition (eg, deploy new version of an image).

I've opened #163 which keeps this working with a new input variable, is this a reasonable compromise do you think?

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants