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: Correct issue where custom launch template is not used when EKS managed node group is used externally #1824

Merged

Conversation

bryantbiggs
Copy link
Member

@bryantbiggs bryantbiggs commented Feb 1, 2022

Description

  • correct issue where custom launch template is not used when EKS managed node group is used externally
  • update documentation and provide example of utilizing containerd runtime on EKS managed node group
  • update static AMI IDs in examples to use data source; ensures the current AMI is always pulled when testing
  • minor formatting for correct use of EOT and left aligning text

Motivation and Context

Breaking Changes

  • No

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects
    - Using the eks-managed-node-group example
    - eks-manged-node-group example was deployed as its defined today
    - Added in sub-module snippet provided in issue here Cannot use custom launch template when launch_template_name is not specified #1816
    - 2nd apply created new node group using external sub-module - confirmed that the node group was not using the custom launch template
    - Found issue and made correction on line 33 (see below) - re-ran plan and apply - only the external sub-module node group was affected and it was an update to use the custom launch template (desired affect)

@@ -30,7 +30,7 @@ module "user_data" {
################################################################################

locals {
use_custom_launch_template = var.launch_template_name != ""
use_custom_launch_template = var.create_launch_template || var.launch_template_name != ""
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the fix for ensuring the custom launch template is used when the sub-module is used externally

there are 4 scenarios here which is why it can be a bit weird to understand:

  1. var.create_launch_template = false && var.launch_template_name == "" => EKS MNG will use its own default LT
  2. var.create_launch_template = false && var.launch_template_name == "something" => User provided custom LT will be used
  3. var.create_launch_template = true && var.launch_template_name == "" => Custom LT will be used, module will provide a default name
  4. var.create_launch_template = true && var.launch_template_name == "something" => Custom LT will be used, LT name is provided by user

Copy link
Member

Choose a reason for hiding this comment

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

Maybe put your comment right above this line as a description for this "light magic"? It will be helpful to know the reason in some time.

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Minor comment. LGTM otherwise.

@@ -30,7 +30,7 @@ module "user_data" {
################################################################################

locals {
use_custom_launch_template = var.launch_template_name != ""
use_custom_launch_template = var.create_launch_template || var.launch_template_name != ""
Copy link
Member

Choose a reason for hiding this comment

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

Maybe put your comment right above this line as a description for this "light magic"? It will be helpful to know the reason in some time.

examples/self_managed_node_group/main.tf Show resolved Hide resolved
@antonbabenko antonbabenko merged commit e16b3c4 into terraform-aws-modules:master Feb 1, 2022
antonbabenko pushed a commit that referenced this pull request Feb 1, 2022
### [18.2.5](v18.2.4...v18.2.5) (2022-02-01)

### Bug Fixes

* Correct issue where custom launch template is not used when EKS managed node group is used externally ([#1824](#1824)) ([e16b3c4](e16b3c4))
@antonbabenko
Copy link
Member

This PR is included in version 18.2.5 🎉

@bryantbiggs bryantbiggs deleted the fix/containerd-docs branch February 1, 2022 17:45
baibailiha added a commit to baibailiha/terraform-aws-eks that referenced this pull request Sep 13, 2022
### [18.2.5](terraform-aws-modules/terraform-aws-eks@v18.2.4...v18.2.5) (2022-02-01)

### Bug Fixes

* Correct issue where custom launch template is not used when EKS managed node group is used externally ([#1824](terraform-aws-modules/terraform-aws-eks#1824)) ([926bde2](terraform-aws-modules/terraform-aws-eks@926bde2))
@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 10, 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.

Cannot use custom launch template when launch_template_name is not specified
2 participants