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

[853] Use live data for ECS task definitions for service pinning #876

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

erbridge
Copy link
Contributor

Trello card URL:

https://trello.com/c/nlsIBurV/853-fix-issue-with-terraform-ecs-task-pinning

Changes in this PR:

Terraform data resources fetch the current state of the live system, rather than relying on the Terraform state. This means we can create new task definitions outside of Terraform and have it
pick them up in a plan. Our build and deploy process does this, which was causing an issue where terraform applys were rolling back to earlier versions if there was no change to the task definition.

After merging this, we should never see changes to the task_definition for module.ecs.aws_ecs_service.* unless we're changing the connected task definition in the same change.

This may cause issues on a fresh system deployment, as there is no matching task definition at that point.

hashicorp/terraform#16380 (comment)

Next steps:

  • Terraform deployment required?

Terraform `data` resources fetch the current state of the live
system, rather than relying on the Terraform state. This means we
can create new task definitions outside of Terraform and have it
pick them up in a plan. Our build and deploy process does this,
which was causing an issue where `terraform apply`s were rolling
back to earlier versions if there was no change to the task
definition.

This may cause issues on a fresh system deployment, as there is no
matching task definition at that point.

hashicorp/terraform#16380 (comment)
@erbridge erbridge requested a review from pezholio April 23, 2019 16:24
@erbridge
Copy link
Contributor Author

Copy link
Contributor

@pezholio pezholio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a sensible way forward - is there any way we can set a default if for any reason we are setting up a fresh instance?

(Edit: Ah, just seen the comment)

@erbridge
Copy link
Contributor Author

We could do an override like we did in #865, but it adds complexity, and it won't help with the fresh instance. If we set up a fresh instance all of our data resources will cause problems anyway.

@erbridge erbridge merged commit c254bce into develop Apr 24, 2019
@erbridge erbridge deleted the fix/terraform-ecs-task-pinning branch April 24, 2019 08:59
@pezholio pezholio mentioned this pull request Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants