Skip to content

Add build for arm64 arch #657

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

Merged
merged 8 commits into from
Jan 31, 2023
Merged

Conversation

cedricziel
Copy link
Contributor

@cedricziel cedricziel commented Dec 23, 2022

Changes

This change introduces another architecture for the images created: arm64

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

@cedricziel cedricziel marked this pull request as ready for review December 23, 2022 11:03
@cedricziel cedricziel requested a review from a team December 23, 2022 11:03
@puckpuck
Copy link
Contributor

@austinlparker is this still needed? I thought we do multi-arch builds already.

@austinlparker
Copy link
Member

We do not do multi arch builds because it takes like 4+ hours.

@cedricziel
Copy link
Contributor Author

I have added it in particular because the featureflagservice keeps segfaulting on my M1 device with and without emulation.

I think it makes a lot of sense to have this available for multiple architectures. If the build time is a problem, it's probably a question of optimization

@TylerHelmuth
Copy link
Member

I also thought we fixed this but I just tried installing the helm chart locally again and featureflag service is crashing again (open-telemetry/opentelemetry-helm-charts#580).

Is there a way that we can do the existing build and publish process and separately build a specific arm64 image either after the existing build finishes or in parallel?

@cedricziel
Copy link
Contributor Author

Is there a way that we can do the existing build and publish process and separately build a specific arm64 image either after the existing build finishes or in parallel?

We sure could. Not sure if this benefits us. Currently all images are built in parallel and in each build sequentially builds and pushed amd64 and arm64. I would argue there is a fair amount of parallelism already in place.

Current duration for the Build & Push job is [19m 47s] - so overall pretty far away from the projected "4hrs" :)

@austinlparker
Copy link
Member

No, it takes about 4 hours: https://github.com/open-telemetry/opentelemetry-demo/actions/runs/3313405848

It might be better now with some of the optimizations we've made to build time but Rust and C++ images take forever to build under emulation.

I would suggest modifying the GHA runner file here to allow builds for PR's (you need to modify the release.yaml file to allow the build_and_push_images action to run on PR and add logic to disable pushing images on non-release builds) in order to see how long it takes.

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jan 24, 2023
@cartersocha cartersocha added v1.4 and removed Stale labels Jan 30, 2023
@austinlparker
Copy link
Member

I went ahead and tested this out on our runner config -- everything builds OK. Shipping service takes an hour which isn't great, but we can work on getting that down.

@cartersocha cartersocha merged commit ea64998 into open-telemetry:main Jan 31, 2023
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* Add build for arm64 arch

* Add change to changelog

* enable build on PR

* remove build on pr

---------

Co-authored-by: Juliano Costa <[email protected]>
Co-authored-by: Carter Socha <[email protected]>
Co-authored-by: Austin Parker <[email protected]>
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.

6 participants