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

feat: Added custom AMI support for managed node groups #1473

Merged
merged 13 commits into from
Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions modules/node_groups/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ The role ARN specified in `var.default_iam_role_arn` will be used by default. In
| additional\_tags | Additional tags to apply to node group | map(string) | Only `var.tags` applied |
| ami\_release\_version | AMI version of workers | string | Provider default behavior |
| ami\_type | AMI Type. See Terraform or AWS docs | string | Provider default behavior |
| ami\_id | ID of custom AMI. If you use a custom AMI, you need to supply bootstrap script via user-data or as AMI built-in. | string | Provider default behavior |
| capacity\_type | Type of instance capacity to provision. Options are `ON_DEMAND` and `SPOT` | string | Provider default behavior |
| create_launch_template | Create and use a default launch template | bool | `false` |
| desired\_capacity | Desired number of workers | number | `var.workers_group_defaults[asg_desired_capacity]` |
Expand Down
8 changes: 5 additions & 3 deletions modules/node_groups/launch_template.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ data "cloudinit_config" "workers_userdata" {
content_type = "text/x-shellscript"
content = templatefile("${path.module}/templates/userdata.sh.tpl",
{
pre_userdata = each.value["pre_userdata"]
kubelet_extra_args = each.value["kubelet_extra_args"]
pre_userdata = each.value["pre_userdata"]
kubelet_extra_args = each.value["kubelet_extra_args"]
cluster_name = var.cluster_name
run_bootstrap_script = lookup(each.value, "ami_id", null) != null
}
)
}
Expand Down Expand Up @@ -64,7 +66,7 @@ resource "aws_launch_template" "workers" {
}

# if you want to use a custom AMI
# image_id = var.ami_id
image_id = lookup(each.value, "ami_id", null)

# If you use a custom AMI, you need to supply via user-data, the bootstrap script as EKS DOESNT merge its managed user-data then
# you can add more than the minimum code you see in the template, e.g. install SSM agent, see https://github.com/aws/containers-roadmap/issues/593#issuecomment-577181345
Expand Down
3 changes: 3 additions & 0 deletions modules/node_groups/templates/userdata.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@
${pre_userdata}

sed -i '/^KUBELET_EXTRA_ARGS=/a KUBELET_EXTRA_ARGS+=" ${kubelet_extra_args}"' /etc/eks/bootstrap.sh
%{ if run_bootstrap_script }
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is optional and rely on ami_id?

how this differ from amazon delivered AMI (how they launching bootstrap part?), asking as I never checked this and just using AWS AMIs

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't now exactly how it works, but I think that if you don't specify a custom AMI, then EKS merges your init scripts with their snippet that does the bootstrap process. However, when you use a custom AMI, they expect you to provide a fully-working init script that includes the bootstrapping section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, @jaimehrubiks is totally right. Here is my comment about this topic - #1473 (comment).

/etc/eks/bootstrap.sh ${cluster_name}
%{ endif }