[0.7.x] Deduplicate builders by diff id (#1286)

* Enable building images that require libgit2 support - This has been removed in the latest versions of kpack but is required for older versions - This requires using a builder with the exisitng git2go buildpack that has not been open sourced at the moment Signed-off-by: Tom Kennedy <[email protected]> * Fix workflow for windows Signed-off-by: Tom Kennedy <[email protected]> * Update schema to match pre 0.11 changes - On main the config directory schema has changed. When backporting the ci workflow that was not taken into consideration so it is using the wrong schema. Signed-off-by: Tom Kennedy <[email protected]> * Add e2e to Makefile Signed-off-by: Tom Kennedy <[email protected]> * Deduplicate builder layers by diff id - 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]> --------- Signed-off-by: Tom Kennedy <[email protected]>
buildpacks-community · Jul 25, 2023 · b860d33 · b860d33
1 parent e7fc01e
commit b860d33
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 23 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -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/[email protected]
@@ -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
@@ -108,6 +110,26 @@ 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
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v3
+    - name: Docker Login
+      uses: docker/[email protected]
+      with:
+        registry: ${{ secrets.REGISTRY_HOST }}
+        username: ${{ secrets.REGISTRY_USER }}
+        password: ${{ secrets.REGISTRY_PASSWORD }}
+    - name: Build
+      uses: ./.github/actions/pack-build
+      with:
+        pack_version: ${{ env.PACK_VERSION }}
+        tag: ${{ env.PUBLIC_IMAGE_DEV_REPO }}/build-waiter
+        bp_go_targets: "./cmd/build-waiter"
 
   rebase-image:
     runs-on: ubuntu-latest
@@ -145,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
@@ -222,14 +244,14 @@ 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 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 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
 
@@ -426,15 +448,15 @@ 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 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 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

diff --git a/Makefile b/Makefile
@@ -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
diff --git a/pkg/cnb/builder_builder.go b/pkg/cnb/builder_builder.go
@@ -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,
@@ -136,7 +136,7 @@ func (bb *builderBlder) WriteableImage() (v1.Image, error) {
 				stackLayer,
 				orderLayer,
 			},
-		)...)
+		))...)
 	if err != nil {
 		return nil, err
 	}
@@ -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
+}
diff --git a/pkg/cnb/create_builder_test.go b/pkg/cnb/create_builder_test.go
@@ -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",
+							},
+						},
 					},
 				},
 			},
@@ -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() {
@@ -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)
@@ -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,
+						},
 					},
 				},
 			})
@@ -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)
@@ -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",
@@ -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)