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

improvement: Make the cpu_credits optional for workers launch template #1030

Merged
merged 3 commits into from
Oct 5, 2020
Merged

improvement: Make the cpu_credits optional for workers launch template #1030

merged 3 commits into from
Oct 5, 2020

Conversation

gillg
Copy link
Contributor

@gillg gillg commented Oct 1, 2020

PR o'clock

Description

If you choose non bustable families in your template (c5 for example...) and you no remove credit_specification section, ASG can't start any instance.
Now you can set module cpu_credits option to null to made it fonctionnal.

Checklist

  • CI tests are passing

@gillg gillg changed the title Improvement: CPU credits could be removed from workers launch template improvement: CPU credits could be removed from workers launch template Oct 1, 2020
@barryib
Copy link
Member

barryib commented Oct 4, 2020

Thanks @gillg for opening this PR. I was wondering if setting the cpu_credit to null doesn't work without your change ?

@gillg
Copy link
Contributor Author

gillg commented Oct 4, 2020

Hi @barryib I tried it but it's an invalid value for aws provider API.
So the solution is to remove it

@barryib barryib changed the title improvement: CPU credits could be removed from workers launch template improvement: Make the CPU optional for workers launch template Oct 4, 2020
barryib
barryib suggested changes Oct 4, 2020
View reviewed changes
workers_launch_template.tf Show resolved Hide resolved
@barryib
Copy link
Member

barryib commented Oct 4, 2020

Hi @barryib I tried it but it's an invalid value for aws provider API.
So the solution is to remove it

Ok. Good to know. I just added one comment for the coding style. Just resolves that and we're good to go ^^.

@barryib
Copy link
Member

barryib commented Oct 4, 2020

Please rebase your branch to fix the CI.

@barryib barryib self-assigned this Oct 4, 2020
@barryib barryib added this to the v13.0.0 milestone Oct 4, 2020
@gillg
Copy link
Contributor Author

gillg commented Oct 4, 2020

Everything is ok !

barryib
barryib approved these changes Oct 5, 2020
View reviewed changes
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.

Thanks @gillg for your contribution.

@barryib barryib changed the title improvement: Make the CPU optional for workers launch template improvement: Make the cpu_credits optional for workers launch template Oct 5, 2020
@barryib barryib merged commit 93614b3 into terraform-aws-modules:master Oct 5, 2020
barryib pushed a commit to Polyconseil/terraform-aws-eks that referenced this pull request Oct 25, 2020
@gillg
improvement: Make the cpu_credits optional for workers launch templ…
f2aefd9 
…ate (terraform-aws-modules#1030)

NOTES: `credit_specification` for worker groups launch template can now be set to `null` so that we can use non burstable EC2 families
@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 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@barryib barryib barryib approved these changes

Assignees

@barryib barryib

Labels
None yet
Projects
None yet
Milestone
v13.0.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@gillg @barryib