-
-
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
feat!: Use launch_template_id
instead of launch_template_name
, raise MSV of AWS provider and Terraform to 5.0 and 1.0 respectively
#204
Conversation
08c537d
to
8e4561a
Compare
@antonbabenko What do you think about this fix? |
@bryantbiggs WDYT? |
it seems valid - I haven't had a chance to test before/after change to see if that introduces any interruption to current launch templates (i.e. - breaking change) |
I am back from vacation and ran a few tests. Create Auto Scaling Group based on version 6.5.2:
Change name of Launch Template based on version 6.5.2:
Change name of Launch Template (Error) based on version 6.5.2:
Change name of Launch Template (Success) based on branch launch_template_name:
As you can see, the pull request works as expected. Even if the diff still shows the old name of the launch template, the output has been updated accordingly.
|
8e4561a
to
c5cfa97
Compare
c5cfa97
to
adbc394
Compare
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 have not spent a significant amount of time reading and thinking about this PR, but I have a few comments:
- Changes to variables like rename are breaking changes that we should try to avoid.
- If necessary, we can use data-source
aws_launch_template
to get ID from the name (just as an idea). - We can add another variable and mark existing as deprecated so that we remove it during the next more significant update.
- It is ok to have 4 y.o. kid learning more about AWS/Terraform. Don't ask how I know about it :)
@antonbabenko I really tried to avoid introducing breaking changes, but right now I am out of ideas. The resource I also tried to use the data source I do not know how common it is for someone to change the name of a launch template or not manage the launch template with this Terraform module, but in the end this is a bug. I wonder how long it will take for your kid to work on the first Issues / Pull Requests. I am pretty sure you can use all the help you can get ;) |
This PR has been automatically marked as stale because it has been open 30 days |
…ws-autoscaling into launch_template_name
launch_template_id
instead of launch_template_name
, raise MSV of AWS provider and Terraform to 5.0 and 1.0 respectively
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.
thank you for your patience!
This PR is included in version 7.0.0 🎉 |
## [7.0.0](v6.10.0...v7.0.0) (2023-10-09) ### ⚠ BREAKING CHANGES * Use `launch_template_id` instead of `launch_template_name`, raise MSV of AWS provider and Terraform to 5.0 and 1.0 respectively (#204) ### Features * Use `launch_template_id` instead of `launch_template_name`, raise MSV of AWS provider and Terraform to 5.0 and 1.0 respectively ([#204](#204)) ([1d988c0](1d988c0))
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
launch_template_id
is used in place oflaunch_template_name
. If using an external launch template, you will need to now pass alaunch_template_id
instead oflaunch_template_name
v5.0
and Terraform raised tov1.0
scaling_policies
step_adjustment
argument accepted a single map and has since changed to accept multiple maps of scaling step adjustmentsIn case the value of
launch_template_name
changes, a new launch template will be created but the auto scaling group still points to the old launch template.If you execute
terraform apply
a second time, the auto scaling group tries to change the name of the launch template, but keeps the id of the old launch template which is not valid anymore.Using the
launch_template_id
instead oflaunch_template_name
everything is working as expected.Motivation and Context
Breaking Changes
launch_template_id
is used in place oflaunch_template_name
. If using an external launch template, you will need to now pass alaunch_template_id
instead oflaunch_template_name
v5.0
and Terraform raised tov1.0
scaling_policies
step_adjustment
argument accepted a single map and has since changed to accept multiple maps of scaling step adjustmentsHow Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request