Merge pull request #2196 from buildpacks/lazy-ephemeral-builder

Don't create an ephemeral builder if it isn't truly needed

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()))

pkg/client/build.go

@@ -510,11 +510,25 @@ 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
 	}
-	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" {
@@ -1502,6 +1516,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 +1555,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)

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{