Add image registry cache

* Split up image cache configuration * Split up build cache configuration * Add registy/image based cache * Remove unused buildCacheName arg * Update documentation * Add basic registry-cache e2e test * Fix duplicate import * Fix unmatched quotes * Add home volume to restore step * Allow build.cache to be nil * Persist cache image in build * Use cache image digest if present * Move changes to buildapi from v1alpha1 to v1alpha2 * Add test cache for Spec.Cache = nil * Add validation that only one type of cache is used for build and image Co-authored-by: Ralf Pannemans <ralf.pannemans@sap.com> Co-authored-by: Johannes Dillmann <j.dillmann@sap.com> Co-authored-by: Sumit Kulhadia <sumit.kulhadia@sap.com> Co-authored-by: Pavel Busko <pavel.busko@sap.com>
buildpacks-community · Aug 24, 2021 · 5b57689 · 5b57689
1 parent 0c846f7
commit 5b57689
Show file tree

Hide file tree

Showing 23 changed files with 728 additions and 176 deletions.
diff --git a/docs/build.md b/docs/build.md
@@ -17,9 +17,11 @@ spec:
   serviceAccount: service-account
   builder:
     image: gcr.io/paketo-buildpacks/builder:base
-    imagePullSecrets: 
-    - name: builder-secret  
-  cacheName: persisent-volume-claim-name
+    imagePullSecrets:
+    - name: builder-secret
+  cache:
+    volume:
+      persistentVolumeClaimName: persisent-volume-claim-name
   projectDescriptorPath: path/to/project.toml
   source:
     git:
@@ -42,7 +44,9 @@ spec:
 - `builder.image`: This is the tag to the [Cloud Native Buildpacks builder image](https://buildpacks.io/docs/using-pack/working-with-builders/) to use in the build. Unlike on the Image resource, this is an image not a reference to a Builder resource.    
 - `builder.imagePullSecrets`: An optional list of pull secrets if the builder is in a private registry. [To create this secret please reference this link](https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/#registry-secret-existing-credentials)
 - `source`: The source location that will be the input to the build. See the [Source Configuration](#source-config) section below.
-- `cacheName`: Optional name of a persistent volume claim to used for a build cache across builds.
+- `cache`: Caching configuration, two variants are available:
+  - `volume.persistentVolumeClaimName`: Optional name of a persistent volume claim used for a build cache across builds.
+  - `registry.imageTag`: Optional name of a tag used for a build cache across builds.
 - `env`: Optional list of build time environment variables.
 - `projectDescriptorPath`: Path to the [project descriptor file](https://buildpacks.io/docs/reference/config/project-descriptor/) relative to source root dir or `subPath` if set. If unset, kpack will look for `project.toml` at the root dir or `subPath` if set.
 - `resources`: Optional configurable resource limits on `CPU` and `memory`.

diff --git a/docs/image.md b/docs/image.md
@@ -9,7 +9,9 @@ The following defines the relevant fields of the `image` resource spec in more d
 - `builder`: Configuration of the `builder` resource the image builds will use. See more info [Builder Configuration](builders.md).
 - `serviceAccount`: The Service Account name that will be used for credential lookup.
 - `source`: The source code that will be monitored/built into images. See the [Source Configuration](#source-config) section below.
-- `cacheSize`: The size of the Volume Claim that will be used by the build cache.
+- `cache`: Caching configuration, two variants are available:
+  - `volume.size`: Creates a Volume Claim of the given size
+  - `registry.imageTag`: Creates an image with cached contents
 - `failedBuildHistoryLimit`: The maximum number of failed builds for an image that will be retained.
 - `successBuildHistoryLimit`: The maximum number of successful builds for an image that will be retained.
 - `imageTaggingStrategy`: Allow for builds to be additionally tagged with the build number. Valid options are `None` and `BuildNumber`.
@@ -153,7 +155,9 @@ spec:
   builder:
     name: sample-builder
     kind: ClusterBuilder
-  cacheSize: "1.5Gi" # Optional, if not set then the caching feature is disabled
+  cache:
+    volume:
+      size: "1.5Gi" # Optional, if not set then the caching feature is disabled
   failedBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   successBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   source:
@@ -196,7 +200,9 @@ spec:
   builder:
     name: sample-builder
     kind: ClusterBuilder
-  cacheSize: "1.5Gi" # Optional, if not set then the caching feature is disabled
+  cache:
+    volume:
+      size: "1.5Gi" # Optional, if not set then the caching feature is disabled
   failedBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   successBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   source:

diff --git a/pkg/apis/build/v1alpha2/build.go b/pkg/apis/build/v1alpha2/build.go
@@ -3,7 +3,6 @@ package v1alpha2
 import (
 	"strconv"
 
-	"github.com/google/go-containerregistry/pkg/name"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"knative.dev/pkg/kmeta"
@@ -96,6 +95,17 @@ func (b *Build) BuiltImage() string {
 	return b.Status.LatestImage
 }
 
+func (b *Build) CacheImage() string {
+	if b == nil {
+		return ""
+	}
+	if !b.IsSuccess() {
+		return ""
+	}
+
+	return b.Status.LatestCacheImage
+}
+
 func (b *Build) IsSuccess() bool {
 	if b == nil {
 		return false
@@ -134,36 +144,3 @@ func (b *Build) rebasable(builderStack string) bool {
 	return b.Spec.LastBuild != nil &&
 		b.Annotations[BuildReasonAnnotation] == BuildReasonStack && b.Spec.LastBuild.StackId == builderStack
 }
-
-func (b *Build) builtWithStack(runImage string) bool {
-	if b.Status.Stack.RunImage == "" {
-		return false
-	}
-
-	lastBuildRunImageRef, err := name.ParseReference(b.Status.Stack.RunImage)
-	if err != nil {
-		return false
-	}
-
-	builderRunImageRef, err := name.ParseReference(runImage)
-	if err != nil {
-		return false
-	}
-
-	return lastBuildRunImageRef.Identifier() == builderRunImageRef.Identifier()
-}
-
-func (b *Build) builtWithBuildpacks(buildpacks BuildpackMetadataList) bool {
-	for _, bp := range b.Status.BuildMetadata {
-		if !buildpacks.Include(bp) {
-			return false
-		}
-	}
-
-	return true
-}
-
-func (b *Build) additionalBuildNeeded() bool {
-	_, ok := b.Annotations[BuildNeededAnnotation]
-	return ok
-}
diff --git a/pkg/apis/build/v1alpha2/build_pod.go b/pkg/apis/build/v1alpha2/build_pod.go
@@ -9,7 +9,6 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/kmeta"
 )
 
@@ -163,18 +162,26 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, taints
 		SubPath:   b.Spec.Source.SubPath, // empty string is a nop
 	}
 
-	var cacheArgs []string
+	var genericCacheArgs []string
+	var exporterCacheArgs []string
 	var cacheVolumes []corev1.VolumeMount
-	if b.Spec.CacheName == "" || config.OS == "windows" {
-		cacheArgs = nil
-		cacheVolumes = nil
+
+	if (!b.Spec.NeedVolumeCache() && !b.Spec.NeedRegistryCache()) || config.OS == "windows" {
+		genericCacheArgs = nil
+	} else if b.Spec.NeedRegistryCache() {
+		useCacheFromLastBuild := (b.Spec.LastBuild != nil && b.Spec.LastBuild.Cache.Image != "")
+		if useCacheFromLastBuild {
+			genericCacheArgs = []string{fmt.Sprintf("-cache-image=%s", b.Spec.LastBuild.Cache.Image)}
+		}
+		exporterCacheArgs = []string{fmt.Sprintf("-cache-image=%s", b.Spec.Cache.Registry.ImageTag)}
 	} else {
-		cacheArgs = []string{"-cache-dir=/cache"}
+		genericCacheArgs = []string{"-cache-dir=/cache"}
 		cacheVolumes = []corev1.VolumeMount{cacheVolume}
+		exporterCacheArgs = genericCacheArgs
 	}
 
 	return &corev1.Pod{
-		ObjectMeta: v1.ObjectMeta{
+		ObjectMeta: metav1.ObjectMeta{
 			Name:      b.PodName(),
 			Namespace: b.Namespace,
 			Labels: combine(b.Labels, map[string]string{
@@ -305,7 +312,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, taints
 							"-layers=/layers",
 							"-group=/layers/group.toml",
 							"-analyzed=/layers/analyzed.toml"},
-							cacheArgs,
+							genericCacheArgs,
 							func() []string {
 								if b.Spec.LastBuild != nil && b.Spec.LastBuild.Image != "" {
 									return []string{b.Spec.LastBuild.Image}
@@ -341,9 +348,10 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, taints
 						Args: args([]string{
 							"-group=/layers/group.toml",
 							"-layers=/layers",
-						}, cacheArgs),
+						}, genericCacheArgs),
 						VolumeMounts: append([]corev1.VolumeMount{
 							layersVolume,
+							homeVolume,
 						}, cacheVolumes...),
 						Env: []corev1.EnvVar{
 							{
@@ -392,7 +400,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, taints
 							"-group=/layers/group.toml",
 							"-analyzed=/layers/analyzed.toml",
 							"-project-metadata=/layers/project-metadata.toml"},
-							cacheArgs,
+							exporterCacheArgs,
 							func() []string {
 								if platformAPI == "0.3" {
 									return nil
@@ -645,14 +653,14 @@ func (b *Build) rebasePod(secrets []corev1.Secret, images BuildPodImages, config
 }
 
 func (b *Build) cacheVolume(os string) []corev1.Volume {
-	if b.Spec.CacheName == "" || os == "windows" {
+	if !b.Spec.NeedVolumeCache() || os == "windows" {
 		return []corev1.Volume{}
 	}
 
 	return []corev1.Volume{{
 		Name: cacheDirName,
 		VolumeSource: corev1.VolumeSource{
-			PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: b.Spec.CacheName},
+			PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: b.Spec.Cache.Volume.ClaimName},
 		},
 	}}
 }

diff --git a/pkg/apis/build/v1alpha2/build_pod_test.go b/pkg/apis/build/v1alpha2/build_pod_test.go
@@ -43,8 +43,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
 		Image: builderImage,
 		ImagePullSecrets: []corev1.LocalObjectReference{
 			{Name: "some-image-secret"},
-		},
-	}
+		}}
 
 	build := &buildapi.Build{
 		ObjectMeta: metav1.ObjectMeta{
@@ -67,7 +66,11 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
 					Revision: "gitrev1234",
 				},
 			},
-			CacheName: "some-cache-name",
+			Cache: &buildapi.BuildCacheConfig{
+				Volume: &buildapi.BuildPersistentVolumeCache{
+					ClaimName: "some-cache-name",
+				},
+			},
 			Bindings: []buildapi.Binding{
 				{
 					Name: "database",
@@ -581,6 +584,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
 			assert.Equal(t, pod.Spec.InitContainers[3].Image, builderImage)
 			assert.Equal(t, []string{
 				"layers-dir",
+				"home-dir",
 				"cache-dir",
 			}, names(pod.Spec.InitContainers[3].VolumeMounts))
 
@@ -668,9 +672,125 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
 			}, pod.Spec.Volumes[0])
 		})
 
+		when("registry cache is requested (first build)", func() {
+			podWithVolumeCache, _ := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+			build.Spec.Cache.Volume = nil
+			build.Spec.Cache.Registry = &buildapi.RegistryCache{ImageTag: "test-cache-image"}
+
+			it("creates a pod without cache volume", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				assert.Len(t, podWithImageCache.Spec.Volumes, len(podWithVolumeCache.Spec.Volumes)-1)
+			})
+
+			it("does not add the cache to analyze container", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				analyzeContainer := podWithImageCache.Spec.InitContainers[2]
+				assert.NotContains(t, analyzeContainer.Args, "-cache-image=test-cache-image")
+			})
+			it("does not add the cache to restore container", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				restoreContainer := podWithImageCache.Spec.InitContainers[3]
+				assert.NotContains(t, restoreContainer.Args, "-cache-image=test-cache-image")
+			})
+			it("adds the cache to export container", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				exportContainer := podWithImageCache.Spec.InitContainers[5]
+				assert.Contains(t, exportContainer.Args, "-cache-image=test-cache-image")
+			})
+		})
+
+		when("registry cache is requested (second build)", func() {
+			podWithVolumeCache, _ := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+			build.Spec.Cache.Volume = nil
+			build.Spec.Cache.Registry = &buildapi.RegistryCache{ImageTag: "test-cache-image"}
+			build.Spec.LastBuild = &buildapi.LastBuild{
+				Cache: buildapi.BuildCache{
+					Image: "test-cache-image@sha",
+				},
+			}
+
+			it("creates a pod without cache volume", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				assert.Len(t, podWithImageCache.Spec.Volumes, len(podWithVolumeCache.Spec.Volumes)-1)
+			})
+
+			it("adds the cache to analyze container", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				analyzeContainer := podWithImageCache.Spec.InitContainers[2]
+				assert.Contains(t, analyzeContainer.Args, "-cache-image=test-cache-image@sha")
+			})
+			it("adds the cache to restore container", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				restoreContainer := podWithImageCache.Spec.InitContainers[3]
+				assert.Contains(t, restoreContainer.Args, "-cache-image=test-cache-image@sha")
+			})
+			it("adds the cache to export container", func() {
+				podWithImageCache, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				exportContainer := podWithImageCache.Spec.InitContainers[5]
+				assert.Contains(t, exportContainer.Args, "-cache-image=test-cache-image")
+			})
+		})
+
+		when("ImageTag is empty", func() {
+			var pod *corev1.Pod
+			var err error
+			build.Spec.Cache.Registry = &buildapi.RegistryCache{ImageTag: ""}
+
+			it("does not add the cache to analyze container", func() {
+				pod, err = build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				analyzeContainer := pod.Spec.InitContainers[2]
+				assert.NotContains(t, analyzeContainer.Args, "-cache-image")
+			})
+			it("does not add the cache to restore container", func() {
+				pod, err = build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				restoreContainer := pod.Spec.InitContainers[3]
+				assert.NotContains(t, restoreContainer.Args, "-cache-image")
+			})
+			it("does not add the cache to export container", func() {
+				pod, err = build.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				exportContainer := pod.Spec.InitContainers[5]
+				assert.NotContains(t, exportContainer.Args, "-cache-image")
+			})
+		})
+
+		when("Cache is nil", func() {
+			buildCopy := build.DeepCopy()
+			podWithCache, _ := buildCopy.BuildPod(config, nil, nil, buildPodBuilderConfig)
+			buildCopy.Spec.Cache = nil
+
+			it("creates a pod without cache volume", func() {
+				pod, err := buildCopy.BuildPod(config, nil, nil, buildPodBuilderConfig)
+				require.NoError(t, err)
+
+				assert.Len(t, pod.Spec.Volumes, len(podWithCache.Spec.Volumes)-1)
+			})
+		})
+
 		when("CacheName is empty", func() {
 			podWithCache, _ := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
-			build.Spec.CacheName = ""
+			build.Spec.Cache.Volume = &buildapi.BuildPersistentVolumeCache{ClaimName: ""}
 
 			it("creates a pod without cache volume", func() {
 				pod, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
@@ -1285,7 +1405,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
 				buildPodBuilderConfigLinux := buildPodBuilderConfig.DeepCopy()
 				buildPodBuilderConfigLinux.OS = "linux"
 				podWithCache, _ := build.BuildPod(config, nil, nil, *buildPodBuilderConfigLinux)
-				build.Spec.CacheName = "non-empty"
+				build.Spec.Cache.Volume.ClaimName = "non-empty"
 
 				pod, err := build.BuildPod(config, nil, nil, buildPodBuilderConfig)
 				require.NoError(t, err)