From d81ec1600ce8663a596ff8e3b43cbd979b0ad0a6 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 20 May 2024 15:15:18 -0400 Subject: [PATCH 1/8] Support `pack build --platform` Fixes https://github.com/buildpacks/pack/issues/2154 Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 21 ++++ acceptance/managers/image_manager.go | 7 ++ internal/commands/build.go | 5 + internal/commands/build_test.go | 20 ++++ pkg/client/build.go | 29 ++++- pkg/client/build_test.go | 152 +++++++++++++++++++++++++++ pkg/image/fetcher.go | 9 +- pkg/image/fetcher_test.go | 2 +- 8 files changed, 237 insertions(+), 8 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index c5ced80561..cb3bb66125 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2508,6 +2508,27 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] }) }) }) + + when("--platform", func() { + wrongArch := "arm64" + it.Before(func() { + h.SkipIf(t, imageManager.HostOS() == "windows", "Not relevant on windows") + if hostArch := imageManager.HostArch(); hostArch == wrongArch { + wrongArch = "amd64" + } + // FIXME: on an M1 with emulation enabled this test might pass when we expect it to fail + }) + + it("uses the builder with the desired platform", func() { + output, err := pack.Run( + "build", repoName, + "-p", filepath.Join("testdata", "mock_app"), + "--platform", fmt.Sprintf("linux/%s", wrongArch), + ) + h.AssertNotNil(t, err) + h.AssertContains(t, output, "was found but does not match the specified platform") + }) + }) }) when("build --buildpack ", func() { diff --git a/acceptance/managers/image_manager.go b/acceptance/managers/image_manager.go index b968b4ceaa..ca3baa554b 100644 --- a/acceptance/managers/image_manager.go +++ b/acceptance/managers/image_manager.go @@ -62,6 +62,13 @@ func (im ImageManager) HostOS() string { return daemonInfo.OSType } +func (im ImageManager) HostArch() string { + im.testObject.Helper() + daemonInfo, err := im.dockerCli.Info(context.Background()) + im.assert.Nil(err) + return daemonInfo.Architecture +} + func (im ImageManager) TagImage(image, ref string) { im.testObject.Helper() err := im.dockerCli.ImageTag(context.Background(), image, ref) diff --git a/internal/commands/build.go b/internal/commands/build.go index d32f96cf8b..50ccebeabd 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -35,6 +35,7 @@ type BuildFlags struct { Builder string Registry string RunImage string + Platform string Policy string Network string DescriptorPath string @@ -132,6 +133,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } + var lifecycleImage string if flags.LifecycleImage != "" { ref, err := name.ParseReference(flags.LifecycleImage) @@ -140,6 +142,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob } lifecycleImage = ref.Name() } + var gid = -1 if cmd.Flags().Changed("gid") { gid = flags.GID @@ -165,6 +168,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob Image: inputImageName.Name(), Publish: flags.Publish, DockerHost: flags.DockerHost, + Platform: flags.Platform, PullPolicy: pullPolicy, ClearCache: flags.ClearCache, TrustBuilder: func(string) bool { @@ -257,6 +261,7 @@ Special value 'inherit' may be used in which case DOCKER_HOST environment variab This option may set DOCKER_HOST environment variable for the build container if needed. `) cmd.Flags().StringVar(&buildFlags.LifecycleImage, "lifecycle-image", cfg.LifecycleImage, `Custom lifecycle image to use for analysis, restore, and export when builder is untrusted.`) + cmd.Flags().StringVar(&buildFlags.Platform, "platform", "", `Platform to build on (e.g., "linux/amd64").`) cmd.Flags().StringVar(&buildFlags.Policy, "pull-policy", "", `Pull policy to use. Accepted values are always, never, and if-not-present. (default "always")`) cmd.Flags().StringVarP(&buildFlags.Registry, "buildpack-registry", "r", cfg.DefaultRegistryName, "Buildpack Registry by name") cmd.Flags().StringVar(&buildFlags.RunImage, "run-image", "", "Run image (defaults to default stack's run image)") diff --git a/internal/commands/build_test.go b/internal/commands/build_test.go index 6a59810004..07561e175a 100644 --- a/internal/commands/build_test.go +++ b/internal/commands/build_test.go @@ -148,6 +148,17 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) { }) }) + when("--platform", func() { + it("sets platform", func() { + mockClient.EXPECT(). + Build(gomock.Any(), EqBuildOptionsWithPlatform("linux/amd64")). + Return(nil) + + command.SetArgs([]string{"image", "--builder", "my-builder", "--platform", "linux/amd64"}) + h.AssertNil(t, command.Execute()) + }) + }) + when("--pull-policy", func() { it("sets pull-policy=never", func() { mockClient.EXPECT(). @@ -958,6 +969,15 @@ func EqBuildOptionsDefaultProcess(defaultProc string) gomock.Matcher { } } +func EqBuildOptionsWithPlatform(platform string) gomock.Matcher { + return buildOptionsMatcher{ + description: fmt.Sprintf("Platform=%s", platform), + equals: func(o client.BuildOptions) bool { + return o.Platform == platform + }, + } +} + func EqBuildOptionsWithPullPolicy(policy image.PullPolicy) gomock.Matcher { return buildOptionsMatcher{ description: fmt.Sprintf("PullPolicy=%s", policy), diff --git a/pkg/client/build.go b/pkg/client/build.go index 68547d910f..08edbf4e6f 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -159,6 +159,9 @@ type BuildOptions struct { // Process type that will be used when setting container start command. DefaultProcessType string + // Platform is the desired platform to build on (e.g., linux/amd64) + Platform string + // Strategy for updating local images before a build. PullPolicy image.PullPolicy @@ -320,7 +323,14 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return errors.Wrapf(err, "invalid builder '%s'", opts.Builder) } - rawBuilderImage, err := c.imageFetcher.Fetch(ctx, builderRef.Name(), image.FetchOptions{Daemon: true, PullPolicy: opts.PullPolicy}) + rawBuilderImage, err := c.imageFetcher.Fetch( + ctx, + builderRef.Name(), + image.FetchOptions{ + Daemon: true, + Platform: opts.Platform, + PullPolicy: opts.PullPolicy}, + ) if err != nil { return errors.Wrapf(err, "failed to fetch builder image '%s'", builderRef.Name()) } @@ -335,6 +345,11 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return errors.Wrapf(err, "getting builder architecture") } + platformToUse := opts.Platform + if platformToUse == "" { + platformToUse = fmt.Sprintf("%s/%s", builderOS, builderArch) // TODO: what about arch variant, etc. + } + bldr, err := c.getBuilder(rawBuilderImage) if err != nil { return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder)) @@ -343,7 +358,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { fetchOptions := image.FetchOptions{ Daemon: !opts.Publish, PullPolicy: opts.PullPolicy, - Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), + Platform: platformToUse, } runImageName := c.resolveRunImage(opts.RunImage, imgRegistry, builderRef.Context().RegistryStr(), bldr.DefaultRunImage(), opts.AdditionalMirrors, opts.Publish, fetchOptions) @@ -418,7 +433,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { image.FetchOptions{ Daemon: true, PullPolicy: opts.PullPolicy, - Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), + Platform: platformToUse, }, ) if err != nil { @@ -1213,10 +1228,16 @@ func (c *Client) fetchBuildpack(ctx context.Context, bp string, relativeBaseDir if err != nil { return nil, nil, errors.Wrapf(err, "getting builder architecture") } + + platformToUse := opts.Platform + if platformToUse == "" { + platformToUse = fmt.Sprintf("%s/%s", builderOS, builderArch) // TODO: what about arch variant, etc. + } + downloadOptions := buildpack.DownloadOptions{ RegistryName: registry, ImageOS: builderOS, - Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), + Platform: platformToUse, RelativeBaseDir: relativeBaseDir, Daemon: !publish, PullPolicy: pullPolicy, diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index e378fbcb15..65d7ab1c17 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -2261,6 +2261,158 @@ api = "0.2" }) }) + when("Platform option", func() { + var fakePackage imgutil.Image + + it.Before(func() { + metaBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + WithAPI: api.MustParse("0.3"), + WithInfo: dist.ModuleInfo{ + ID: "meta.buildpack.id", + Version: "meta.buildpack.version", + Homepage: "http://meta.buildpack", + }, + WithStacks: nil, + WithOrder: dist.Order{{ + Group: []dist.ModuleRef{{ + ModuleInfo: dist.ModuleInfo{ + ID: "child.buildpack.id", + Version: "child.buildpack.version", + }, + Optional: false, + }}, + }}, + }) + + childBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + WithAPI: api.MustParse("0.3"), + WithInfo: dist.ModuleInfo{ + ID: "child.buildpack.id", + Version: "child.buildpack.version", + Homepage: "http://child.buildpack", + }, + WithStacks: []dist.Stack{ + {ID: defaultBuilderStackID}, + }, + }) + + bpLayers := dist.ModuleLayers{ + "meta.buildpack.id": { + "meta.buildpack.version": { + API: api.MustParse("0.3"), + Order: dist.Order{{ + Group: []dist.ModuleRef{{ + ModuleInfo: dist.ModuleInfo{ + ID: "child.buildpack.id", + Version: "child.buildpack.version", + }, + Optional: false, + }}, + }}, + LayerDiffID: diffIDForFile(t, metaBuildpackTar), + }, + }, + "child.buildpack.id": { + "child.buildpack.version": { + API: api.MustParse("0.3"), + Stacks: []dist.Stack{ + {ID: defaultBuilderStackID}, + }, + LayerDiffID: diffIDForFile(t, childBuildpackTar), + }, + }, + } + + md := buildpack.Metadata{ + ModuleInfo: dist.ModuleInfo{ + ID: "meta.buildpack.id", + Version: "meta.buildpack.version", + }, + Stacks: []dist.Stack{ + {ID: defaultBuilderStackID}, + }, + } + + fakePackage = fakes.NewImage("example.com/some/package", "", nil) + h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpack.layers", bpLayers)) + h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpackage.metadata", md)) + + h.AssertNil(t, fakePackage.AddLayer(metaBuildpackTar)) + h.AssertNil(t, fakePackage.AddLayer(childBuildpackTar)) + + fakeImageFetcher.LocalImages[fakePackage.Name()] = fakePackage + }) + + when("provided", func() { + it("uses the provided platform to pull the builder, run image, packages, and lifecycle image", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + Buildpacks: []string{ + "example.com/some/package", + }, + Platform: "linux/arm64", + PullPolicy: image.PullAlways, + })) + + args := fakeImageFetcher.FetchCalls[defaultBuilderName] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "linux/arm64") + + args = fakeImageFetcher.FetchCalls["default/run"] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "linux/arm64") + + args = fakeImageFetcher.FetchCalls[fakePackage.Name()] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "linux/arm64") + + args = fakeImageFetcher.FetchCalls[fmt.Sprintf("%s:%s", cfg.DefaultLifecycleImageRepo, builder.DefaultLifecycleVersion)] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "linux/arm64") + }) + }) + + when("not provided", func() { + it("defaults to builder os/arch", func() { + // defaultBuilderImage has linux/amd64 + + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + Buildpacks: []string{ + "example.com/some/package", + }, + PullPolicy: image.PullAlways, + })) + + args := fakeImageFetcher.FetchCalls[defaultBuilderName] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "") + + args = fakeImageFetcher.FetchCalls["default/run"] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "linux/amd64") + + args = fakeImageFetcher.FetchCalls[fakePackage.Name()] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "linux/amd64") + + args = fakeImageFetcher.FetchCalls[fmt.Sprintf("%s:%s", cfg.DefaultLifecycleImageRepo, builder.DefaultLifecycleVersion)] + h.AssertEq(t, args.Daemon, true) + h.AssertEq(t, args.PullPolicy, image.PullAlways) + h.AssertEq(t, args.Platform, "linux/amd64") + }) + }) + }) + when("PullPolicy", func() { when("never", func() { it("uses the local builder and run images without updating", func() { diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 3d34d1d02e..9c85da48f9 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -110,9 +110,12 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) f.logger.Debugf("Pulling image %s", style.Symbol(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") { + // FIXME: this matching is brittle and the fallback should be removed when https://github.com/buildpacks/pack/issues/2079 + // has been fixed for a sufficient amount of time. + // 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") && + strings.HasSuffix(strings.TrimSpace(err.Error()), "actual: linux") { err = f.pullImage(ctx, name, "") } } diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index b9491b24c2..a391f0f8f3 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -232,7 +232,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, img.Save()) }) - it("retry without setting platform", func() { + it("retries 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 6f50db6611d2d0c4da22f4123c8ef433f27fc6cb Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 20 May 2024 15:34:36 -0400 Subject: [PATCH 2/8] Fix acceptance by skipping test if feature not supported Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 1 + acceptance/invoke/pack.go | 4 ++++ pkg/client/build.go | 4 ++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index cb3bb66125..c1cd457a8c 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2512,6 +2512,7 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] when("--platform", func() { wrongArch := "arm64" it.Before(func() { + h.SkipIf(t, !pack.SupportsFeature(invoke.PlatformOption), "") h.SkipIf(t, imageManager.HostOS() == "windows", "Not relevant on windows") if hostArch := imageManager.HostArch(); hostArch == wrongArch { wrongArch = "amd64" diff --git a/acceptance/invoke/pack.go b/acceptance/invoke/pack.go index d63170d1eb..e225983bb6 100644 --- a/acceptance/invoke/pack.go +++ b/acceptance/invoke/pack.go @@ -239,6 +239,7 @@ const ( FlattenBuilderCreationV2 FixesRunImageMetadata ManifestCommands + PlatformOption ) var featureTests = map[Feature]func(i *PackInvoker) bool{ @@ -278,6 +279,9 @@ var featureTests = map[Feature]func(i *PackInvoker) bool{ ManifestCommands: func(i *PackInvoker) bool { return i.atLeast("v0.34.0") }, + PlatformOption: func(i *PackInvoker) bool { + return i.atLeast("v0.34.0") + }, } func (i *PackInvoker) SupportsFeature(f Feature) bool { diff --git a/pkg/client/build.go b/pkg/client/build.go index 08edbf4e6f..7c58ad6afa 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -347,7 +347,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { platformToUse := opts.Platform if platformToUse == "" { - platformToUse = fmt.Sprintf("%s/%s", builderOS, builderArch) // TODO: what about arch variant, etc. + platformToUse = fmt.Sprintf("%s/%s", builderOS, builderArch) // FIXME: what about arch variant, etc. } bldr, err := c.getBuilder(rawBuilderImage) @@ -1231,7 +1231,7 @@ func (c *Client) fetchBuildpack(ctx context.Context, bp string, relativeBaseDir platformToUse := opts.Platform if platformToUse == "" { - platformToUse = fmt.Sprintf("%s/%s", builderOS, builderArch) // TODO: what about arch variant, etc. + platformToUse = fmt.Sprintf("%s/%s", builderOS, builderArch) // FIXME: what about arch variant, etc. } downloadOptions := buildpack.DownloadOptions{ From f2cd826552e324512c8e2440b7aa63d06c9f9849 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 20 May 2024 16:00:00 -0400 Subject: [PATCH 3/8] Fix tests - WCOW units by checking for 'windows' in error string - LCOW acceptance by printing a log line and checking it instead of expecting an error Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 6 ++---- pkg/image/fetcher.go | 11 +++++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index c1cd457a8c..53e82b7917 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2517,17 +2517,15 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] if hostArch := imageManager.HostArch(); hostArch == wrongArch { wrongArch = "amd64" } - // FIXME: on an M1 with emulation enabled this test might pass when we expect it to fail }) it("uses the builder with the desired platform", func() { - output, err := pack.Run( + output, _ := pack.Run( "build", repoName, "-p", filepath.Join("testdata", "mock_app"), "--platform", fmt.Sprintf("linux/%s", wrongArch), ) - h.AssertNotNil(t, err) - h.AssertContains(t, output, "was found but does not match the specified platform") + h.AssertContainsMatch(t, output, fmt.Sprintf("Pulling image '.+' with platform 'linux/%s'", wrongArch)) }) }) }) diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 9c85da48f9..5f59e17782 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "fmt" "io" "strings" @@ -108,14 +109,20 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) } } - f.logger.Debugf("Pulling image %s", style.Symbol(name)) + msg := fmt.Sprintf("Pulling image %s", style.Symbol(name)) + if options.Platform != "" { + msg = fmt.Sprintf("Pulling image %s with platform %s", style.Symbol(name), style.Symbol(options.Platform)) + } + f.logger.Debug(msg) if err = f.pullImage(ctx, name, options.Platform); err != nil { // FIXME: this matching is brittle and the fallback should be removed when https://github.com/buildpacks/pack/issues/2079 // has been fixed for a sufficient amount of time. // 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") && - strings.HasSuffix(strings.TrimSpace(err.Error()), "actual: linux") { + (strings.HasSuffix(strings.TrimSpace(err.Error()), "actual: linux") || + strings.HasSuffix(strings.TrimSpace(err.Error()), "actual: windows")) { + f.logger.Debugf(fmt.Sprintf("Pulling image %s", style.Symbol(name))) err = f.pullImage(ctx, name, "") } } From f8a10339367011316d0a9edb527a234c9e909bbd Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 20 May 2024 17:12:51 -0400 Subject: [PATCH 4/8] Make acceptance test a little more robust Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 16 ++++------------ acceptance/managers/image_manager.go | 7 ------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 53e82b7917..59557d702b 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2510,22 +2510,14 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] }) when("--platform", func() { - wrongArch := "arm64" - it.Before(func() { - h.SkipIf(t, !pack.SupportsFeature(invoke.PlatformOption), "") - h.SkipIf(t, imageManager.HostOS() == "windows", "Not relevant on windows") - if hostArch := imageManager.HostArch(); hostArch == wrongArch { - wrongArch = "amd64" - } - }) - it("uses the builder with the desired platform", func() { - output, _ := pack.Run( + output, err := pack.Run( "build", repoName, "-p", filepath.Join("testdata", "mock_app"), - "--platform", fmt.Sprintf("linux/%s", wrongArch), + "--platform", "linux/not-exist-arch", ) - h.AssertContainsMatch(t, output, fmt.Sprintf("Pulling image '.+' with platform 'linux/%s'", wrongArch)) + h.AssertNotNil(t, err) + h.AssertContainsMatch(t, output, "Pulling image '.*test/builder.*' with platform 'linux/not-exist-arch") }) }) }) diff --git a/acceptance/managers/image_manager.go b/acceptance/managers/image_manager.go index ca3baa554b..b968b4ceaa 100644 --- a/acceptance/managers/image_manager.go +++ b/acceptance/managers/image_manager.go @@ -62,13 +62,6 @@ func (im ImageManager) HostOS() string { return daemonInfo.OSType } -func (im ImageManager) HostArch() string { - im.testObject.Helper() - daemonInfo, err := im.dockerCli.Info(context.Background()) - im.assert.Nil(err) - return daemonInfo.Architecture -} - func (im ImageManager) TagImage(image, ref string) { im.testObject.Helper() err := im.dockerCli.ImageTag(context.Background(), image, ref) From da674445a57a18a383c249468ef59cd71d08ace4 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 20 May 2024 17:22:01 -0400 Subject: [PATCH 5/8] Bring back needed skip Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 59557d702b..65548ebbae 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2510,6 +2510,10 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] }) when("--platform", func() { + it.Before(func() { + h.SkipIf(t, !pack.SupportsFeature(invoke.PlatformOption), "") + }) + it("uses the builder with the desired platform", func() { output, err := pack.Run( "build", repoName, From 84930f51131025f2f378f97a969626d75cded11c Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 21 May 2024 10:15:25 -0400 Subject: [PATCH 6/8] Remove error check LCOW appears to fallback to the arch it can find Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 65548ebbae..63e96035c3 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2515,12 +2515,11 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] }) it("uses the builder with the desired platform", func() { - output, err := pack.Run( + output, _ := pack.Run( "build", repoName, "-p", filepath.Join("testdata", "mock_app"), "--platform", "linux/not-exist-arch", ) - h.AssertNotNil(t, err) h.AssertContainsMatch(t, output, "Pulling image '.*test/builder.*' with platform 'linux/not-exist-arch") }) }) From 08995083ff7fdf4ad6999666fe69b7a9991844f8 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 23 May 2024 09:37:06 -0400 Subject: [PATCH 7/8] Remove unneeded things Signed-off-by: Natalie Arellano --- pkg/client/build.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/client/build.go b/pkg/client/build.go index 5720509c9e..aea26e89bc 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -516,13 +516,8 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { } defer c.docker.ImageRemove(context.Background(), ephemeralBuilder.Name(), types.RemoveOptions{Force: true}) - builderOS, err := rawBuilderImage.OS() - if err != nil { - return fmt.Errorf("failed to get builder OS: %w", err) - } - if len(bldr.OrderExtensions()) > 0 || len(ephemeralBuilder.OrderExtensions()) > 0 { - if builderOS == "windows" { + if targetToUse.OS == "windows" { return fmt.Errorf("builder contains image extensions which are not supported for Windows builds") } if !(opts.PullPolicy == image.PullAlways) { @@ -534,7 +529,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { opts.ContainerConfig.Volumes = appendLayoutVolumes(opts.ContainerConfig.Volumes, pathsConfig) } - processedVolumes, warnings, err := processVolumes(builderOS, opts.ContainerConfig.Volumes) + processedVolumes, warnings, err := processVolumes(targetToUse.OS, opts.ContainerConfig.Volumes) if err != nil { return err } @@ -1258,7 +1253,6 @@ func (c *Client) fetchBuildpack(ctx context.Context, bp string, relativeBaseDir default: downloadOptions := buildpack.DownloadOptions{ RegistryName: registry, - ImageOS: targetToUse.OS, Target: targetToUse, RelativeBaseDir: relativeBaseDir, Daemon: !publish, From 89cb6173808d0f159658869debc8fd3e43cb9aed Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 23 May 2024 09:43:56 -0400 Subject: [PATCH 8/8] Dedup test setup Signed-off-by: Natalie Arellano --- pkg/client/build_test.go | 231 ++++++++++++++------------------------- 1 file changed, 81 insertions(+), 150 deletions(-) diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 03b0984ab0..ec6a5d39a1 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -1122,81 +1122,7 @@ api = "0.2" var fakePackage *fakes.Image it.Before(func() { - metaBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ - WithAPI: api.MustParse("0.3"), - WithInfo: dist.ModuleInfo{ - ID: "meta.buildpack.id", - Version: "meta.buildpack.version", - Homepage: "http://meta.buildpack", - }, - WithStacks: nil, - WithOrder: dist.Order{{ - Group: []dist.ModuleRef{{ - ModuleInfo: dist.ModuleInfo{ - ID: "child.buildpack.id", - Version: "child.buildpack.version", - }, - Optional: false, - }}, - }}, - }) - - childBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ - WithAPI: api.MustParse("0.3"), - WithInfo: dist.ModuleInfo{ - ID: "child.buildpack.id", - Version: "child.buildpack.version", - Homepage: "http://child.buildpack", - }, - WithStacks: []dist.Stack{ - {ID: defaultBuilderStackID}, - }, - }) - - bpLayers := dist.ModuleLayers{ - "meta.buildpack.id": { - "meta.buildpack.version": { - API: api.MustParse("0.3"), - Order: dist.Order{{ - Group: []dist.ModuleRef{{ - ModuleInfo: dist.ModuleInfo{ - ID: "child.buildpack.id", - Version: "child.buildpack.version", - }, - Optional: false, - }}, - }}, - LayerDiffID: diffIDForFile(t, metaBuildpackTar), - }, - }, - "child.buildpack.id": { - "child.buildpack.version": { - API: api.MustParse("0.3"), - Stacks: []dist.Stack{ - {ID: defaultBuilderStackID}, - }, - LayerDiffID: diffIDForFile(t, childBuildpackTar), - }, - }, - } - - md := buildpack.Metadata{ - ModuleInfo: dist.ModuleInfo{ - ID: "meta.buildpack.id", - Version: "meta.buildpack.version", - }, - Stacks: []dist.Stack{ - {ID: defaultBuilderStackID}, - }, - } - - fakePackage = fakes.NewImage("example.com/some/package", "", nil) - h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpack.layers", bpLayers)) - h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpackage.metadata", md)) - - h.AssertNil(t, fakePackage.AddLayer(metaBuildpackTar)) - h.AssertNil(t, fakePackage.AddLayer(childBuildpackTar)) - + fakePackage = makeFakePackage(t, tmpDir, defaultBuilderStackID) fakeImageFetcher.LocalImages[fakePackage.Name()] = fakePackage }) @@ -2265,81 +2191,7 @@ api = "0.2" var fakePackage imgutil.Image it.Before(func() { - metaBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ - WithAPI: api.MustParse("0.3"), - WithInfo: dist.ModuleInfo{ - ID: "meta.buildpack.id", - Version: "meta.buildpack.version", - Homepage: "http://meta.buildpack", - }, - WithStacks: nil, - WithOrder: dist.Order{{ - Group: []dist.ModuleRef{{ - ModuleInfo: dist.ModuleInfo{ - ID: "child.buildpack.id", - Version: "child.buildpack.version", - }, - Optional: false, - }}, - }}, - }) - - childBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ - WithAPI: api.MustParse("0.3"), - WithInfo: dist.ModuleInfo{ - ID: "child.buildpack.id", - Version: "child.buildpack.version", - Homepage: "http://child.buildpack", - }, - WithStacks: []dist.Stack{ - {ID: defaultBuilderStackID}, - }, - }) - - bpLayers := dist.ModuleLayers{ - "meta.buildpack.id": { - "meta.buildpack.version": { - API: api.MustParse("0.3"), - Order: dist.Order{{ - Group: []dist.ModuleRef{{ - ModuleInfo: dist.ModuleInfo{ - ID: "child.buildpack.id", - Version: "child.buildpack.version", - }, - Optional: false, - }}, - }}, - LayerDiffID: diffIDForFile(t, metaBuildpackTar), - }, - }, - "child.buildpack.id": { - "child.buildpack.version": { - API: api.MustParse("0.3"), - Stacks: []dist.Stack{ - {ID: defaultBuilderStackID}, - }, - LayerDiffID: diffIDForFile(t, childBuildpackTar), - }, - }, - } - - md := buildpack.Metadata{ - ModuleInfo: dist.ModuleInfo{ - ID: "meta.buildpack.id", - Version: "meta.buildpack.version", - }, - Stacks: []dist.Stack{ - {ID: defaultBuilderStackID}, - }, - } - - fakePackage = fakes.NewImage("example.com/some/package", "", nil) - h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpack.layers", bpLayers)) - h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpackage.metadata", md)) - - h.AssertNil(t, fakePackage.AddLayer(metaBuildpackTar)) - h.AssertNil(t, fakePackage.AddLayer(childBuildpackTar)) - + fakePackage = makeFakePackage(t, tmpDir, defaultBuilderStackID) fakeImageFetcher.LocalImages[fakePackage.Name()] = fakePackage }) @@ -3325,6 +3177,85 @@ api = "0.2" }) } +func makeFakePackage(t *testing.T, tmpDir string, stackID string) *fakes.Image { + metaBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + WithAPI: api.MustParse("0.3"), + WithInfo: dist.ModuleInfo{ + ID: "meta.buildpack.id", + Version: "meta.buildpack.version", + Homepage: "http://meta.buildpack", + }, + WithStacks: nil, + WithOrder: dist.Order{{ + Group: []dist.ModuleRef{{ + ModuleInfo: dist.ModuleInfo{ + ID: "child.buildpack.id", + Version: "child.buildpack.version", + }, + Optional: false, + }}, + }}, + }) + + childBuildpackTar := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + WithAPI: api.MustParse("0.3"), + WithInfo: dist.ModuleInfo{ + ID: "child.buildpack.id", + Version: "child.buildpack.version", + Homepage: "http://child.buildpack", + }, + WithStacks: []dist.Stack{ + {ID: stackID}, + }, + }) + + bpLayers := dist.ModuleLayers{ + "meta.buildpack.id": { + "meta.buildpack.version": { + API: api.MustParse("0.3"), + Order: dist.Order{{ + Group: []dist.ModuleRef{{ + ModuleInfo: dist.ModuleInfo{ + ID: "child.buildpack.id", + Version: "child.buildpack.version", + }, + Optional: false, + }}, + }}, + LayerDiffID: diffIDForFile(t, metaBuildpackTar), + }, + }, + "child.buildpack.id": { + "child.buildpack.version": { + API: api.MustParse("0.3"), + Stacks: []dist.Stack{ + {ID: stackID}, + }, + LayerDiffID: diffIDForFile(t, childBuildpackTar), + }, + }, + } + + md := buildpack.Metadata{ + ModuleInfo: dist.ModuleInfo{ + ID: "meta.buildpack.id", + Version: "meta.buildpack.version", + }, + Stacks: []dist.Stack{ + {ID: stackID}, + }, + } + + fakePackage := fakes.NewImage("example.com/some/package", "", nil) + h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpack.layers", bpLayers)) + h.AssertNil(t, dist.SetLabel(fakePackage, "io.buildpacks.buildpackage.metadata", md)) + + h.AssertNil(t, fakePackage.AddLayer(metaBuildpackTar)) + h.AssertNil(t, fakePackage.AddLayer(childBuildpackTar)) + + return fakePackage +} + func diffIDForFile(t *testing.T, path string) string { file, err := os.Open(path) h.AssertNil(t, err)