-
-
Notifications
You must be signed in to change notification settings - Fork 627
feat: Add new vpc_id input #353
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
base: master
Are you sure you want to change the base?
feat: Add new vpc_id input #353
Conversation
Sometimes, it happens that Terraform tries to recreate the security group of the ECS service whereas the VPC did not actually change. To avoid this issue, let's use the dependency inversion principle (described here https://developer.hashicorp.com/terraform/language/modules/develop/composition#dependency-inversion) by passing the VPC ID as an input.
81eac13
to
4d16e1f
Compare
Hey @antonbabenko 👋 I submitted this PR to improve the service module. When you have a few minutes, it'd be super cool to have your review 😸 Thanks a lot. |
Do you have a reproduction of the issue? |
Hi @bryantbiggs 👋 Yeah, this issue is triggered when you upgrade from AWS provider v5 to v6. I was able to reproduce the issue from this repository on one of my accounts with
|
you can work around that with: terraform apply -target='module.ecs_service.data.aws_subnet.this[0]' \
-target="module.ecs_service.data.aws_caller_identity.current" \
-target="module.ecs_service.data.aws_partition.current" \
-target="module.ecs_service.data.aws_region.current" \
-target="module.ecs_task_definition.data.aws_caller_identity.current" \
-target="module.ecs_task_definition.data.aws_partition.current" \
-target="module.ecs_task_definition.data.aws_region.current" that ensures any data sources have been updated before proceeding with resource updates. data sources are updated due to the new AWS provider version ( |
to be clear, you are migrating across a major version of both the module as well as the AWS provider which both are breaking changes. we aim to minimize disruption as much as possible but it is not guaranteed - some manual intervention may be required during upgrades |
This may be due to the migration to version 6 in this case but we had this bug before with AWS provider v5 and we had to fork this repository to fix the issue by adding a vpc_id input. I'll try to provide you a reproduction of this problem. |
I also experiment the issue sporadically, but unsure what causes it (I thought it could be due to the ordering of It is slightly annoying when it happens as terraform re-creates the ECS service as well. |
Sometimes, it happens that Terraform tries to recreate the security group of the ECS service whereas the VPC did not actually change.
To avoid this issue, let's use the dependency inversion principle (described here https://developer.hashicorp.com/terraform/language/modules/develop/composition#dependency-inversion) by passing the VPC ID as an input.
Before this MR, here is what could happen during the plan:
Because Terraform needs the datasource
aws_subnet
to get the VPC ID, it forces the replacement of theaws_security_group
. Passing the VPC ID as input of the module fixes this issue.How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request