From d022a549432e1eb0fc7c040514b0056d696a6b72 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Thu, 9 Nov 2023 12:03:12 -0500 Subject: [PATCH 1/3] Adding a retry logic when pulling an image and the platform doesn't match, in this case we will retry without any platform defined (previous behavior) Signed-off-by: Juan Bustamante --- pkg/image/fetcher.go | 10 ++++++++++ pkg/image/fetcher_test.go | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index a014dc8b9..3835b0bd3 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -82,6 +82,7 @@ func NewFetcher(logger logging.Logger, docker DockerClient, opts ...FetcherOptio } var ErrNotFound = errors.New("not found") +var ErrPlatformNotMatch = errors.New("platform does not match") func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) (imgutil.Image, error) { name, err := pname.TranslateRegistry(name, f.registryMirrors, f.logger) @@ -110,6 +111,10 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) f.logger.Debugf("Pulling image %s", style.Symbol(name)) err = f.pullImage(ctx, name, options.Platform) + if errors.Is(err, ErrPlatformNotMatch) { + f.logger.Info(err.Error()) + err = f.pullImage(ctx, name, "") + } if err != nil && !errors.Is(err, ErrNotFound) { return nil, err } @@ -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 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") { + return errors.Wrap(ErrPlatformNotMatch, err.Error()) + } return err } diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index 03b1f0467..3b119e15f 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -6,8 +6,11 @@ import ( "fmt" "os" "path/filepath" + "runtime" "testing" + "github.com/buildpacks/imgutil" + "github.com/buildpacks/imgutil/local" "github.com/buildpacks/imgutil/remote" "github.com/docker/docker/client" @@ -48,12 +51,17 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { repoName string repo string outBuf bytes.Buffer + osType string ) it.Before(func() { repo = "some-org/" + h.RandString(10) repoName = registryConfig.RepoName(repo) imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker) + + info, err := docker.Info(context.TODO()) + h.AssertNil(t, err) + osType = info.OSType }) when("#Fetch", func() { @@ -213,6 +221,19 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullAlways, Platform: "some-unsupported-platform"}) h.AssertError(t, err, "unknown operating system or architecture") }) + + when("remote platform does not match", func() { + it.Before(func() { + img, err := remote.NewImage(repoName, authn.DefaultKeychain, remote.WithDefaultPlatform(imgutil.Platform{OS: osType, Architecture: ""})) + h.AssertNil(t, err) + h.AssertNil(t, img.Save()) + }) + + it("retry without setting platform", func() { + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullAlways, Platform: fmt.Sprintf("%s/%s", osType, runtime.GOARCH)}) + h.AssertNil(t, err) + }) + }) }) }) From 5451248a476058f746c0d91f50f193c1ab4c6981 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 13 Nov 2023 15:41:12 -0500 Subject: [PATCH 2/3] Add acceptance test Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 38 +++++++++++++++++++++++++++++++++++ pkg/image/fetcher.go | 16 ++++++--------- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index da19ad6fd..7e51324d2 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1551,6 +1551,44 @@ func testAcceptance( assertBuildpackOutput := assertions.NewTestBuildpackOutputAssertionManager(t, output) assertBuildpackOutput.ReportsBuildStep("Simple Layers Buildpack") }) + + when("buildpackage is in a registry", func() { + it("adds the buildpacks to the builder and runs them", func() { + packageImageName = registryConfig.RepoName("buildpack-" + h.RandString(8)) + + packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, "package_for_build_cmd.toml", imageManager.HostOS()) + packageImage := buildpacks.NewPackageImage( + t, + pack, + packageImageName, + packageTomlPath, + buildpacks.WithRequiredBuildpacks( + buildpacks.BpFolderSimpleLayersParent, + buildpacks.BpFolderSimpleLayers, + ), + buildpacks.WithPublish(), + ) + + buildpackManager.PrepareBuildModules(tmpDir, packageImage) + + output := pack.RunSuccessfully( + "build", repoName, + "-p", filepath.Join("testdata", "mock_app"), + "--buildpack", packageImageName, + ) + + assertOutput := assertions.NewOutputAssertionManager(t, output) + assertOutput.ReportsAddingBuildpack( + "simple/layers/parent", + "simple-layers-parent-version", + ) + assertOutput.ReportsAddingBuildpack("simple/layers", "simple-layers-version") + assertOutput.ReportsSuccessfulImageBuild(repoName) + + assertBuildpackOutput := assertions.NewTestBuildpackOutputAssertionManager(t, output) + assertBuildpackOutput.ReportsBuildStep("Simple Layers Buildpack") + }) + }) }) when("the argument is a buildpackage file", func() { diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 3835b0bd3..60548fcc1 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -82,7 +82,6 @@ func NewFetcher(logger logging.Logger, docker DockerClient, opts ...FetcherOptio } var ErrNotFound = errors.New("not found") -var ErrPlatformNotMatch = errors.New("platform does not match") func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) (imgutil.Image, error) { name, err := pname.TranslateRegistry(name, f.registryMirrors, f.logger) @@ -110,10 +109,12 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) } f.logger.Debugf("Pulling image %s", style.Symbol(name)) - err = f.pullImage(ctx, name, options.Platform) - if errors.Is(err, ErrPlatformNotMatch) { - f.logger.Info(err.Error()) - err = f.pullImage(ctx, name, "") + if err = f.pullImage(ctx, name, options.Platform); err != nil { + // sample error from docker engine: + // image with reference 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") { + err = f.pullImage(ctx, name, "") + } } if err != nil && !errors.Is(err, ErrNotFound) { return nil, err @@ -197,11 +198,6 @@ 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 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") { - return errors.Wrap(ErrPlatformNotMatch, err.Error()) - } return err } From 63cf11a67ab01ed6196dd7bcd6d4b74a6fcf0065 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 13 Nov 2023 15:52:36 -0500 Subject: [PATCH 3/3] Skip test on pack versions that aren't fixed yet Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 1 + acceptance/invoke/pack.go | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 7e51324d2..edff79cd4 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1554,6 +1554,7 @@ func testAcceptance( when("buildpackage is in a registry", func() { it("adds the buildpacks to the builder and runs them", func() { + h.SkipIf(t, !pack.SupportsFeature(invoke.PlatformRetries), "") packageImageName = registryConfig.RepoName("buildpack-" + h.RandString(8)) packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, "package_for_build_cmd.toml", imageManager.HostOS()) diff --git a/acceptance/invoke/pack.go b/acceptance/invoke/pack.go index 35bd58f5b..caa60f136 100644 --- a/acceptance/invoke/pack.go +++ b/acceptance/invoke/pack.go @@ -235,6 +235,7 @@ const ( ForceRebase BuildpackFlatten MetaBuildpackFolder + PlatformRetries ) var featureTests = map[Feature]func(i *PackInvoker) bool{ @@ -262,6 +263,9 @@ var featureTests = map[Feature]func(i *PackInvoker) bool{ MetaBuildpackFolder: func(i *PackInvoker) bool { return i.atLeast("v0.30.0") }, + PlatformRetries: func(i *PackInvoker) bool { + return i.atLeast("v0.32.1") + }, } func (i *PackInvoker) SupportsFeature(f Feature) bool {