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.

Sign up for GitHub

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

Jump to bottom

Adding support for launch templates & userdata parameters #27

Merged
merged 17 commits into from
Aug 29, 2020
Merged

Adding support for launch templates & userdata parameters #27

merged 17 commits into from
Aug 29, 2020

Conversation

danjbh
Copy link
Contributor

@danjbh danjbh commented Aug 28, 2020 •
edited by Nuru
Loading

what

  • Adding default launch template configuration
  • Adding ability to provide your own launch template by overriding the launch template id & version
  • Adding dynamic config options for user_data
  • Bumping various upstream module versions & tests
  • Keeping instance_types as a list but adding TF 0.13 variable validation

why

In previous version of the AWS provider (2.x), you could not define your own launch template for aws_eks_node_group. Additionally, the tags specified in the aws_eks_node_group definition were not being passed down to the EC2 instances created by the ASG, which made tasks like monitoring and cost tracking difficult.

The latest versions of the AWS provider (3.x) give us the ability to specify our own launch template directly from aws_eks_node_group, which allows us to set our own options (e.g. tag_specifications. user_data, etc.).

This also should satisfy the requests in #24

references

@danjbh danjbh requested review from a team as code owners August 28, 2020 12:51
@danjbh danjbh self-assigned this Aug 28, 2020
@danjbh danjbh requested review from adamcrews and aknysh and removed request for a team August 28, 2020 12:51
danjbh
danjbh commented Aug 28, 2020
View reviewed changes
README.md Outdated Show resolved Hide resolved
danjbh
danjbh commented Aug 28, 2020
View reviewed changes
README.md Show resolved Hide resolved
danjbh
danjbh commented Aug 28, 2020
View reviewed changes
main.tf Show resolved Hide resolved
@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

/test all

@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

/rebuild-readme

@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

/test all

@danjbh danjbh mentioned this pull request Aug 28, 2020
Closed
dotCipher
dotCipher reviewed Aug 28, 2020
View reviewed changes
Copy link
Contributor

@dotCipher dotCipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly some questions, but looks great

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

I still need to port over the logic from terraform-aws-eks-workers to parameterize userdata, so I'm going to move this to draft.

@danjbh danjbh marked this pull request as draft August 28, 2020 15:04
@danjbh
- Reverting to using 'instance_types' but w/ validation
6302b90 
- Bumping project to TF 13 so that we can use variable validation
- Adding dynamic userdata template
@danjbh danjbh requested a review from htplbc August 28, 2020 17:14
@danjbh danjbh marked this pull request as ready for review August 28, 2020 17:16
@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

/test all

@danjbh danjbh added the terraform/0.13 Module requires Terraform 0.13 or later label Aug 28, 2020
@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

/test all

@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

/test all

@dotCipher
Copy link
Contributor

This will require TF 0.13 right?

@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

This will require TF 0.13 right?

Yep yep.. we had a discussion about it and the preference was to go w/ 0.13 and use the new validation functionality basically.

@Gowiem Gowiem self-requested a review August 28, 2020 21:58
@Gowiem
Copy link
Member

Gowiem commented Aug 28, 2020

This is looking awesome @danjbh! Went down the rabbit hole of trying to figure why my node-group instances didn't have name tags and stumbled across this PR, so I'm stoked for this to land. If my project was already on 0.13 I'd give it a test run and then give you a review, but without being able to test this many changes I'll hold off for a bit until I can give it a real pass.

@danjbh danjbh requested a review from Nuru August 28, 2020 22:31
@danjbh
Copy link
Contributor Author

danjbh commented Aug 28, 2020

/test all

Nuru
Nuru requested changes Aug 29, 2020
View reviewed changes
userdata.tpl Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
danjbh and others added 6 commits August 28, 2020 18:38
@danjbh @Nuru
Removing apieserver-endpoint from bootstraph.sh
310d66f 
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@danjbh @Nuru
Removing cluster_endpoint variable definition
1670287 
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@danjbh @Nuru
Removing cluster_endpoint from userdata_vars
e6c3bd2 
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@danjbh
Removing cluster_endpoint references from complete example
94a3746
@danjbh @Nuru
Removing tags input from label module
1770c5a 
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@danjbh
Updating README
0c1744e
@danjbh
Copy link
Contributor Author

danjbh commented Aug 29, 2020

/test all

@danjbh danjbh changed the title Adding support for launch templates & user_data override Adding support for launch templates & user_data parameters Aug 29, 2020
@danjbh danjbh changed the title Adding support for launch templates & user_data parameters Adding support for launch templates & userdata parameters Aug 29, 2020
@danjbh danjbh requested a review from Nuru August 29, 2020 02:14
@danjbh danjbh merged commit 842e0a6 into cloudposse:master Aug 29, 2020
@danjbh danjbh deleted the add-launch-template-support branch August 29, 2020 03:18
xario pushed a commit to xario/terraform-aws-eks-node-group that referenced this pull request Aug 31, 2020
@danjbh @xario
Adding support for launch templates & userdata parameters (cloudposse#27
f674352 
)
@xario xario mentioned this pull request Aug 31, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotCipher dotCipher dotCipher left review comments

@Nuru Nuru Nuru approved these changes

@adamcrews adamcrews Awaiting requested review from adamcrews

@aknysh aknysh Awaiting requested review from aknysh

@htplbc htplbc Awaiting requested review from htplbc

@Gowiem Gowiem Awaiting requested review from Gowiem

Assignees

@danjbh danjbh

Labels
terraform/0.13 Module requires Terraform 0.13 or later
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@danjbh @dotCipher @Gowiem @Nuru @actions-bot