-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Fix docker cross-platform builds #10827
Conversation
Please give priority to this PR, we need the arm64 image before the hardfork on op-node 1.7.7 |
This PR works on M1 Mac, but not on AWS Graviton instance. Error Message |
Semgrep found 1 Malformed revert statement style. Ignore this finding from sol-style-malformed-revert.Semgrep found 28
No Semgrep found 1 Potential Semgrep found 1 Do not use Semgrep found 2 Please create a GitHub ticket for this TODO. Ignore this finding from todos_require_linear.Semgrep found 1 New code should use errors.Is with the appropriate error type Ignore this finding from os-error-is-not-exist.Semgrep found 2 MarshalJSON with a pointer receiver has surprising results: golang/go#22967 Ignore this finding from marshal-json-pointer-receiver. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #10827 +/- ##
========================================
Coverage 60.69% 60.69%
========================================
Files 20 20
Lines 1781 1781
Branches 71 71
========================================
Hits 1081 1081
Misses 667 667
Partials 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. |
798b85b
to
7362544
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding op-program: the host part is not subject to the same reproduction and platform target restrictions as the client part
The docker images being pushed to docker registry by our CI do not work on arm64 machines. This PR attempts to fix that.
Fixes #11081
Key change to fix cross compiling issues
Removed
TARGETOS
andTARGETARCH
declarations from the top of the dockerfile. These args both get set automatically by the docker build kit, and the global declarations in the dockerfile caused an issue where the values were not set or passed to the underlyingmake
commands which invokego build
.Additional context
Also added
CGO_ENABLED=0
to docker images that are being built for cross-platform. This will cause the build to fail if C code is included in the go source code. This seems better than allowing the build to pass and only catching the failure further downstream when the image is pulled and attempted to run on an arm64 machine. If we want the cross-platform builds to work with C code included, we will have to spend some more effort adding appropriateCC
andCXX
packages to our docker builder, then allowCGO_ENABLED=1
. I'm open to removing this part if reviewers feel this is a bad ideaTests
Added
check-cross-platform
ci jobs for every image published duringscheduled-docker-publish
that is built for multiple platforms (i.e.platforms: "linux/amd64,linux/arm64"
for thedocker-build
job)