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

Use cheapest EC2 instance as default #14536

Merged
merged 3 commits into from
Mar 11, 2024

Conversation

ali92hm
Copy link
Contributor

@ali92hm ali92hm commented Oct 9, 2022

Description

Add documentation for using the AWS graviton instances that are generally cheaper.
More info on the motivation and context below, I'm also open to suggestions.

Motivation and Context

For existing AWS customers or the accounts that have used their first year "free credits", the cheapest EC2 option is the graviton (arm64) EC2 class with the smallest size t4g.nano. I used this instance size with spot instances for a couple of months for as low as $2/month (everything included)

Also added a section about the AWS promotion on t4g.small instances.

How Has This Been Tested?

Deployed an instance to my aws account.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation change

These changes do not modify any existing instances, but the default behavior for the new EC2 instances launched will be different as the default size and architecture have changed. This could be viewed as a breaking change.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@CLAassistant
Copy link

CLAassistant commented Oct 9, 2022

CLA assistant check
All committers have signed the CLA.

@glennschler
Copy link
Contributor

glennschler commented Feb 14, 2023

I agree it is a good idea, though some may want to fully understand they are using wireguard on ARM. I do not recall the complete history, though feel it has been available for only a few years vs Intel. At least with Ubuntu.

Secondly, AWS is on the 3rd year free trial of one t4g.small for everyone, regardless if your 12-month free trial is over. This is what I have been using as one full time reserved instance, and then spot instances for others. https://aws.amazon.com/ec2/faqs/#t4g-instances

@ali92hm
Copy link
Contributor Author

ali92hm commented Feb 15, 2023

Ahh that's good to know, sad that it will only go to the end of this year, but I can make the change to t4g.small instead.

Regarding the arm vs x86, did you just wanted me to add a note on the read me?

@glennschler
Copy link
Contributor

The t4g.small free promotion has been extended the past two Decembers, so it is possible AWS does that again this December for another free year.

When the Algo project was created, the default image size may have been chosen not only based on price, but also performance. I feel it is best to not change the default, but yes a document comment can explain there are other choices to get the best price, especially highlighting the new ARM64 choices which did not exist in the past.

@glennschler
Copy link
Contributor

I do not believe the default EC2 should be changed to t4g.nano. A document comment is a better idea to explain there are other choices to get the best price, especially highlighting the new t4g ARM64 choices which did not exist in the past.

@ali92hm
Copy link
Contributor Author

ali92hm commented Mar 5, 2023

Apologies, will make the change. It's been a little busy lately

@glennschler
Copy link
Contributor

Well I was not specific enough in my previous comments, plus It took some time to think about the issue. Still it's only my opinion which might not be the same as others.

@ali92hm
Copy link
Contributor Author

ali92hm commented Apr 23, 2023

@glennschler made the changes you suggested and updated the PR description

@glennschler
Copy link
Contributor

LGTM

@ali92hm
Copy link
Contributor Author

ali92hm commented May 6, 2023

I think I do need your approval :)

@ali92hm
Copy link
Contributor Author

ali92hm commented Mar 11, 2024

Resolved a conflict

@jackivanov jackivanov enabled auto-merge (squash) March 11, 2024 20:38
@jackivanov jackivanov merged commit ebf127d into trailofbits:master Mar 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants