Build manager on the host #2289
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
swiatekm
force-pushed
the
build/cross-compile
branch
2 times, most recently
from
c895491
to
ffadd08
Compare
swiatekm
changed the title
swiatekm
force-pushed
the
build/cross-compile
branch
from
ffadd08
to
0df10e6
Compare
jaronoff97
reviewed
Oct 30, 2023
| @@ -86,19 +101,5 @@ jobs: | |||
| push: ${{ github.event_name != 'pull_request' }} | |||
| tags: ${{ steps.docker_meta.outputs.tags }} | |||
| labels: ${{ steps.docker_meta.outputs.labels }} | |||
| build-args: | | |||
There was a problem hiding this comment.
why do we not need these args anymore?
There was a problem hiding this comment.
They're set while building the binary, straight from the environment. They're not needed for anything else.
jaronoff97
approved these changes
Oct 30, 2023
There was a problem hiding this comment.
I think this LGTM, but we should wait on @pavolloffay 's review here too
swiatekm
force-pushed
the
build/cross-compile
branch
2 times, most recently
from
d1d6705
to
4e61c5a
Compare
swiatekm
changed the title
pavolloffay
approved these changes
Nov 3, 2023
Instead of building the manager binary during the docker image build, instead build it on the host and copy it to the image. This massively improves the performance of cross compilation, which is much faster natively vs emulation via QEMU.
This way we can cache based on the state of go.sum.
swiatekm
force-pushed
the
build/cross-compile
branch
from
4e61c5a
to
c334692
Compare
jaronoff97
approved these changes
Nov 6, 2023
ItielOlenick
pushed a commit
to ItielOlenick/opentelemetry-operator
that referenced
this pull request
May 1, 2024
* Build manager on the host Instead of building the manager binary during the docker image build, instead build it on the host and copy it to the image. This massively improves the performance of cross compilation, which is much faster natively vs emulation via QEMU. * Setup Go after checking our code in E2E tests This way we can cache based on the state of go.sum. * Minor improvements
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Cross-compile the manager binary on the host and copy the appropriate binary into the container during the build. This makes image building much faster, as we don't need to rely on emulation via QEMU.
I've also removed our local toolchain's dependency on buildx while I was at it, as it's not strictly necessary.
Looks like we could build the binaries in a matrix job, but given that they take around 15 minutes without cache, I don't think it's worth it to mess around with artifacts.
Issue: #2295
Testing:
Temporarily made the build happen for this PR, and it worked fine.