From 55f7f36825b0c3ce9398f2faa97b27543087a452 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 2 Jul 2024 13:34:50 -0400 Subject: [PATCH 1/2] Don't create an ephemeral builder if it isn't truly needed Fixes https://github.com/buildpacks/pack/issues/2195 Signed-off-by: Natalie Arellano --- internal/builder/builder.go | 23 ++++++++++++++----- pkg/client/build.go | 44 ++++++++++++++++++++++++++++++++++++- pkg/client/build_test.go | 12 ++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 069a00935..adffa98cb 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -85,6 +85,7 @@ type Builder struct { order dist.Order orderExtensions dist.Order validateMixins bool + saveProhibited bool } type orderTOML struct { @@ -102,9 +103,10 @@ type moduleWithDiffID struct { type BuilderOption func(*options) error type options struct { - toFlatten buildpack.FlattenModuleInfos - labels map[string]string - runImage string + toFlatten buildpack.FlattenModuleInfos + labels map[string]string + runImage string + saveProhibited bool } func WithRunImage(name string) BuilderOption { @@ -114,6 +116,13 @@ func WithRunImage(name string) BuilderOption { } } +func WithoutSave() BuilderOption { + return func(o *options) error { + o.saveProhibited = true + return nil + } +} + // FromImage constructs a builder from a builder image func FromImage(img imgutil.Image) (*Builder, error) { return constructBuilder(img, "", true) @@ -149,8 +158,7 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, } if opts.runImage != "" { - // Do we need to look for available mirrors? for now the mirrors are gone if you override the run-image - // create an issue if you want to preserve the mirrors + // FIXME: for now the mirrors are gone if you override the run-image (open an issue if preserving the mirrors is desired) metadata.RunImages = []RunImageMetadata{{Image: opts.runImage}} metadata.Stack.RunImage = RunImageMetadata{Image: opts.runImage} } @@ -173,6 +181,7 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, validateMixins: true, additionalBuildpacks: buildpack.NewManagedCollectionV2(opts.toFlatten), additionalExtensions: buildpack.NewManagedCollectionV2(opts.toFlatten), + saveProhibited: opts.saveProhibited, } if err := addImgLabelsToBuildr(bldr); err != nil { @@ -435,6 +444,10 @@ func (b *Builder) SetValidateMixins(to bool) { // Save saves the builder func (b *Builder) Save(logger logging.Logger, creatorMetadata CreatorMetadata) error { + if b.saveProhibited { + return fmt.Errorf("failed to save builder %s as saving is not allowed", b.Name()) + } + logger.Debugf("Creating builder with the following buildpacks:") for _, bpInfo := range b.metadata.Buildpacks { logger.Debugf("-> %s", style.Symbol(bpInfo.FullName())) diff --git a/pkg/client/build.go b/pkg/client/build.go index 4825f09f3..9578d1e7e 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -510,7 +510,16 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { buildEnvs[k] = v } - ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, buildEnvs, order, fetchedBPs, orderExtensions, fetchedExs, usingPlatformAPI.LessThan("0.12"), opts.RunImage) + ephemeralBuilder, err := c.createEphemeralBuilder( + rawBuilderImage, + buildEnvs, + order, + fetchedBPs, + orderExtensions, + fetchedExs, + usingPlatformAPI.LessThan("0.12"), + opts.RunImage, + ) if err != nil { return err } @@ -1502,6 +1511,10 @@ func (c *Client) createEphemeralBuilder( validateMixins bool, runImage string, ) (*builder.Builder, error) { + if !ephemeralBuilderNeeded(env, order, buildpacks, orderExtensions, extensions, runImage) { + return builder.New(rawBuilderImage, rawBuilderImage.Name(), builder.WithoutSave()) + } + origBuilderName := rawBuilderImage.Name() bldr, err := builder.New(rawBuilderImage, fmt.Sprintf("pack.local/builder/%x:latest", randString(10)), builder.WithRunImage(runImage)) if err != nil { @@ -1537,6 +1550,35 @@ func (c *Client) createEphemeralBuilder( return bldr, nil } +func ephemeralBuilderNeeded( + env map[string]string, + order dist.Order, + buildpacks []buildpack.BuildModule, + orderExtensions dist.Order, + extensions []buildpack.BuildModule, + runImage string, +) bool { + if len(env) > 0 { + return true + } + if len(order) > 0 && len(order[0].Group) > 0 { + return true + } + if len(buildpacks) > 0 { + return true + } + if len(orderExtensions) > 0 && len(orderExtensions[0].Group) > 0 { + return true + } + if len(extensions) > 0 { + return true + } + if runImage != "" { + return true + } + return false +} + // Returns a string iwith lowercase a-z, of length n func randString(n int) string { b := make([]byte, n) diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index ec6a5d39a..0c97f1697 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -150,6 +150,18 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }) when("#Build", func() { + when("ephemeral builder is not needed", func() { + it("does not create one", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Builder: defaultBuilderName, + Image: "example.com/some/repo:tag", + })) + h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderName) + bldr := fakeLifecycle.Opts.Builder.(*builder.Builder) + h.AssertNotNil(t, bldr.Save(logger, builder.CreatorMetadata{})) // it shouldn't be possible to save this builder, as that would overwrite the original builder + }) + }) + when("Workspace option", func() { it("uses the specified dir", func() { h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ From 423f59667110a4c602955debf835a6b15220de57 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 2 Jul 2024 17:22:23 -0400 Subject: [PATCH 2/2] Don't cleanup the ephemeral builder if it is the original builder Signed-off-by: Natalie Arellano --- pkg/client/build.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/client/build.go b/pkg/client/build.go index 9578d1e7e..960f4038c 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -523,7 +523,12 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { if err != nil { return err } - defer c.docker.ImageRemove(context.Background(), ephemeralBuilder.Name(), types.RemoveOptions{Force: true}) + defer func() { + if ephemeralBuilder.Name() == rawBuilderImage.Name() { + return + } + _, _ = c.docker.ImageRemove(context.Background(), ephemeralBuilder.Name(), types.RemoveOptions{Force: true}) + }() if len(bldr.OrderExtensions()) > 0 || len(ephemeralBuilder.OrderExtensions()) > 0 { if targetToUse.OS == "windows" {