-
-
Notifications
You must be signed in to change notification settings - Fork 556
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: Ignore changes to load_balancers
and target_group_arns
#252
fix: Ignore changes to load_balancers
and target_group_arns
#252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to comprehend all the messages that were posted on this and linked issues and I don't understand WHY do we want to ignore changes in these additional arguments? They seem to be very important and it should have been addressed in the Terraform AWS provider, if their behavior is not consistent.
Am I missing something? Could you please explain briefly why this change is needed to be fixed in the module and not in provider?
@antonbabenko because each plan wants to remove asg instances from target groups. Next run will join it. This obviously causing downtime.
next run:
|
also, @antonbabenko and @bryantbiggs from this doc we can find:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the brief explanation, @dusansusic ! It is very clear to me now. Time to merge. 👌
### [7.3.1](v7.3.0...v7.3.1) (2023-11-30) ### Bug Fixes * Ignore changes to `load_balancers` and `target_group_arns` ([#252](#252)) ([ef8235c](ef8235c))
This PR is included in version 7.3.1 🎉 |
ya'll beat me to it 😅 - I was thinking about this a bit more last night and was going to add the |
I appreciate that my (but failed) pull request motivated you to make your pull request. |
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. |
Description
load_balancers
andtarget_group_arns
Motivation and Context
desired_capacity
andtarget_group_arns
#250Breaking Changes
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request