-
Notifications
You must be signed in to change notification settings - Fork 294
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
Retrying logic when pulling an image and platform doesn't match #1969
Conversation
d00f1a9
to
66da647
Compare
…atch, in this case we will retry without any platform defined (previous behavior) Signed-off-by: Juan Bustamante <[email protected]>
119750f
to
d022a54
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the better approach would be to have 3 attempts:
- os/arch
- os (currently this one is skipped and "linux" is never requested, right?)
- anything (is this even needed?)
pkg/image/fetcher.go
Outdated
@@ -192,6 +197,11 @@ func (f *Fetcher) pullImage(ctx context.Context, imageID string, platform string | |||
|
|||
err = jsonmessage.DisplayJSONMessagesStream(rc, &colorizedWriter{writer}, termFd, isTerm, nil) | |||
if err != nil { | |||
// sample error from docker engine: | |||
// image with reference <image> was found but does not match the specified platform: wanted linux/amd64, actual: linux | |||
if strings.Contains(err.Error(), "does not match the specified platform") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comparing an error string feels a bit fragile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree... but, the type of error returned is *jsonmessage.JSONError
😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alternative I guess would be to always retry
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
I added an acceptance test in 5451248 and cleaned up the error wrapping so that we're passing less strings around. I played around with retrying with platform WDYT @jjbustamante @c0d1ngm0nk3y ? |
Great. Because for use, this is issue is quite nasty, so this should help prevent such things in future. |
Summary
The Paketo team is trying to create
arm64
builders and Buildpacks, but they are doing iteratively, because of that, they are in a situation where they need some mixed support forarm64
andamd64
users. With the latest pack release 0.32.0 we tried to fix an issue where users running M1 laptops and trying to build aamd64
application image ended up with an application image havingamd64
code on the top of aarm64
run image. see this PRThe problem was, we added a
platform
configuration (os/arch) when pulling Buildpacks and unfortunately this is not valid because most of the Buildpack ecosystem doesn't specify architecture for their buildpacks.As a consequence, running
pack build
orpack builder create
or any command that is pulling Builpacks throws an error, I added a retry logic to pull again, without platform (previous behavior), when an error is found with the messagedoes not match the specified platform
Output
Before
After
pack
will silently retrylatest: Pulling from paketo-buildpacks/dotnet-core Digest: sha256:bf28b7631c222794f01e637c2de0d097f39079c0aa6c8fbdaefeef239d404219 Status: Image is up to date for gcr.io/paketo-buildpacks/dotnet-core:latest latest: Pulling from paketo-buildpacks/dotnet-core Digest: sha256:bf28b7631c222794f01e637c2de0d097f39079c0aa6c8fbdaefeef239d404219
Documentation
Related
Resolves #1968