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

Get Dockerfile env vars properly working: #75

Merged
merged 1 commit into from
Jan 15, 2023

Conversation

jacobweinstock
Copy link
Member

@jacobweinstock jacobweinstock commented Jan 14, 2023

Description

The TARGETARCH and TARGETOS env vars weren't making it to the go build command so the GOOS and GOARCH were would be empty and the build would only use amd64.

Why is this needed

Manually tested by running this locally:
docker buildx build --cache-from type=registry,ref=quay.io/tinkerbell/rufio:latest --file ./Dockerfile --load -t rufio:test --platform linux/arm64 ./
and then extracting the manager binary and validate the binary type to beARM aarch64.

Also, locally in the build logs you see [builder 7/7] RUN CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build -a -o manager main.go versus in the GitHub actions log you see [linux/amd64 builder 7/7] RUN CGO_ENABLED=0 GOOS=$TARGETOS GOARCH=$TARGETARCH go build -a -o manager main.go
https://github.com/tinkerbell/rufio/actions/runs/3831046658/jobs/6519718105#step:6:269
Fixes: #74

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

The TARGETARCH and TARGETOS env vars weren't making
it to the `go build` command so the GOOS and GOARCH
were would be empty and the build would only use amd64.

Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
@codecov
Copy link

codecov bot commented Jan 14, 2023

Codecov Report

Merging #75 (a3f3122) into main (58b9f07) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #75   +/-   ##
=======================================
  Coverage   59.50%   59.50%           
=======================================
  Files           4        4           
  Lines         321      321           
=======================================
  Hits          191      191           
  Misses         97       97           
  Partials       33       33           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@chrisdoherty4 chrisdoherty4 added the ready-to-merge Mergify: Ready for Merging label Jan 15, 2023
@jacobweinstock jacobweinstock removed the request for review from pokearu January 15, 2023 01:36
@mergify mergify bot merged commit dee5b26 into tinkerbell:main Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Mergify: Ready for Merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ARM Image fails to run on ARM
2 participants