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

docs: Use IRSA for Node Termination Handler IAM policy attachement in Instance Refresh example #1373

Merged
merged 1 commit into from
May 19, 2021
Merged

Conversation

stevehipwell
Copy link
Contributor

PR o'clock

Description

I've updated the example for how to use instance refresh.

  • Removed policy attachment to worker security group
  • Added spot instance termination support
  • Enable lifecycle for all worker groups
  • Rename Terraform resources to make it clearer

Checklist

@stevehipwell
Copy link
Contributor Author

/assign @barryib

@barryib barryib self-assigned this May 19, 2021
@barryib barryib changed the title fix: Update instance refresh example docs: Use IRSA for Node Termination Handler IAM policy attachement in Instance Refresh example May 19, 2021
Copy link
Member

@barryib barryib left a comment

Choose a reason for hiding this comment

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

Awesome ❤️ ! Thanks @stevehipwell for your contribution.

@barryib barryib merged commit 7c654a6 into terraform-aws-modules:master May 19, 2021
@bashims
Copy link
Contributor

bashims commented May 19, 2021

@stevehipwell I believe the example was already using IRSA according to aws-node-termination-handler's logs:

│ 2021/05/19 01:53:08 DBG AWS Credentials retrieved from provider: WebIdentityCredentials                                                             │
│ 2021/05/19 01:53:08 INF Started watching for interruption events                                                                                    │
│ 2021/05/19 01:53:08 INF Kubernetes AWS Node Termination Handler has started successfully!                                                             

Thanks for removing the unnecessary policy attachment!

@stevehipwell
Copy link
Contributor Author

@bashims it was but the policy was also attached to the worker SG, I assumed it was a typo. Thanks for the great work on getting the instance refresh integrated and released.

@bashims
Copy link
Contributor

bashims commented May 19, 2021

This change seems to break the instance_refresh example:

Error: Invalid for_each argument

  on main.tf line 220, in resource "aws_autoscaling_lifecycle_hook" "aws_node_termination_handler":
 220:   for_each = toset(module.eks.workers_asg_names)

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.

@stevehipwell stevehipwell deleted the instance-refresh-docs branch May 19, 2021 14:14
barryib pushed a commit to barryib/terraform-aws-eks that referenced this pull request May 20, 2021
ArchiFleKs pushed a commit to ArchiFleKs/terraform-aws-eks that referenced this pull request Jun 1, 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 14, 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.

4 participants