Merge pull request #1029 from pivotal/storageclass

Add configurable storage class name for image resource

docs/image.md

@@ -14,6 +14,7 @@ The following defines the relevant fields of the `image` resource spec in more d
 - `source`: The source code that will be monitored/built into images. See the [Source Configuration](#source-config) section below.
 - `cache`: Caching configuration, two variants are available:
   - `volume.size`: Creates a Volume Claim of the given size
+  - `volume.storageClassName`: (Optional) Creates a Volume Claim of the given storageClassName. If unset, the default storage class is used. The field is immutable.
   - `registry.tag`: 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.
@@ -254,6 +255,7 @@ spec:
   cache:
     volume:
       size: "1.5Gi" # Optional, if not set then the caching feature is disabled
+      storageClassName: "my-storage-class" # Optional, if not set then the default storageclass is used
   failedBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   successBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   source:
@@ -299,6 +301,7 @@ spec:
   cache:
     volume:
       size: "1.5Gi" # Optional, if not set then the caching feature is disabled
+      storageClassName: "my-storage-class" # Optional, if not set then the default storageclass is used
   failedBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   successBuildHistoryLimit: 5 # Optional, if not present defaults to 10
   source:

pkg/apis/build/v1alpha2/image_builds.go

@@ -205,6 +205,10 @@ func (im *Image) CacheName() string {
 }
 
 func (im *Image) BuildCache() *corev1.PersistentVolumeClaim {
+	var storageClassName *string
+	if im.Spec.Cache.Volume.StorageClassName != "" {
+		storageClassName = &im.Spec.Cache.Volume.StorageClassName
+	}
 	return &corev1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      im.CacheName(),
@@ -221,6 +225,7 @@ func (im *Image) BuildCache() *corev1.PersistentVolumeClaim {
 					corev1.ResourceStorage: *im.Spec.Cache.Volume.Size,
 				},
 			},
+			StorageClassName: storageClassName,
 		},
 	}
 }

pkg/apis/build/v1alpha2/image_conversion.go

@@ -10,10 +10,18 @@ import (
 	"github.com/pivotal/kpack/pkg/apis/build/v1alpha1"
 )
 
+const (
+	servicesConversionAnnotation         = "kpack.io/services"
+	storageClassNameConversionAnnotation = "kpack.io/cache.volume.storageClassName"
+)
+
 func (i *Image) ConvertTo(_ context.Context, to apis.Convertible) error {
 	switch toImage := to.(type) {
 	case *v1alpha1.Image:
 		i.ObjectMeta.DeepCopyInto(&toImage.ObjectMeta)
+		if toImage.Annotations == nil {
+			toImage.Annotations = map[string]string{}
+		}
 		i.Spec.convertTo(&toImage.Spec)
 		i.Status.convertTo(&toImage.Status)
 		if build := i.Spec.Build; build != nil {
@@ -22,12 +30,12 @@ func (i *Image) ConvertTo(_ context.Context, to apis.Convertible) error {
 				if err != nil {
 					return err
 				}
-				if toImage.Annotations == nil {
-					toImage.Annotations = map[string]string{}
-				}
-				toImage.Annotations["kpack.io/services"] = string(bytes)
+				toImage.Annotations[servicesConversionAnnotation] = string(bytes)
 			}
 		}
+		if i.Spec.Cache != nil && i.Spec.Cache.Volume != nil && i.Spec.Cache.Volume.StorageClassName != "" {
+			toImage.Annotations[storageClassNameConversionAnnotation] = i.Spec.Cache.Volume.StorageClassName
+		}
 	default:
 		return fmt.Errorf("unknown version, got: %T", toImage)
 	}
@@ -40,15 +48,25 @@ func (i *Image) ConvertFrom(_ context.Context, from apis.Convertible) error {
 		fromImage.ObjectMeta.DeepCopyInto(&i.ObjectMeta)
 		i.Spec.convertFrom(&fromImage.Spec)
 		i.Status.convertFrom(&fromImage.Status)
-		if servicesJson, ok := i.Annotations["kpack.io/services"]; ok {
+		if servicesJson, ok := i.Annotations[servicesConversionAnnotation]; ok {
 			var services Services
 			err := json.Unmarshal([]byte(servicesJson), &services)
 			if err != nil {
 				return err
 			}
 
 			i.Spec.Build.Services = services
-			delete(i.Annotations, "kpack.io/services")
+			delete(i.Annotations, servicesConversionAnnotation)
+		}
+		if storageClassName, ok := i.Annotations[storageClassNameConversionAnnotation]; ok {
+			if i.Spec.Cache == nil {
+				i.Spec.Cache = &ImageCacheConfig{}
+			}
+			if i.Spec.Cache.Volume == nil {
+				i.Spec.Cache.Volume = &ImagePersistentVolumeCache{}
+			}
+			i.Spec.Cache.Volume.StorageClassName = storageClassName
+			delete(i.Annotations, storageClassNameConversionAnnotation)
 		}
 	default:
 		return fmt.Errorf("unknown version, got: %T", fromImage)

pkg/apis/build/v1alpha2/image_conversion_test.go

@@ -41,7 +41,8 @@ func testImageConversion(t *testing.T, when spec.G, it spec.S) {
 				},
 				Cache: &ImageCacheConfig{
 					Volume: &ImagePersistentVolumeCache{
-						Size: &cacheSize,
+						Size:             &cacheSize,
+						StorageClassName: "some-storage-class",
 					},
 				},
 				FailedBuildHistoryLimit:  &buildHistoryLimit,
@@ -100,7 +101,8 @@ func testImageConversion(t *testing.T, when spec.G, it spec.S) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "my-super-convertable-image",
 				Annotations: map[string]string{
-					"kpack.io/services": `[{"kind":"Secret","name":"some-secret","apiVersion":"v1"}]`,
+					"kpack.io/services":                      `[{"kind":"Secret","name":"some-secret","apiVersion":"v1"}]`,
+					"kpack.io/cache.volume.storageClassName": "some-storage-class",
 				},
 			},
 			Spec: v1alpha1.ImageSpec{

pkg/apis/build/v1alpha2/image_types.go

@@ -88,7 +88,8 @@ type ImageCacheConfig struct {
 
 // +k8s:openapi-gen=true
 type ImagePersistentVolumeCache struct {
-	Size *resource.Quantity `json:"size,omitempty"`
+	Size             *resource.Quantity `json:"size,omitempty"`
+	StorageClassName string             `json:"storageClassName,omitempty"`
 }
 
 // +k8s:openapi-gen=true

pkg/apis/build/v1alpha2/image_validation.go

@@ -127,8 +127,8 @@ func (is *ImageSpec) validateSameRegistry() *apis.FieldError {
 }
 
 func (is *ImageSpec) validateVolumeCache(ctx context.Context) *apis.FieldError {
-	if is.Cache != nil && is.Cache.Volume != nil && ctx.Value(HasDefaultStorageClass) == nil {
-		return apis.ErrGeneric("spec.cache.volume.size cannot be set with no default StorageClass")
+	if is.Cache != nil && is.Cache.Volume != nil && is.Cache.Volume.StorageClassName == "" && ctx.Value(HasDefaultStorageClass) == nil {
+		return apis.ErrGeneric("spec.cache.volume.size cannot be set without spec.cache.volume.storageClassName or a default StorageClass")
 	}
 
 	if apis.IsInUpdate(ctx) {
@@ -147,6 +147,8 @@ func (is *ImageSpec) validateVolumeCache(ctx context.Context) *apis.FieldError {
 					Details: fmt.Sprintf("current: %v, requested: %v", original.Spec.Cache.Volume.Size, is.Cache.Volume.Size),
 				}
 			}
+
+			return validate.ImmutableField(original.Spec.Cache.Volume.StorageClassName, is.Cache.Volume.StorageClassName, "cache.volume.storageClassName")
 		}
 	}
 
@@ -194,7 +196,7 @@ func (is *ImageSpec) validateBuildHistoryLimit() *apis.FieldError {
 	return nil
 }
 
-func (c *ImageCacheConfig) Validate(context context.Context) *apis.FieldError {
+func (c *ImageCacheConfig) Validate(ctx context.Context) *apis.FieldError {
 	if c != nil && c.Volume != nil && c.Registry != nil {
 		return apis.ErrGeneric("only one type of cache can be specified", "volume", "registry")
 	}

pkg/apis/build/v1alpha2/image_validation_test.go

@@ -44,7 +44,8 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) {
 			},
 			Cache: &ImageCacheConfig{
 				Volume: &ImagePersistentVolumeCache{
-					Size: &cacheSize,
+					Size:             &cacheSize,
+					StorageClassName: "sc-name",
 				},
 			},
 			FailedBuildHistoryLimit:  &limit,
@@ -285,10 +286,11 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) {
 
 		})
 
-		it("validates cache size is not set when there is no default StorageClass", func() {
+		it("validates cache size is not set when there is no default StorageClass and no storageClassName", func() {
 			ctx = context.TODO()
+			image.Spec.Cache.Volume.StorageClassName = ""
 
-			assertValidationError(image, ctx, apis.ErrGeneric("spec.cache.volume.size cannot be set with no default StorageClass"))
+			assertValidationError(image, ctx, apis.ErrGeneric("spec.cache.volume.size cannot be set without spec.cache.volume.storageClassName or a default StorageClass"))
 		})
 
 		it("combining errors", func() {
@@ -315,6 +317,14 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) {
 			assert.EqualError(t, err, "Field cannot be decreased: spec.cache.volume.size\ncurrent: 5G, requested: 4G")
 		})
 
+		it("image.cache.volume.storageClassName has not changed", func() {
+			original := image.DeepCopy()
+
+			image.Spec.Cache.Volume.StorageClassName = "sc-different"
+			err := image.Validate(apis.WithinUpdate(ctx, original))
+			assert.EqualError(t, err, "Immutable field changed: spec.cache.volume.storageClassName\ngot: sc-different, want: sc-name")
+		})
+
 		when("validating the cosign config", func() {
 			// cosign: nil
 			it("handles nil cosign", func() {

pkg/cnb/env_vars.go

@@ -1,10 +1,11 @@
 package cnb
 
 import (
-	"github.com/pkg/errors"
 	"io/ioutil"
 	"os"
 	"path"
+
+	"github.com/pkg/errors"
 )
 
 func serializeEnvVars(envVars []envVariable, platformDir string) error {

pkg/reconciler/image/image_test.go

@@ -408,7 +408,6 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 			}
 
 			it("creates a cache if requested", func() {
-
 				rt.Test(rtesting.TableRow{
 					Key: key,
 					Objects: []runtime.Object{
@@ -460,20 +459,41 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 			it("does not create a cache if a cache already exists", func() {
 				image.Spec.Cache.Volume.Size = &cacheSize
 				image.Status.BuildCacheName = image.CacheName()
+				storageClassName := "some-name"
 
 				rt.Test(rtesting.TableRow{
 					Key: key,
 					Objects: []runtime.Object{
 						image,
 						image.SourceResolver(),
-						image.BuildCache(),
+						&corev1.PersistentVolumeClaim{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      image.CacheName(),
+								Namespace: namespace,
+								OwnerReferences: []metav1.OwnerReference{
+									*kmeta.NewControllerRef(image),
+								},
+								Labels: map[string]string{
+									someLabelKey: someValueToPassThrough,
+								},
+							},
+							Spec: corev1.PersistentVolumeClaimSpec{
+								AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
+								Resources: corev1.ResourceRequirements{
+									Requests: corev1.ResourceList{
+										corev1.ResourceStorage: cacheSize,
+									},
+								},
+								StorageClassName: &storageClassName,
+							},
+						},
 						builder,
 					},
 					WantErr: false,
 				})
 			})
 
-			it("updates build cache if desired configuration changed", func() {
+			it("updates build cache if size changes", func() {
 				var imageCacheName = image.CacheName()
 
 				image.Status.BuildCacheName = imageCacheName
@@ -622,6 +642,57 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 					},
 				})
 			})
+
+			it("uses the storageClassName if provided", func() {
+				image.Spec.Cache.Volume.StorageClassName = "some-storage-class"
+				rt.Test(rtesting.TableRow{
+					Key: key,
+					Objects: []runtime.Object{
+						image,
+						image.SourceResolver(),
+						builder,
+					},
+					WantErr: false,
+					WantCreates: []runtime.Object{
+						&corev1.PersistentVolumeClaim{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      image.CacheName(),
+								Namespace: namespace,
+								OwnerReferences: []metav1.OwnerReference{
+									*kmeta.NewControllerRef(image),
+								},
+								Labels: map[string]string{
+									someLabelKey: someValueToPassThrough,
+								},
+							},
+							Spec: corev1.PersistentVolumeClaimSpec{
+								AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
+								Resources: corev1.ResourceRequirements{
+									Requests: corev1.ResourceList{
+										corev1.ResourceStorage: cacheSize,
+									},
+								},
+								StorageClassName: &image.Spec.Cache.Volume.StorageClassName,
+							},
+						},
+					},
+					WantStatusUpdates: []clientgotesting.UpdateActionImpl{
+						{
+							Object: &buildapi.Image{
+								ObjectMeta: image.ObjectMeta,
+								Spec:       image.Spec,
+								Status: buildapi.ImageStatus{
+									BuildCacheName: image.CacheName(),
+									Status: corev1alpha1.Status{
+										ObservedGeneration: originalGeneration,
+										Conditions:         conditionReadyUnknown(),
+									},
+								},
+							},
+						},
+					},
+				})
+			})
 		})
 
 		when("reconciling builds", func() {