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

Support pack build --platform #2162

Merged
merged 9 commits into from
May 23, 2024
22 changes: 22 additions & 0 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2508,6 +2508,28 @@ 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"
}
// 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 <flattened buildpack>", func() {
Expand Down
4 changes: 4 additions & 0 deletions acceptance/invoke/pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ const (
FlattenBuilderCreationV2
FixesRunImageMetadata
ManifestCommands
PlatformOption
)

var featureTests = map[Feature]func(i *PackInvoker) bool{
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions acceptance/managers/image_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
5 changes: 5 additions & 0 deletions internal/commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type BuildFlags struct {
Builder string
Registry string
RunImage string
Platform string
Policy string
Network string
DescriptorPath string
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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)")
Expand Down
20 changes: 20 additions & 0 deletions internal/commands/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down Expand Up @@ -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),
Expand Down
29 changes: 25 additions & 4 deletions pkg/client/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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())
}
Expand All @@ -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) // FIXME: what about arch variant, etc.
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved
}

bldr, err := c.getBuilder(rawBuilderImage)
if err != nil {
return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder))
Expand All @@ -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)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) // FIXME: 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,
Expand Down
152 changes: 152 additions & 0 deletions pkg/client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved

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() {
Expand Down
9 changes: 6 additions & 3 deletions pkg/image/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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") {
// 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 <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") &&
strings.HasSuffix(strings.TrimSpace(err.Error()), "actual: linux") {
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to make the acceptance test work, but I think it's actually closer to what we want anyway

err = f.pullImage(ctx, name, "")
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
Loading