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

LaunchTemplate support for MNG broken with v14.0.0 #1211

Closed
1 task done
philicious opened this issue Feb 1, 2021 · 14 comments
Closed
1 task done

LaunchTemplate support for MNG broken with v14.0.0 #1211

philicious opened this issue Feb 1, 2021 · 14 comments

Comments

@philicious
Copy link
Contributor

philicious commented Feb 1, 2021

I have issues

I'm submitting a...

  • bug report

What is the current behavior?

#1129 that got released with v14.0.0 breaks the LT-support for MNG that got added by me with #997 and 13.1 !

when using an LT, you cannot set instance-type on the MNG itself ! however this commit breaks the logic

8978997#diff-7a3fc6c7df17fda0c341e61255461bf1f149256a9ddf14d4a18ab6f020d08136

using v14.0.0, TF wrongly wants to add the instance_type to existing MNG with LT but then also fails while trying so.

InvalidRequestException: Cannot specify instance types in launch template and API request

If this is a bug, how to reproduce? Please include a code sample if relevant.

try to create or run on existing MNG with LT

What's the expected behavior?

no regression. behavior <14.0 is expected

Are you able to fix this problem and submit a PR? Link here if you have already.

Environment details

  • Affected module version: v14.0.0
  • OS: macOS 11.0.1
  • Terraform version: 0.14.5

Any other relevant info

@psoares
Copy link

psoares commented Feb 1, 2021

Can you try with my branch? See #1161. It achieves the same goal as the PR that was merged without renaming the instance_type property.

@philicious
Copy link
Contributor Author

@psoares on a quick glance your PR looks more promising.
I'll give it a shot tomorrow.

@philicious
Copy link
Contributor Author

@psoares I can confirm that your branch / ( #1161 ) is not affected and properly treats the instance_type on the MNG in case a LT is set.

@ado120
Copy link

ado120 commented Mar 1, 2021

I would like to add that I am also running into this issue, set a instance type in a LT as well as under node_groups and am getting the same error

@philicious
Copy link
Contributor Author

@ado120 if I understand correctly: you set instance_type on both LT and the node_group block itself.
So that is forbidden. You can only set in either.

However, without this fix here, you still get an error if you only set in LT as the module wrongly adds a default value then in the node_group

@ado120
Copy link

ado120 commented Mar 2, 2021

@philicious Got it. Yep at first I had it set in both locations. However I then set it to only the LT and still getting the same error. I guess for now just take out the instance type in the LT and add it in var.node_groups till the issue is solved?

@philicious
Copy link
Contributor Author

@ado120 well, I set instance_type on the LT but pinned the module to version = "<14.0.0" but setting it only on the node_group should also work and w/o pinning according to

If you deploy a node group using a launch template, then you can specify zero or one Instance type under Launch template contents in a launch template or you can specify 0-20 instance types for Instance types on the Set compute and scaling configuration page in the console, or using other tools that use the Amazon EKS API

@pmargarc
Copy link

pmargarc commented Apr 9, 2021

I just ran into the mentioned exception today. What should be the best solution to the problem?
I checked the module's docs and didn't find anything about instance type attribute in launch_template block for aws_eks_node_group.
:) thank you

@philicious
Copy link
Contributor Author

@pmargarc set instance_type either in the LT or in the node_groups { ..} block.
you also need to pin the module version version = "<14.0.0" in case you set instance_type in the LT

@pmargarc
Copy link

Solved!! It didn't have anything to do with that finally :)
Somehow instance_type was being defined in 2 files at the same time (launchtemplate.tf and node_groups.tf). That was generating the issue.

Btw, specifying version = "<14.0.0" got me this error -> "Invalid kubernetes version <14.0.0" and had to switch to the original settings (version = lookup(each.value, "version", null).

Thanks a lot 👍

@philicious
Copy link
Contributor Author

fixed by #1138

@jai
Copy link

jai commented Jan 30, 2022

I'm experiencing this issue in 17.1.0 - not sure if it's a regression?

@philicious
Copy link
Contributor Author

@jai it should not happen anymore.

  • make sure you only set instance_type on either the Launchtemplate or the node_group, not both.
  • try updating module to a newer or latest 17.x

If that doesn't help, maybe you can share your EKS code

@github-actions
Copy link

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.

@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 a pull request may close this issue.

5 participants