Skip to content

Commit

Permalink
Deduplicate builder layers by diff id
Browse files Browse the repository at this point in the history
- This will remove duplicate layers that result from two buildpacks in a builder that have the same layer diff id

Co-authored-by Matthew McNew <[email protected]>
  • Loading branch information
tomkennedy513 authored and sambhav committed Jul 25, 2023
1 parent a68725c commit b4089a3
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 4 deletions.
24 changes: 22 additions & 2 deletions pkg/cnb/builder_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (bb *builderBlder) WriteableImage() (v1.Image, error) {
}

image, err := mutate.AppendLayers(bb.baseImage,
layers(
deduplicateLayers(layers(
[]v1.Layer{
defaultLayer,
bb.lifecycleLayer,
Expand All @@ -136,7 +136,7 @@ func (bb *builderBlder) WriteableImage() (v1.Image, error) {
stackLayer,
orderLayer,
},
)...)
))...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -388,3 +388,23 @@ func layers(layers ...[]v1.Layer) []v1.Layer {
}
return appendedLayers
}

func deduplicateLayers(layers []v1.Layer) []v1.Layer {
layerMap := map[v1.Hash]struct{}{}
res := make([]v1.Layer, 0)

for _, l := range layers {
diffId, err := l.DiffID()
if err != nil {
res = append(res, l)
continue
}

if _, ok := layerMap[diffId]; !ok {
res = append(res, l)
layerMap[diffId] = struct{}{}
}
}

return res
}
62 changes: 60 additions & 2 deletions pkg/cnb/create_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
},
Optional: true,
},
{
BuildpackInfo: corev1alpha1.BuildpackInfo{
Id: "io.buildpack.4",
Version: "v4",
},
},
},
},
},
Expand Down Expand Up @@ -251,6 +257,30 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
},
})

buildpackWithDuplicateLayer := buildpackLayer{
v1Layer: buildpack3Layer,
BuildpackInfo: DescriptiveBuildpackInfo{
BuildpackInfo: corev1alpha1.BuildpackInfo{
Id: "io.buildpack.4",
Version: "v4",
},
Homepage: "buildpack.4.com",
},
BuildpackLayerInfo: BuildpackLayerInfo{
API: "0.3",
LayerDiffID: buildpack3Layer.diffID,
Stacks: []corev1alpha1.BuildpackStack{
{
ID: stackID,
},
{
ID: "io.some.other.stack",
},
},
},
}
buildpackRepository.AddBP("io.buildpack.4", "v4", []buildpackLayer{buildpackWithDuplicateLayer})

registryClient.AddSaveKeychain("custom/example", keychain)

when("CreateBuilder", func() {
Expand Down Expand Up @@ -301,10 +331,11 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
builderRecord, err := subject.CreateBuilder(keychain, store, stack, clusterBuilderSpec)
require.NoError(t, err)

assert.Len(t, builderRecord.Buildpacks, 3)
assert.Len(t, builderRecord.Buildpacks, 4)
assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.1", Version: "v1", Homepage: "buildpack.1.com"})
assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.2", Version: "v2", Homepage: "buildpack.2.com"})
assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.3", Version: "v3", Homepage: "buildpack.3.com"})
assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.4", Version: "v4", Homepage: "buildpack.4.com"})
assert.Equal(t, corev1alpha1.BuildStack{RunImage: runImage, ID: stackID}, builderRecord.Stack)
assert.Equal(t, int64(10), builderRecord.ObservedStoreGeneration)
assert.Equal(t, int64(11), builderRecord.ObservedStackGeneration)
Expand All @@ -321,6 +352,10 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
BuildpackInfo: corev1alpha1.BuildpackInfo{Id: "io.buildpack.2", Version: "v2"},
Optional: true,
},
{
BuildpackInfo: corev1alpha1.BuildpackInfo{Id: "io.buildpack.4", Version: "v4"},
Optional: false,
},
},
},
})
Expand Down Expand Up @@ -449,14 +484,18 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
id = "io.buildpack.2"
version = "v2"
optional = true
[[order.group]]
id = "io.buildpack.4"
version = "v4"
`}})

})

buildpackOrder, err := imagehelpers.GetStringLabel(savedImage, buildpackOrderLabel)
assert.NoError(t, err)
assert.JSONEq(t, //language=json
`[{"group":[{"id":"io.buildpack.1","version":"v1"},{"id":"io.buildpack.2","version":"v2","optional":true}]}]`, buildpackOrder)
`[{"group":[{"id":"io.buildpack.1","version":"v1"},{"id":"io.buildpack.2","version":"v2","optional":true},{"id":"io.buildpack.4","version":"v4"}]}]`, buildpackOrder)

buildpackMetadata, err := imagehelpers.GetStringLabel(savedImage, buildpackMetadataLabel)
assert.NoError(t, err)
Expand Down Expand Up @@ -491,6 +530,11 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
"version": "v1.2.3 (git sha: abcdefg123456)"
},
"buildpacks": [
{
"id": "io.buildpack.4",
"version": "v4",
"homepage": "buildpack.4.com"
},
{
"id": "io.buildpack.3",
"version": "v3",
Expand Down Expand Up @@ -554,6 +598,20 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
}
]
}
},
"io.buildpack.4": {
"v4": {
"api": "0.3",
"layerDiffID": "sha256:3bf8899667b8d1e6b124f663faca32903b470831e5e4e992644ac5c839ab3462",
"stacks": [
{
"id": "io.buildpacks.stacks.some-stack"
},
{
"id": "io.some.other.stack"
}
]
}
}
}`, buildpackLayers)

Expand Down

0 comments on commit b4089a3

Please sign in to comment.