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

fix: Regression broken MNG with LT, introduced in 14.0 #1221

Closed
wants to merge 1 commit into from
Closed

fix: Regression broken MNG with LT, introduced in 14.0 #1221

wants to merge 1 commit into from

Conversation

philicious
Copy link
Contributor

@philicious philicious commented Feb 3, 2021

PR o'clock

Description

fixes #1211 , introduced in #1129

Checklist

@philicious philicious changed the title Fix regression broken MNG with LT, introduced in 14.0 fix: Regression broken MNG with LT, introduced in 14.0 Feb 3, 2021
@philicious
Copy link
Contributor Author

cc @ArchiFleKs

@philicious
Copy link
Contributor Author

@barryib could have a look at this ? Cannot request reviews

@ArchiFleKs
Copy link
Contributor

Do you want me to include this in my PR @philicious or wait until this is merge to rebase ?

@dusansusic
Copy link

I tested this but it still wants to recreate my existing node groups with launch templates. Have you tested this, @philicious ? I will have more time during the weekend.

@philicious
Copy link
Contributor Author

@dusansusic its not a fix to prevent the recreation of MNGs. that might still happen because the addition of the capacity_type will cause them to be recreated. (but can be fixed up manually in TF-state if you feel lucky)

this PR fixes the situation where you are unable to either create or update MNGs at all , as the instance-type is set on the MNG by default, no matter if an LT is present, but you can only set it in either one of both

@philicious
Copy link
Contributor Author

@ArchiFleKs uhm, I guess your PR will take longer to be finished and eventually merged. so I would prefer my PR/fix to be standalone and with a higher chance of getting merged earlier. but of course feel free to use the fix already

@thehar
Copy link

thehar commented Feb 11, 2021

This regression is painful. Is there an ETA on shipping a patch?

@jcam
Copy link

jcam commented Feb 13, 2021

+1 what thehar said. I guess I'll force my systems to use the v13 module for now, though i'm building new clusters this week and it'd be nice to not have to recreate everything a zillion times

@barryib
Copy link
Member

barryib commented Apr 19, 2021

@philicious is this PR still needed ? If yes, can you please update your branch ?

@philicious
Copy link
Contributor Author

@barryib nope, fixed by #1138

@philicious philicious closed this Apr 19, 2021
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LaunchTemplate support for MNG broken with v14.0.0
6 participants