-
Notifications
You must be signed in to change notification settings - Fork 32
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(go/base): set GOTOOLCHAIN=local and skip building arm using the base Dockerfile #524
Conversation
Disable the automatic downloads of the Go toolchain. For the goal of reproducible builds using golang-crossbuild, we want all of the binaries used to produce our artifacts to be self-contained in the image. Adding GOTOOLCHAIN=local disable downloads of newer toolchains. This changes the entrypoint of the images to document that GOTOOLCHAIN is set during builds. And it sets GOTOOLCHAIN=local in the Dockerfile so that when building entrypoint.go it uses the toolchain that was previously installed. The two Dockerfiles (base and base-arm) had different 'go build' commands. This unifies them to both have CGO_ENABLED=0 and to explicitly set GOARCH to avoid accidentally producing a binary for the wrong target architecture (which we've seen happen for unknown reasons). https://go.dev/doc/toolchain
This pull request does not have a backport label. Could you fix it @dliappis? 🙏
|
To help with reviews: this PR builds on top of #523 and makes an additional change to address the problem mentioned in https://github.com/elastic/golang-crossbuild/pull/524/files#diff-78f24cf6131fffe363db9182e23c8e5ff491034e44c49dfc5678bb85ed5146e8L1 Using this PR, The change has been tested by the image produced in the 1.23 branch via #527 ; using #527 me and @v1v tested the CI produced image |
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.
LGTM
This reverts commit 7cae477.
8b4a1e3
to
e665ff1
Compare
This reverts commit e665ff1.
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.
LGTM
Reviewers/codeowners, I had to push a change that adds a buildkite step for the fpm image so that we have an Everything else remains the same in this PR, so please take another look cc @andrewkroh / @swiatekm / @pazone |
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.
LGTM. Will update the elastic-agent PR as soon as this one is merged
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.
LGTM
This reverts commit 4822107.
Apologies, I had to revert this; the commit 4822107 didn't properly add the -arm variant as a multiarch image, so the arm64 version of @pazone we talked with @pchila about what this means for your PR and he explained that we don't need to have dedicated PR based packaging steps for deb+rpm on arm64 -- it's enough to only test those two on amd64, as it's been the case until now. |
This reverts commit ea548d9.
💚 Build Succeeded
History
cc @dliappis |
💚 Build Succeeded
History
cc @dliappis |
💔 Build Failed
Failed CI Steps
History
cc @dliappis |
Disable the automatic downloads of the Go toolchain.
For the goal of reproducible builds using golang-crossbuild, we want all of the binaries used to produce our artifacts to be self-contained in the image. Adding GOTOOLCHAIN=local disable downloads of newer toolchains.
This changes the entrypoint of the images to document that GOTOOLCHAIN is set during builds. And it sets GOTOOLCHAIN=local in the Dockerfile so that when building entrypoint.go it uses the toolchain that was previously installed.
The two Dockerfiles (base and base-arm) had different 'go build' commands. This unifies them to both have CGO_ENABLED=0 and to explicitly set GOARCH to avoid accidentally producing a binary for the wrong target architecture (which we've seen happen for unknown reasons).
Additionally we remove the arm target from the base Dockerfile, to address the CI concurrency problem described
in #523 (comment)
https://go.dev/doc/toolchain
Co-Authored by @andrewkroh