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

Add architecture for images created withpack buildpack package #2080

Closed

Conversation

jericop
Copy link
Contributor

@jericop jericop commented Feb 26, 2024

Summary

This changes the behavior of pack buildpack package to include the platform architecture when the docker daemon is available, or from runtime.GOARCH otherwise. It also has support for user-supplied architecture, but this is currently not supported through any pack cli arguments.

Output

Before

The current behavior is described in #2079

After

You can verify the change in this PR by following the steps below.

Note that docker needs to be configured to build multi-arch images for this to work as described here.

Steps
  1. Create the following dockerfile in a directory.
FROM golang:1.21 as builder
WORKDIR /pack

RUN git clone --depth 1 --branch add-arch-in-buildpack-package https://github.com/jericop/pack.git /pack
RUN make mod-tidy build

FROM bitnami/git
USER root
WORKDIR /workspace

COPY --from=builder /pack/out/pack /usr/local/bin/pack

RUN <<RUN_EOF

image_arch=amd64
if [ $(arch) = "aarch64" ]; then
  image_arch=arm64
fi

git clone --depth 1 --branch main https://github.com/buildpacks/samples.git

pack buildpack package ttl.sh/jericop-buildpack-add-arch-in-buildpack-package:${image_arch} --publish \
  --path samples/buildpacks/hello-world

RUN_EOF

  1. Run the following docker buildx command to create and publish buildpacks to ttl.sh, an ephemeral registry.
docker buildx build --platform linux/amd64,linux/arm64 .
  1. Inspect the image config with crane to confirm the architecture is an empty string.
➜  pack-arch-issue crane ls ttl.sh/jericop-buildpack-add-arch-in-buildpack-package
amd64
arm64
➜  pack-arch-issue crane config ttl.sh/jericop-buildpack-add-arch-in-buildpack-package:amd64 | jq .architecture
"amd64"
➜  pack-arch-issue crane config ttl.sh/jericop-buildpack-add-arch-in-buildpack-package:arm64 | jq .architecture
"arm64"
➜  pack-arch-issue 

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

It is generally expected that when you build an image on a certain platform and architecture, the image you produce should be meant to run on that platform and architecture. As a result I don't believe this change needs to be documented.

Related

Resolves #2079
It should also resolve #1968, which was fixed by retry logic.

@jericop jericop requested review from a team as code owners February 26, 2024 05:52
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Feb 26, 2024
@github-actions github-actions bot added this to the 0.34.0 milestone Feb 26, 2024
@jericop jericop added epic/multi-arch type/bug Issue that reports an unexpected behaviour. labels Feb 26, 2024
Signed-off-by: Jerico Pena <jericop@gmail.com>
@jericop jericop force-pushed the add-arch-in-buildpack-package branch from f84b5b0 to 1900afe Compare February 26, 2024 16:55
Copy link

codecov bot commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 79.70%. Comparing base (2df4445) to head (1900afe).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2080      +/-   ##
==========================================
+ Coverage   78.96%   79.70%   +0.75%     
==========================================
  Files         176      176              
  Lines       13246    13261      +15     
==========================================
+ Hits        10459    10569     +110     
+ Misses       2119     2023      -96     
- Partials      668      669       +1     
Flag Coverage Δ
os_linux 78.61% <83.34%> (+0.01%) ⬆️
os_macos 76.34% <83.34%> (+0.05%) ⬆️
os_windows 79.10% <83.34%> (+0.79%) ⬆️
unit 79.70% <83.34%> (+0.75%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@jericop jericop changed the title Add architecture for pack buildpack package Add architecture for images created withpack buildpack package Feb 26, 2024
OS string `toml:"os"`
OS string `toml:"os"`
Architecture string `toml:"architecture"`
Platform string `toml:"platform,omitempty" json:"platform,omitempty" yaml:"platform,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit weird - why is Platform a field on Platform?

Copy link
Member

Choose a reason for hiding this comment

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

This Platform struct is used by the package.toml, as part of the multi-arch RFC, we are suggesting deprecating this field and adding a set of Targets.

Maybe we can add a single Target as part of this PR? here is what I did on the multi-arch PoC.

@jericop what about getting the information you need from there for buildpack packages and extensions?

@jjbustamante
Copy link
Member

This issue is related to the following comment in the multi-arch RFC

@edmorley
Copy link
Contributor

edmorley commented May 2, 2024

This would resolve the missing arch metadata we're seeing in our (new) multi-arch buildpack images, eg:
https://hub.docker.com/repository/docker/heroku/buildpack-go/tags

@jjbustamante jjbustamante modified the milestones: 0.34.0, 0.35.0 May 8, 2024
@jjbustamante
Copy link
Member

Closing this PR, because we decided to include the implementation of the RFC 0128 with pack 0.34.0, that implementation solves the issues this PR is trying to fix.

See my comment

@jjbustamante jjbustamante removed this from the 0.35.0 milestone May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic/multi-arch type/bug Issue that reports an unexpected behaviour. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
4 participants