-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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 a panic that could occur if no ECS Cluster was found for a given … #3829
Conversation
This reminds me I should probably do the same thing for ECS service https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_ecs_service.go#L159 Is this now a preferred approach to deal with manual interventions? I'm just thinking it may cause strange situations when user chooses wrong set of credentials (different AWS account) or just wrong region and then wipes out the resource from state by just running If we accept such edge cases, then this LGTM. |
It seems heavy handed, doesn't it. We have this established behavior of "it's not found, mark it removed" in some other places, ideally so that they are then recreated, but it does seem... wrong here. At least that's how I felt. I'd like @phinze 's opinion here as well. |
Hey @radeksimko – I chatted with Paul a bit and we agree that the behavior here is correct. Regarding loss of state, those should be intact in the The main reasonsing here is that loss of resources like this, outside of the edge case of different credentials or changing a region, will prevent a user from moving forward with a plan. They'll be blocked until they remove the resource from the config or the statefile, or both. Thanks for bringing this up 😄 Will you be doing this kind of update for ECS service or should I pick that up? |
LGTM! |
provider/aws: Fix issue that could occur if no ECS Cluster was found for a give name
I can do the update, but not until #3828 gets merged, to avoid potential conflicts. 😉 |
I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
…cluster name
Fixes #3826 by looping the results and only setting if we find a match for that name