Skip to content

Commit

Permalink
Don't create an ephemeral builder if it isn't truly needed
Browse files Browse the repository at this point in the history
Fixes #2195

Signed-off-by: Natalie Arellano <[email protected]>
  • Loading branch information
natalieparellano committed Jul 2, 2024
1 parent ce8db3c commit 55f7f36
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 6 deletions.
23 changes: 18 additions & 5 deletions internal/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type Builder struct {
order dist.Order
orderExtensions dist.Order
validateMixins bool
saveProhibited bool
}

type orderTOML struct {
Expand All @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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()))
Expand Down
44 changes: 43 additions & 1 deletion pkg/client/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions pkg/client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 55f7f36

Please sign in to comment.