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

fix: Use docker image to build binary for different platforms #9657

Closed
wants to merge 1 commit into from

Conversation

alphanota
Copy link
Contributor

@alphanota alphanota commented Jan 14, 2025

Fixes: #nnn
Related: Relevant tracking issues, for context
Merge before/after: Dependent or prerequisite PRs

Description
An update to one of the skaffold dependencies introduced a new dependency on the runtime/cgo package, breaking the ability to built skafofld binaries for different platforms with the existing logic.

Although the official golang docker images allow cross-compilation, it doesn't work when cgo is enabled. Although installing the platform-specific is possible with the golang image, I instead opted to use images from the elastic/crossbuild project which maintain docker images specifically designed for cross-compilation.

User facing changes (remove if N/A)

A new depedency on docker when building the skaffold binary for different platforms.

Test
The cross-compilation was tested with a github action running on mac here: #9658
Follow-up Work (remove if N/A)

@alphanota alphanota requested a review from a team as a code owner January 14, 2025 05:47
Copy link

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi team,

I'm currently reviewing this pull request and will provide my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly understand the changes:

This pull request addresses an issue where building Skaffold binaries for different platforms was broken due to a new dependency on the runtime/cgo package in an updated dependency. The previous method of using the official Golang Docker image for cross-compilation failed because cgo was enabled.

To solve this, I've switched to using Docker images from the elastic/golang-crossbuild project, which are specifically designed for cross-compilation with cgo enabled.

Here's a breakdown of the changes:

  • Makefile modifications:

    • Added GOVERSION ?= 1.23.4 to specify the Go version (line 14 in Makefile).
    • Replaced the existing cross-compilation logic in the Makefile (lines 93-104) with a new approach using the elastic/golang-crossbuild Docker images (lines 97-126).
    • The new logic dynamically selects the appropriate elastic/golang-crossbuild image based on the target operating system and architecture.
    • The new logic uses docker run to execute the build within a container, ensuring the correct environment and dependencies are available.
  • User-facing changes: A new dependency on Docker is introduced when building Skaffold binaries for different platforms.

I'll be checking the build process and testing thoroughly once I'm done with this summary. Let me know if you have any questions in the meantime.


And now for a little haiku to lighten the mood:

Cross-compile's woes,
Docker's image brings new hope,
Builds now run so smooth.

Copy link

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR addresses a critical issue with cross-compilation introduced by a dependency update. Switching to dedicated cross-compilation images is a good approach. However, there are a few points to address before merging.

@alphanota alphanota mentioned this pull request Jan 14, 2025
@alphanota alphanota closed this Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant