Explode flattened buildpacks when `pack build` #1787

jjbustamante · 2023-05-30T20:39:50Z

Summary

After the pack 0.30.0-pre2 was released some users reported some troubles when trying to use the new flatten flag added to the pack builder create and pack buildpack package commands. See slack thread

When pack build app --buildpack was used with a flattened buildpack on a GHA runner it was causing the runner to run out of disk space
When pack build app --buildpack was used with a flattened buildpack that is also present into the builder we want to use, pack was not able to deduplicate those buildpacks correctly.

This PR is trying to fix both issues.

Output

Before

Issue 1, when a buildpack is flattened we are saving in the same layer N number of buildpacks, during pack build an ephemeral builder is created, in this process we read each buildpack layer tar to disk in a temp folder and then create this builder (that is deleted at the end of the command execution) adding those tars to it. Because each flattened layer contains more than 1 buildpack, the iteration over the layers creates a file on disk with all the flattened buildpacks. that is the reason for the process to consume more hard drive space.
Issue 2, when pack is creating the ephemeral builder and some buildpacks are pass through -b flag, pack calculates the hash of the tar file inside the provided buildpack and then compares this value against the hash of the same buildpack in the builder, because the provided buildpack is flattened (there are more than 1 buildpack in the layer) we need to split those buildpacks into individual ones.

After

To fix issue 1: This PR adds some logic to keep track of the layers DiffIds when a buildpack is fetched, if the same layer DiffID is found in more than 1 buildpack then only 1 of them returns the content of the layer tar (with all the other buildpacks inside of it) and the other returns an empty tar. In this way we only use the same hard drive space when the layer tar is written into disk.
To fix issue 2: This PR adds some logic when a flattened buildpack is fetched, Pack will try to write different tar files for each buildpack flattened inside a layer and after that the calculation of the hash to execute the deduplication logic remains the same.

Documentation

Should this change be documented?
- Yes, see #___
- No

When `pack build --buildpack <flattened buildpack>` is invoked, `pack` doesn't know the buildpack is flattened, so it treats it like `pack build --buildpack bp/1 --buildpack bp2`, resulting in unnecessary tar files being created and duplicate layers being added to the ephemeral builder, which can exhaust disk space depending on the environment. This optimization avoids writing the tar file more than once by returning a no-op read closer when the diffID has already been queued for downloading. Signed-off-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

internal/builder/builder.go

pkg/buildpack/buildpack.go

calculate which buildpacks must be added into the ephemeral builder Skipping pack build -b <flattened-buildpack> in windows container because acceptance tests doesn't support it yet Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

internal/builder/builder.go

pkg/buildpack/buildpack.go

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

jjbustamante · 2023-06-29T23:32:49Z

pkg/buildpack/buildpack.go

+	// write the rest of the package
+	var header *tar.Header
+	for {
+		header, err = tr.Next()


handled at line 435

jjbustamante · 2023-06-29T23:32:37Z

pkg/buildpack/buildpack.go

+	if origVersion == "" {
+		// the first header only contains the id - e.g., /cnb/buildpacks/<buildpack-id>,
+		// read the next header to get the version
+		secondHeader, err := tr.Next()


handled at line 397

jjbustamante · 2023-06-29T23:32:21Z

pkg/buildpack/buildpack.go

+	)
+
+	for {
+		header, err = tr.Next()


Handled at line 353

Perhaps we don't need to do it as part of this PR (because we use tar readers all over) but it would be cleaner to wrap tar.NewReader with our own reader that sanitizes the path as part of calling tr.Next()

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

jjbustamante · 2023-06-30T18:05:08Z

@natalieparellano @joe-kimmel-vmw @jkutner I will love if you can review the PR again

natalieparellano

Looks good to me :) Thanks for all of your hard work in getting this over the finish line @jjbustamante

joe-kimmel-vmw · 2023-07-06T17:18:54Z

IMO this is in good shape. I did make one tiny suggestion in this PR (set to merge into this branch): https://github.com/buildpacks/pack/pull/1829/files

github-actions bot added this to the 0.30.0 milestone May 30, 2023

github-actions bot added type/chore Issue that requests non-user facing changes. type/enhancement Issue that requests a new feature or improvement. labels May 30, 2023

jjbustamante force-pushed the fix/flatten-tar-extras-2 branch from 215b882 to e610d3f Compare May 30, 2023 20:41

github-advanced-security bot found potential problems May 30, 2023

View reviewed changes

pkg/buildpack/buildpack.go Fixed Show fixed Hide fixed

jjbustamante added 2 commits June 2, 2023 15:08

WIP

362052a

Signed-off-by: Natalie Arellano <narellano@vmware.com> Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

jjbustamante force-pushed the fix/flatten-tar-extras-2 branch 2 times, most recently from 3cb9f5a to 6f76a88 Compare June 7, 2023 20:27

jjbustamante added type/bug Issue that reports an unexpected behaviour. and removed type/enhancement Issue that requests a new feature or improvement. labels Jun 7, 2023

github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Jun 8, 2023

jjbustamante force-pushed the fix/flatten-tar-extras-2 branch 2 times, most recently from 97e1fda to c8041e7 Compare June 9, 2023 16:24

jjbustamante commented Jun 9, 2023

View reviewed changes

internal/builder/builder.go Show resolved Hide resolved

jjbustamante force-pushed the fix/flatten-tar-extras-2 branch 2 times, most recently from 285b0dc to da011d3 Compare June 13, 2023 18:42

jjbustamante added flatten feature and removed type/chore Issue that requests non-user facing changes. labels Jun 13, 2023

jjbustamante mentioned this pull request Jun 13, 2023

Fixups for flatten feature on pack 0.30.0 #1798

Closed

5 tasks

github-actions bot added the type/chore Issue that requests non-user facing changes. label Jun 14, 2023

github-advanced-security bot found potential problems Jun 15, 2023

View reviewed changes

pkg/buildpack/buildpack.go Fixed Show resolved Hide resolved

jjbustamante commented Jun 15, 2023

View reviewed changes

pkg/buildpack/buildpack.go Outdated Show resolved Hide resolved

jjbustamante commented Jun 15, 2023

View reviewed changes

pkg/buildpack/buildpack.go Outdated Show resolved Hide resolved

jjbustamante force-pushed the fix/flatten-tar-extras-2 branch 2 times, most recently from 56fc8bf to d4441ca Compare June 15, 2023 02:03

jjbustamante force-pushed the fix/flatten-tar-extras-2 branch from d4441ca to 03808b7 Compare June 15, 2023 13:33

jjbustamante added 2 commits June 15, 2023 08:52

skipping --flatten test on previous versions of pack

f5cce3c

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

buildpack directories not supported on windows

c456691

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

natalieparellano and others added 3 commits June 26, 2023 12:59

Removing skips on Windows

39f29d2

Signed-off-by: Natalie Arellano <narellano@vmware.com>

for -> from

b17287b

Signed-off-by: Natalie Arellano <narellano@vmware.com>

fixing windows issue

31060e0

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

natalieparellano reviewed Jun 27, 2023

View reviewed changes

pkg/buildpack/buildpack.go Outdated Show resolved Hide resolved

Cleanups from PR review

6f50a75

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano mentioned this pull request Jun 27, 2023

Cleanups from PR review #1813

Merged

natalieparellano and others added 7 commits June 27, 2023 13:17

Fix more stuff

e7c0a12

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Update comment

cead534

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Variable rename and pend tests to see if acceptance is passing

e14d0d9

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Fix units by not writing parent headers twice

03116fe

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Try to fix Windows

84ee822

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Other small cleanups

d5cc7d9

Signed-off-by: Natalie Arellano <narellano@vmware.com>

merging suggestions from Natalie and Joe to simplify the code

e73699a

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

github-advanced-security bot found potential problems Jun 28, 2023

View reviewed changes

jjbustamante added 5 commits June 28, 2023 14:07

Merge branch 'main' into fix/flatten-tar-extras-2

744aefc

removing unused code

c5aa199

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

adding test coverage

d5ab961

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

Attempt to fix CodeQL errors

f956222

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

removing Focus

d9a254b

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

jjbustamante requested review from natalieparellano, joe-kimmel-vmw and jkutner June 30, 2023 18:04

natalieparellano approved these changes Jul 5, 2023

View reviewed changes

jkutner approved these changes Jul 10, 2023

View reviewed changes

jkutner merged commit 34a6b7c into main Jul 10, 2023

jkutner deleted the fix/flatten-tar-extras-2 branch July 10, 2023 00:52

chenbh mentioned this pull request Jul 25, 2023

Deduplicate builder layers by diff id buildpacks-community/kpack#1232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explode flattened buildpacks when `pack build` #1787

Explode flattened buildpacks when `pack build` #1787

jjbustamante commented May 30, 2023 •

edited

Loading

jjbustamante Jun 29, 2023

jjbustamante Jun 29, 2023

jjbustamante Jun 29, 2023

natalieparellano Jul 5, 2023

jjbustamante commented Jun 30, 2023

natalieparellano left a comment

joe-kimmel-vmw commented Jul 6, 2023

Explode flattened buildpacks when pack build #1787

Explode flattened buildpacks when pack build #1787

Conversation

jjbustamante commented May 30, 2023 • edited Loading

Summary

Output

Before

After

Documentation

Related

jjbustamante Jun 29, 2023

Choose a reason for hiding this comment

jjbustamante Jun 29, 2023

Choose a reason for hiding this comment

jjbustamante Jun 29, 2023

Choose a reason for hiding this comment

natalieparellano Jul 5, 2023

Choose a reason for hiding this comment

jjbustamante commented Jun 30, 2023

natalieparellano left a comment

Choose a reason for hiding this comment

joe-kimmel-vmw commented Jul 6, 2023

Explode flattened buildpacks when `pack build` #1787

Explode flattened buildpacks when `pack build` #1787

jjbustamante commented May 30, 2023 •

edited

Loading