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

Use daemon or runtime provided arch instead of amd64 #232

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

jericop
Copy link
Contributor

@jericop jericop commented Nov 1, 2023

The current defaultPlatform functions sets the architecture to "amd64", which causes pack to use the amd64 build image, and to install the amd64 lifecycle, when creating a new builder from an arm64 host with no docker daemon installed.

This changes the default platform functions to use the docker daemon os and architecture for local images, and runtime.GOARCH as the architecture for new remote images.

The following log file is the output of a script that was used to identify the issue.
https://github.com/jericop/github-actions/blob/main/buildpacks/create-multi-arch-builders/standalone/imgutil-pr-232-verify-amd64-issue.sh.log

You can see that the script doesn't finish. You can also see that it downloads an amd64 lifecycle rather than the arm64 as expected.
https://github.com/jericop/github-actions/blob/main/buildpacks/create-multi-arch-builders/standalone/imgutil-pr-232-verify-amd64-issue.sh.log#L159

The following log file is the output of a script was used to identify that an updated version of pack that uses this fix.
https://github.com/jericop/github-actions/blob/main/buildpacks/create-multi-arch-builders/standalone/imgutil-pr-232-verify-fix.sh.log

@jericop jericop requested a review from a team as a code owner November 1, 2023 17:00
@jjbustamante
Copy link
Member

jjbustamante commented Nov 1, 2023

Summary

This PR is fixing an issue when running pack using emulation with docker buildx which I think it is great!!

Additionally, I think the root problem is that we can't define the platform when creating Builders, something we are trying to solve with the multi-arch RFC but improving the default values to be more realistic than hard coding something, I think is a great improvement

Local change

  • We are changing from using the /info API, and reading OSType: type of the operating system of the host, as returned by the Go runtime (GOOS). Currently returned values are "linux" and "windows", and hard coding amd64
  • To use /version API, and reading
    • Os The operating system that the daemon is running on ("linux" or "windows")
    • Arch The architecture that the daemon is running on
      This makes a lot of sense to me, because with the local interface we are going to look for images in the daemon.

Remote change

  • We are changing the hard coded amd64 for runtime.GOARCH which is defined as: is the running program's architecture target: one of 386, amd64, arm, s390x, and so on.

    This also makes sense to me, because when we are running pack commands, we expect to match the host machine

Copy link
Member

@jjbustamante jjbustamante left a comment

Choose a reason for hiding this comment

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

I tested Jerico script and check pack running and everything looks good to me!

@natalieparellano if you can take a look will be awesome!

@natalieparellano natalieparellano merged commit 84d6321 into buildpacks:main Nov 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants