Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.8.x] Deduplicate builders by diff id #1285

Merged
merged 5 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 25 additions & 21 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
go-version-file: 'go.mod'

- name: Run tests
run: make unit-ci
uses: ./.github/actions/run-tests

- name: Report coverage
uses: codecov/codecov-action@v3.1.4
Expand All @@ -54,6 +54,8 @@ jobs:
pack_version: ${{ env.PACK_VERSION }}
tag: ${{ env.PUBLIC_IMAGE_DEV_REPO }}/controller
bp_go_targets: "./cmd/controller"
builder: gcr.io/cf-build-service-public/ci/kpack-builder
additional_pack_args: '--env BP_GIT2GO_ENABLED="true" --env BP_GIT2GO_USE_LIBSSL="true"'

webhook-image:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -108,6 +110,8 @@ jobs:
pack_version: ${{ env.PACK_VERSION }}
tag: ${{ env.PUBLIC_IMAGE_DEV_REPO }}/build-init
bp_go_targets: "./cmd/build-init"
builder: gcr.io/cf-build-service-public/ci/kpack-builder
additional_pack_args: '--env BP_GIT2GO_ENABLED="true" --env BP_GIT2GO_USE_LIBSSL="true"'

build-waiter-image:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -163,7 +167,7 @@ jobs:
tag: ${{ env.PUBLIC_IMAGE_DEV_REPO }}/build-init-windows
bp_go_targets: '.\cmd\build-init;.\cmd\network-wait-launcher'
builder: gcr.io/cf-build-service-public/kpack-windows-builder
additional_pack_args: "--trust-builder"
additional_pack_args: '--trust-builder --env BP_MSYS2_COPY_FILES="c:\layers\samples_msys2\msys2\msys64\mingw64\bin\libgit2.dll;c:\layers\samples_msys2\msys2\msys64\mingw64\bin\zlib1.dll;c:\layers\samples_msys2\msys2\msys64\mingw64\bin\libssl-1_1-x64.dll;c:\layers\samples_msys2\msys2\msys64\mingw64\bin\libssh2-1.dll;c:\layers\samples_msys2\msys2\msys64\mingw64\bin\libcrypto-1_1-x64.dll;c:\layers\samples_msys2\msys2\msys64\mingw64\bin\libhttp_parser-2.dll"'

completion-windows-image:
runs-on: windows-2019
Expand Down Expand Up @@ -241,15 +245,15 @@ jobs:
- name: Build release yaml
run: |
ytt -f config/ \
-v controller.image=$(cat controller) \
-v webhook.image=$(cat webhook) \
-v build_init.image=$(cat build-init) \
-v build_init_windows.image=$(cat build-init-windows) \
-v build_waiter.image=$(cat build-waiter) \
-v rebase.image=$(cat rebase) \
-v completion.image=$(cat completion) \
-v completion_windows.image=$(cat completion-windows) \
-v lifecycle.image=$(cat lifecycle) > prerelease.yaml
-v controller_image=$(cat controller) \
-v webhook_image=$(cat webhook) \
-v build_init_image=$(cat build-init) \
-v build_init_windows_image=$(cat build-init-windows) \
-v build_waiter_image=$(cat build-waiter) \
-v rebase_image=$(cat rebase) \
-v completion_image=$(cat completion) \
-v completion_windows_image=$(cat completion-windows) \
-v lifecycle_image=$(cat lifecycle) > prerelease.yaml

cat prerelease.yaml

Expand Down Expand Up @@ -483,16 +487,16 @@ jobs:
run: |
file="release-${{ env.KPACK_VERSION }}.yaml"
ytt -f config/ \
-v controller.image=$(cat final-image-refs/controller) \
-v webhook.image=$(cat final-image-refs/webhook) \
-v build_init.image=$(cat final-image-refs/build-init) \
-v build_init_windows.image=$(cat final-image-refs/build-init-windows) \
-v build_waiter.image=$(cat final-image-refs/build-waiter) \
-v rebase.image=$(cat final-image-refs/rebase) \
-v completion.image=$(cat final-image-refs/completion) \
-v completion_windows.image=$(cat final-image-refs/completion-windows) \
-v lifecycle.image=$(cat final-image-refs/lifecycle) \
-v kpack_version=${{ env.KPACK_VERSION }} > $file
-v controller_image=$(cat final-image-refs/controller) \
-v webhook_image=$(cat final-image-refs/webhook) \
-v build_init_image=$(cat final-image-refs/build-init) \
-v build_init_windows_image=$(cat final-image-refs/build-init-windows) \
-v build_waiter_image=$(cat final-image-refs/build-waiter) \
-v rebase_image=$(cat final-image-refs/rebase) \
-v completion_image=$(cat final-image-refs/completion) \
-v completion_windows_image=$(cat final-image-refs/completion-windows) \
-v lifecycle_image=$(cat final-image-refs/lifecycle) \
-v version=${{ env.KPACK_VERSION }} > $file
echo "sha=$(shasum -a 256 $file)" >> $GITHUB_OUTPUT

- name: Upload Release
Expand Down
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@ unit:
unit-ci:
$(GOCMD) test -v -count=1 -parallel=1 -timeout=0 ./pkg/... -coverprofile=coverage.txt -covermode=atomic

e2e:
$(GOCMD) test --timeout=30m -v ./test/...

.PHONY: unit
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