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

ec2-test-framework enhancements and graviton 4 testing #1715

Merged
merged 5 commits into from
Jul 26, 2024
Merged

ec2-test-framework enhancements and graviton 4 testing #1715

merged 5 commits into from
Jul 26, 2024

Conversation

samuel40791765
Copy link
Contributor

@samuel40791765 samuel40791765 commented Jul 19, 2024 •
edited
Loading

Issues:

Resolves P131897680

Description of changes:

  1. Some of our ec2 instances from the ec2-test-framework were failing to be properly stopped. This adds logic to the job pruner to stop hanging instances.
  2. Also parallelizing the test runs. GV2 is significantly slower which causes the run to be much longer. This also makes it much easier for us to extend testing against new test scripts.
  3. Also added graviton 4 testing to the ec2-test-framework.

Call-outs:

N/A

Testing:

Local fork CI run: samuel40791765#33

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@samuel40791765 samuel40791765 requested a review from skmcgrail July 19, 2024 22:51
@samuel40791765 samuel40791765 requested a review from a team as a code owner July 19, 2024 22:51
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.36%. Comparing base (98ccf4a) to head (76a70c0).
Report is 55 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1715   +/-   ##
=======================================
  Coverage   78.35%   78.36%           
=======================================
  Files         573      573           
  Lines       96065    96123   +58     
  Branches    13764    13781   +17     
=======================================
+ Hits        75270    75325   +55     
+ Misses      20195    20194    -1     
- Partials      600      604    +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andrewhop
andrewhop reviewed Jul 22, 2024
View reviewed changes
tests/ci/cdk/cdk/components.py Outdated Show resolved Hide resolved
tests/ci/lambda/src/bin/purge-stale-builds.rs Outdated
for reservation in ec2_describe_response.unwrap().reservations() {
log::info!("Checking Instance {:?}", reservation.instances());
for instance in reservation.instances() {
// Prune ec2 instances hanging more than 120 minutes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this also handle cleaning up duplicate instances in the case of second push to the same PR creating a new job and the results of the first job being unused?

Copy link
Member

Choose a reason for hiding this comment

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

To add to this, we should probably use tags on the instance to indicate the metadata about what pull request its related to, what the commit head is when you launch it with run-instances, and tag it with something to indicate that it is part of the ec2-test-framework. Then your filter here can be to look for that indicator tag, and then you can use the other tags with the PR number, head commit etc to determine if an instance should be nuked cause its an old push.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback Sean! Yeah it would be a bit more to support this, but doable.
I was planning on adding the CODEBUILD_SOURCE_VERSION(which contains the commit hash) to the ec2 tags, then reuse a bit of your PR commit logic above to terminate the right instances.
I guess that coincides with your proposal, do let me know if there's any part of the puzzle I'm missing though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got things working and the ec2 instances are now pruned at the same pace as our Codebuild jobs.

tests/ci/cdk/cdk/codebuild/ec2_test_framework_omnibus.yaml Show resolved Hide resolved
@samuel40791765 samuel40791765 force-pushed the ec2-job-pruner branch 4 times, most recently from 04df854 to f57697f Compare July 23, 2024 22:45
@samuel40791765 samuel40791765 force-pushed the ec2-job-pruner branch 5 times, most recently from 2b859e6 to 009f925 Compare July 24, 2024 01:13
skmcgrail
skmcgrail previously approved these changes Jul 24, 2024
View reviewed changes
@samuel40791765 samuel40791765 force-pushed the ec2-job-pruner branch 2 times, most recently from ef78581 to f947c39 Compare July 25, 2024 00:25
@samuel40791765 samuel40791765 changed the title Allow job pruner to prune ec2 instances and parallelize test runs ec2-test-framework enhancements and graviton 4 testing Jul 25, 2024
@samuel40791765 samuel40791765 merged commit 916b3d1 into aws:main Jul 26, 2024
104 checks passed
@skmcgrail skmcgrail mentioned this pull request Aug 1, 2024
Merged
skmcgrail added a commit that referenced this pull request Aug 1, 2024
@skmcgrail
Prepare Release for v1.33.0 (#1734)
a265ac9 
## What's Changed
* Added options to x509 tool by @ecdeye in
#1696
* Add support to detect Neoverse V2 cores by @andrewhop in
#1706
* Move OCSP functions for Ruby out of internal.h by @samuel40791765 in
#1704
* Add aes-256-xts to EVP_get_cipherbyname by @torben-hansen in
#1707
* Match using CMAKE_SYSTEM_PROCESSOR_LOWER by @justsmth in
#1709
* Update MySQL to 9.0.0 by @skmcgrail in
#1685
* [EC] Unify scalar multiplication for P-256/384/521 by @dkostic in
#1693
* Adds const qualifier to ciphertext parameter in EVP_PKEY_decapsulate
by @maddeleine in #1713
* Upstream merge 2024 06 24 by @nebeid in
#1661
* NIST SP 800-108r1-upd1: KDF Counter Implementation by @skmcgrail in
#1644
* Upstream merge 2024 07 09 by @nebeid in
#1694
* Design for support of HMAC precomputed keys by @fabrice102 in
#1574
* Fix for select point from table in ec_nistp scalar_mul by @dkostic in
#1719
* X509toolcomparison by @ecdeye in
#1714
* AWS-LC s2n-bignum update 2024-07-22 by @dkostic in
#1718
* Add OpenVPN to CI by @smittals2 in
#1705
* Lower required Go version, add CI test for specific version by
@andrewhop in #1717
* ec2-test-framework enhancements and graviton 4 testing by
@samuel40791765 in #1715
* sha + chacha: Move AArch64/X86-64 dispatching to C. by @justsmth in
#1625
* Show number of pruned ec2 instances in dashboard by @samuel40791765 in
#1728
* rsa and md5 tools by @ecdeye in
#1722
* FIPS 203 IPD update: ML-KEM-IPD-768 and ML-KEM-IPD-1024 by @jakemas in
#1724
* bump mysql CI to 9.0.1 by @samuel40791765 in
#1727
* Support utility OCSP request functions by @samuel40791765 in
#1708
* add support for OCSP_SINGLERESP functions by @samuel40791765 in
#1703
This was referenced Aug 1, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lilpoozie2005 lilpoozie2005 lilpoozie2005 approved these changes

@andrewhop andrewhop andrewhop approved these changes

@skmcgrail skmcgrail skmcgrail approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@samuel40791765 @codecov-commenter @skmcgrail @andrewhop @lilpoozie2005