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

run tests on ubuntu-24.04 and ubuntu-24.04-arm CI runners #1266

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jakob-keller
Copy link
Collaborator

@jakob-keller jakob-keller commented Jan 17, 2025

Description of Change

  • Run tests on ubuntu-24.04 and ubuntu-24.04-arm CI runners
  • Leverage matrix job definition to trigger upload of coverage results to codecov

Assumptions

CI on arm64 runners is currently flaky and is considered experimental. Such transient failures are not expected to affect the status of the CI run.

Checklist for All Submissions

  • I have added change info to CHANGES.rst
  • If this is resolving an issue (needed so future developers can determine if change is still necessary and under what conditions) (can be provided via link to issue with these details): closes [CI] Add linux arm64 to CI #1264
    • Detailed description of issue
    • Alternative methods considered (if any)
    • How issue is being resolved
    • How issue can be reproduced
  • If this is providing a new feature (can be provided via link to issue with these details):
    • Detailed description of new feature
    • Why needed
    • Alternatives methods considered (if any)

Checklist when updating botocore and/or aiohttp versions

  • I have read and followed CONTRIBUTING.rst
  • I have updated test_patches.py where/if appropriate (also check if no changes necessary)
  • I have ensured that the awscli/boto3 versions match the updated botocore version

@jakob-keller jakob-keller added the github_actions Pull requests that update GitHub Actions code label Jan 17, 2025
@jakob-keller jakob-keller self-assigned this Jan 17, 2025
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.45%. Comparing base (f13f103) to head (51b1628).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1266      +/-   ##
==========================================
- Coverage   87.52%   87.45%   -0.07%     
==========================================
  Files          67       67              
  Lines        5917     5917              
==========================================
- Hits         5179     5175       -4     
- Misses        738      742       +4     
Flag Coverage Δ
unittests 87.45% <ø> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@jakob-keller
Copy link
Collaborator Author

I'll pick this up again as soon as https://github.com/orgs/community/discussions/149347 is fixed.

@jakob-keller
Copy link
Collaborator Author

I'll pick this up again as soon as https://github.com/orgs/community/discussions/149347 is fixed.

This initial issue with ubuntu-24.04-arm has been fixed.

Now, it appears that the Lambda CI images used by us so far have not been maintained in 4 years and don't ship for arm64: https://hub.docker.com/r/lambci/lambda

We should probably modernize that part of our test suite ASAP.

@jakob-keller jakob-keller force-pushed the arm64-in-ci branch 2 times, most recently from 60b4e40 to b9a7111 Compare January 18, 2025 14:13
@jakob-keller
Copy link
Collaborator Author

We should probably modernize that part of our test suite ASAP.

#1268 fixes the issue related to outdated Lambda container images being used.

The new arm64 runners appear to be quite flaky when running the Lambda unit test. I don't think this is our fault and will give it a couple of days before trying again.

@jakob-keller jakob-keller force-pushed the arm64-in-ci branch 2 times, most recently from 8849614 to e399082 Compare January 19, 2025 11:00
thehesiod
thehesiod previously approved these changes Jan 19, 2025
@jakob-keller jakob-keller dismissed thehesiod’s stale review January 19, 2025 18:31

The merge-base changed after approval.

@thehesiod
Copy link
Collaborator

I'll pick this up again as soon as https://github.com/orgs/community/discussions/149347 is fixed.

This initial issue with ubuntu-24.04-arm has been fixed.

Now, it appears that the Lambda CI images used by us so far have not been maintained in 4 years and don't ship for arm64: https://hub.docker.com/r/lambci/lambda

We should probably modernize that part of our test suite ASAP.

I actually wrote the original implementation of lambda in moto ;) Haven't touched it in years. We should coordinate with them if there are some issues with it on arm.

@thehesiod
Copy link
Collaborator

aha per https://github.com/lambci/docker-lambda?tab=readme-ov-file they've swapped to the official images here: https://github.com/aws/aws-lambda-base-images

@jakob-keller
Copy link
Collaborator Author

I actually wrote the original implementation of lambda in moto ;) Haven't touched it in years. We should coordinate with them if there are some issues with it on arm.

I think, it's good. The issue was that we were nudging moto to use outdated container images for Lambda. But #1268 should take care of that.

Now it's just the flakiness of the GitHub arm runners that needs to get fixed.

@jakob-keller
Copy link
Collaborator Author

Now it's just the flakiness of the GitHub arm runners that needs to get fixed.

#1275 did not fix reliability of unit tests on arm64. It's still flaky...

@jakob-keller jakob-keller marked this pull request as ready for review January 28, 2025 08:22
@jakob-keller jakob-keller requested a review from webknjaz January 28, 2025 08:23
@jakob-keller
Copy link
Collaborator Author

Now it's just the flakiness of the GitHub arm runners that needs to get fixed.

#1275 did not fix reliability of unit tests on arm64. It's still flaky...

To work around flakiness of arm64 runners, I marked them as experimental for the time being. Experimental jobs that fail do not affect the result of the CI run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] Add linux arm64 to CI
2 participants