-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: Enable throughput & iops configs for managed node_groups #1584
feat: Enable throughput & iops configs for managed node_groups #1584
Conversation
…sing launch templates
@@ -27,7 +27,9 @@ The role ARN specified in `var.default_iam_role_arn` will be used by default. In | |||
| disk\_encrypted | Whether the root disk will be encrypyted. Requires `create_launch_template` to be `true` and `disk_kms_key_id` to be set | bool | false | | |||
| disk\_kms\_key\_id | KMS Key used to encrypt the root disk. Requires both `create_launch_template` and `disk_encrypted` to be `true` | string | "" | | |||
| disk\_size | Workers' disk size | number | Provider default behavior | | |||
| disk\_type | Workers' disk type. Require `create_launch_template` to be `true`| number | `gp3` | | |||
| disk\_type | Workers' disk type. Require `create_launch_template` to be `true`| string | Provider default behavior | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated since we don't seem to be setting it to gp3
as the default value, and the default by the provider is gp2
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template#volume_type
@daroga0002 could you please have a look at this PR? |
Also, wondering if should we set a Lines 51 to 59 in 8d4fdc7
And setting default values in the root terraform-aws-eks/modules/node_groups/locals.tf Lines 3 to 32 in 8d4fdc7
|
@daroga0002 @antonbabenko PTAL |
Hi @junaid-ali ! This PR is ok but it touches some black-belt magic I still don't feel comfortable merging right away. Let me do some other related updates (examples, docs, etc.) at the beginning of next week and then merge this one. Thank you for your understanding. |
@antonbabenko are you able to have a look at this again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@junaid-ali I have tested and everything working fine 🚀 , thank you for contribution 🥇
@antonbabenko lets merge this
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. |
PR o'clock
Description
Fixes: #1567
This PR adds support for setting disk throughput and iops when
create_launch_template
is set totrue
for managed node groups.Example usage:
Checklist