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.

Sign up for GitHub

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

Jump to bottom

Update hardcoded error message to pull images without architecture #2318

Merged
merged 2 commits into from
Jan 15, 2025
Merged

Update hardcoded error message to pull images without architecture #2318

merged 2 commits into from
Jan 15, 2025

Conversation

jjbustamante
Copy link
Member

@jjbustamante jjbustamante commented Jan 14, 2025 •
edited
Loading

Summary

When the --platform flag was released, we wanted to keep compatibility with old buildpacks where architecture was not specified, we also didn't want to add an overhead on trying to pull an image when it doesn't exist in the registry.

The way we did it was to parse the error message returned by the pulling operation and try to match some strings, the message was changed in the upstream docker dependency and this ticket is just updating it.

Output

Before

> pack build my-app -b gcr.io/paketo-buildpacks/nodejs:latest
Warning: Ignoring the provided lifecycle image as the builder is trusted, running the creator in a single container using the provided builder
...
Digest: sha256:ba59495fdb04ec023b79119ae9f0cf811a2d3f4b873db1ec545edcff4434b8b2
Status: Downloaded newer image for gcr.io/paketo-buildpacks/nodejs:latest
ERROR: failed to build: downloading buildpack: extracting from registry gcr.io/paketo-buildpacks/nodejs:latest: fetching image: image with reference gcr.io/paketo-buildpacks/nodejs:latest was found but its platform (linux) does not match the specified platform (linux/amd64)

We can see the error message:

image with reference gcr.io/paketo-buildpacks/nodejs:latest was found but its platform (linux) does not match the specified platform (linux/amd64)

Is the new one updated in the upstream dependecy

After

> /home/juan/go/src/github.com/buildpacks/pack/out/pack build my-app -b gcr.io/paketo-buildpacks/nodejs:latest
Warning: Ignoring the provided lifecycle image as the builder is trusted, running the creator in a single container using the provided builder
latest: Pulling from paketobuildpacks/builder-jammy-base
...
Status: Image is up to date for gcr.io/paketo-buildpacks/nodejs:latest
latest: Pulling from paketo-buildpacks/nodejs
Digest: sha256:ba59495fdb04ec023b79119ae9f0cf811a2d3f4b873db1ec545edcff4434b8b2
Status: Image is up to date for gcr.io/paketo-buildpacks/nodejs:latest
Warning: Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow
0.19.3: Pulling from buildpacksio/lifecycle
Digest: sha256:3184c0c4028b6ca18e851388f3dd54c10fcaea5e6f1e43cf660d0647be69d6cf
Status: Image is up to date for buildpacksio/lifecycle:0.19.3
===> ANALYZING

pack is pulling the image using an empty platform again once the error message match the condition to retry.

Note

This solution is not the best, but it will unblock pack users for now

Documentation

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

Related

Resolves #2315

@jjbustamante
Updating hardcoded error message handling when an image was not found…
105c224 
… in a registry by platform

Signed-off-by: Juan Bustamante <bustamantejj@gmail.com>
@github-actions github-actions bot added this to the 0.37.0 milestone Jan 14, 2025
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Jan 14, 2025
@jjbustamante
removing additional string matching
a2c2365 
Signed-off-by: Juan Bustamante <bustamantejj@gmail.com>
@jjbustamante jjbustamante changed the title Update Update hardcoded error message to pull images without architecture Jan 14, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 14, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.00%. Comparing base (2f1af85) to head (a2c2365).
Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2318      +/-   ##
==========================================
- Coverage   70.00%   70.00%   -0.00%     
==========================================
  Files         254      254              
  Lines       18721    18720       -1     
==========================================
- Hits        13104    13103       -1     
  Misses       4741     4741              
  Partials      876      876
Flag Coverage Δ
os_linux 69.14% <100.00%> (-<0.01%) ⬇️
os_macos-arm64 65.34% <0.00%> (+0.01%) ⬆️
os_windows 69.51% <100.00%> (-<0.01%) ⬇️
unit 70.00% <100.00%> (-<0.01%) ⬇️

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

c0d1ngm0nk3y
c0d1ngm0nk3y reviewed Jan 14, 2025
View reviewed changes
pkg/image/fetcher.go
strings.HasSuffix(strings.TrimSpace(err.Error()), "actual: windows")) {
// `image with reference <image> was found but does not match the specified platform: wanted linux/amd64, actual: linux` or
// `image with reference <image> was found but its platform (linux) does not match the specified platform (linux/amd64)`
if strings.Contains(err.Error(), "does not match the specified platform") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant as a quick fix? Because it would break again if docker decides to change the wording. But a quick fix would be highly appreciated since this is really critical for us.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah! I can take a look later and try to find a better solution, but I wanted to unblock end-users with it

@jjbustamante jjbustamante added type/bug Issue that reports an unexpected behaviour. and removed type/enhancement Issue that requests a new feature or improvement. labels Jan 14, 2025
@jjbustamante jjbustamante modified the milestones: 0.37.0, 0.36.4 Jan 14, 2025
@jjbustamante jjbustamante marked this pull request as ready for review January 14, 2025 13:05
@jjbustamante jjbustamante requested review from a team as code owners January 14, 2025 13:05
@jjbustamante
Copy link
Member Author

@c0d1ngm0nk3y Could you verify the binary and check if it solves your bug?

@jjbustamante jjbustamante merged commit 0adeb45 into main Jan 15, 2025
18 checks passed
@jjbustamante jjbustamante deleted the jjbustamante/fix-pack-2315 branch January 15, 2025 01:34
@c0d1ngm0nk3y
Copy link
Contributor

Will this trigger another release?

@jjbustamante
Copy link
Member Author

Will this trigger another release?

Yes! I also want to include ticket #2299! I left you a message there 😄, if we can verify that fix, I will release 0.36.4 today or tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c0d1ngm0nk3y c0d1ngm0nk3y c0d1ngm0nk3y left review comments

Assignees
No one assigned
Labels
type/bug Issue that reports an unexpected behaviour.
Projects
None yet
Milestone
0.36.4
Development

Successfully merging this pull request may close these issues.

Pulling images without architecture broken due to hardcoded error message
2 participants
@jjbustamante @c0d1ngm0nk3y