diff --git a/docs/image.md b/docs/image.md index f25ce570b..cec6aa5b1 100644 --- a/docs/image.md +++ b/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: diff --git a/pkg/apis/build/v1alpha2/image_builds.go b/pkg/apis/build/v1alpha2/image_builds.go index 73756486e..ce203a6fc 100644 --- a/pkg/apis/build/v1alpha2/image_builds.go +++ b/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, }, } } diff --git a/pkg/apis/build/v1alpha2/image_conversion.go b/pkg/apis/build/v1alpha2/image_conversion.go index 93dbeaa0d..bd770ebf0 100644 --- a/pkg/apis/build/v1alpha2/image_conversion.go +++ b/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,7 +48,7 @@ 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 { @@ -48,7 +56,17 @@ func (i *Image) ConvertFrom(_ context.Context, from apis.Convertible) error { } 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) diff --git a/pkg/apis/build/v1alpha2/image_conversion_test.go b/pkg/apis/build/v1alpha2/image_conversion_test.go index 11fc43931..98cc3b6db 100644 --- a/pkg/apis/build/v1alpha2/image_conversion_test.go +++ b/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{ diff --git a/pkg/apis/build/v1alpha2/image_types.go b/pkg/apis/build/v1alpha2/image_types.go index 81b202743..39556d1ff 100644 --- a/pkg/apis/build/v1alpha2/image_types.go +++ b/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 diff --git a/pkg/apis/build/v1alpha2/image_validation.go b/pkg/apis/build/v1alpha2/image_validation.go index 976a3c15a..4c4d94b3c 100644 --- a/pkg/apis/build/v1alpha2/image_validation.go +++ b/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") } diff --git a/pkg/apis/build/v1alpha2/image_validation_test.go b/pkg/apis/build/v1alpha2/image_validation_test.go index 1b450d045..073f865c9 100644 --- a/pkg/apis/build/v1alpha2/image_validation_test.go +++ b/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() { diff --git a/pkg/cnb/env_vars.go b/pkg/cnb/env_vars.go index c845a1fae..56fd56bc9 100644 --- a/pkg/cnb/env_vars.go +++ b/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 { diff --git a/pkg/reconciler/image/image_test.go b/pkg/reconciler/image/image_test.go index e6710eb66..7f32cc8a2 100644 --- a/pkg/reconciler/image/image_test.go +++ b/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() {