Skip to content

Commit

Permalink
Add configurable storage class name for image resource
Browse files Browse the repository at this point in the history
- If set, will be used as the storageClassName in the pvc
  • Loading branch information
Tyler Phelan committed Sep 6, 2022
1 parent 411ffa0 commit daedd68
Show file tree
Hide file tree
Showing 10 changed files with 208 additions and 7 deletions.
2 changes: 1 addition & 1 deletion cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,4 +305,4 @@ func parseMaxPlatformApiVersion() (*semver.Version, error) {
}

return nil, nil
}
}
3 changes: 3 additions & 0 deletions docs/image.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. The default storage class will be used if unset.
- `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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/build/v1alpha2/image_builds.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -221,6 +225,7 @@ func (im *Image) BuildCache() *corev1.PersistentVolumeClaim {
corev1.ResourceStorage: *im.Spec.Cache.Volume.Size,
},
},
StorageClassName: storageClassName,
},
}
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/apis/build/v1alpha2/image_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ const (
BuilderNotReady = "BuilderNotReady"
)

func (im *Image) NotReady(err error) corev1alpha1.Conditions {
return corev1alpha1.Conditions{
{
Type: corev1alpha1.ConditionReady,
Status: corev1.ConditionFalse,
Message: err.Error(),
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
},
}
}

func (im *Image) BuilderNotFound() corev1alpha1.Conditions {
return corev1alpha1.Conditions{
{
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/build/v1alpha2/image_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 18 additions & 1 deletion pkg/reconciler/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
Expand All @@ -13,8 +14,10 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection"
coreinformers "k8s.io/client-go/informers/core/v1"
storageinformers "k8s.io/client-go/informers/storage/v1"
k8sclient "k8s.io/client-go/kubernetes"
corelisters "k8s.io/client-go/listers/core/v1"
storagelisters "k8s.io/client-go/listers/storage/v1"
"k8s.io/client-go/tools/cache"
"knative.dev/pkg/controller"
"knative.dev/pkg/logging/logkey"
Expand Down Expand Up @@ -42,6 +45,7 @@ func NewController(
duckbuilderInformer *duckbuilder.DuckBuilderInformer,
sourceResolverInformer buildinformers.SourceResolverInformer,
pvcInformer coreinformers.PersistentVolumeClaimInformer,
storageClassListers storageinformers.StorageClassInformer,
enablePriorityClasses bool,
) *controller.Impl {
c := &Reconciler{
Expand All @@ -52,6 +56,7 @@ func NewController(
DuckBuilderLister: duckbuilderInformer.Lister(),
SourceResolverLister: sourceResolverInformer.Lister(),
PvcLister: pvcInformer.Lister(),
StorageClassLister: storageClassListers.Lister(),
EnablePriorityClasses: enablePriorityClasses,
}

Expand Down Expand Up @@ -92,6 +97,7 @@ type Reconciler struct {
BuildLister buildlisters.BuildLister
SourceResolverLister buildlisters.SourceResolverLister
PvcLister corelisters.PersistentVolumeClaimLister
StorageClassLister storagelisters.StorageClassLister
Tracker reconciler.Tracker
K8sClient k8sclient.Interface
EnablePriorityClasses bool
Expand Down Expand Up @@ -145,6 +151,15 @@ func (c *Reconciler) reconcileImage(ctx context.Context, image *buildapi.Image)
return image, nil
}

if image.Spec.NeedVolumeCache() && image.Spec.Cache.Volume.StorageClassName != "" {
_, err := c.StorageClassLister.Get(image.Spec.Cache.Volume.StorageClassName)
if err != nil {
image.Status.Conditions = image.NotReady(errors.Wrapf(err, "failed to validate storageclass '%v'", image.Spec.Cache.Volume.StorageClassName))
image.Status.ObservedGeneration = image.Generation
return image, nil
}
}

buildCacheName, err := c.reconcileBuildCache(ctx, image)
if err != nil {
return nil, err
Expand Down Expand Up @@ -218,6 +233,7 @@ func (c *Reconciler) reconcileBuildCache(ctx context.Context, image *buildapi.Im

existing := buildCache.DeepCopy()
existing.Spec.Resources = desiredBuildCache.Spec.Resources
existing.Spec.StorageClassName = desiredBuildCache.Spec.StorageClassName
existing.ObjectMeta.Labels = desiredBuildCache.ObjectMeta.Labels
_, err = c.K8sClient.CoreV1().PersistentVolumeClaims(image.Namespace).Update(ctx, existing, metav1.UpdateOptions{})
return existing.Name, errors.Wrap(err, "cannot update persistent volume claim")
Expand Down Expand Up @@ -295,5 +311,6 @@ func sourceResolversEqual(desiredSourceResolver *buildapi.SourceResolver, source

func buildCacheEqual(desiredBuildCache *corev1.PersistentVolumeClaim, buildCache *corev1.PersistentVolumeClaim) bool {
return equality.Semantic.DeepEqual(desiredBuildCache.Spec.Resources, buildCache.Spec.Resources) &&
equality.Semantic.DeepEqual(desiredBuildCache.Labels, buildCache.Labels)
equality.Semantic.DeepEqual(desiredBuildCache.Labels, buildCache.Labels) &&
cmp.Equal(desiredBuildCache.Spec.StorageClassName, buildCache.Spec.StorageClassName)
}
160 changes: 158 additions & 2 deletions pkg/reconciler/image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/sclevine/spec"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -67,6 +68,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
DuckBuilderLister: listers.GetDuckBuilderLister(),
SourceResolverLister: listers.GetSourceResolverLister(),
PvcLister: listers.GetPersistentVolumeClaimLister(),
StorageClassLister: listers.GetStorageClassLister(),
Tracker: fakeTracker,
K8sClient: k8sfakeClient,
}
Expand Down Expand Up @@ -408,7 +410,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{
Expand Down Expand Up @@ -473,7 +474,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
})
})

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
Expand Down Expand Up @@ -622,6 +623,161 @@ 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"
storageClass := &storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: image.Spec.Cache.Volume.StorageClassName,
},
}
rt.Test(rtesting.TableRow{
Key: key,
Objects: []runtime.Object{
image,
image.SourceResolver(),
builder,
storageClass,
},
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(),
},
},
},
},
},
})
})

it("updates build cache if the storageClassName changes", func() {
var imageCacheName = image.CacheName()

image.Status.BuildCacheName = imageCacheName
image.Spec.Cache.Volume.StorageClassName = "new-storage-class"

storageClass := &storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: image.Spec.Cache.Volume.StorageClassName,
},
}
rt.Test(rtesting.TableRow{
Key: key,
Objects: []runtime.Object{
image,
image.SourceResolver(),
builder,
&corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: imageCacheName,
Namespace: namespace,
OwnerReferences: []metav1.OwnerReference{
*kmeta.NewControllerRef(image),
},
},
Spec: corev1.PersistentVolumeClaimSpec{
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceStorage: cacheSize,
},
},
},
},
storageClass,
},
WantErr: false,
WantUpdates: []clientgotesting.UpdateActionImpl{
{
Object: &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: imageCacheName,
Namespace: namespace,
Labels: map[string]string{
someLabelKey: someValueToPassThrough,
},
OwnerReferences: []metav1.OwnerReference{
*kmeta.NewControllerRef(image),
},
},
Spec: corev1.PersistentVolumeClaimSpec{
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceStorage: *image.Spec.Cache.Volume.Size,
},
},
StorageClassName: &image.Spec.Cache.Volume.StorageClassName,
},
},
},
},
})
})

it("sets ready false if the storageClassName provided does not exist", func() {
image.Spec.Cache.Volume.StorageClassName = "does-not-exist-class"
rt.Test(rtesting.TableRow{
Key: key,
Objects: []runtime.Object{
image,
image.SourceResolver(),
builder,
},
WantErr: false,
WantStatusUpdates: []clientgotesting.UpdateActionImpl{
{
Object: &buildapi.Image{
ObjectMeta: image.ObjectMeta,
Spec: image.Spec,
Status: buildapi.ImageStatus{
Status: corev1alpha1.Status{
ObservedGeneration: originalGeneration,
Conditions: corev1alpha1.Conditions{
{
Type: corev1alpha1.ConditionReady,
Status: corev1.ConditionFalse,
Message: `failed to validate storageclass 'does-not-exist-class': storageclass.storage.k8s.io "does-not-exist-class" not found`,
},
},
},
},
},
},
},
})
})
})

when("reconciling builds", func() {
Expand Down
1 change: 1 addition & 0 deletions pkg/reconciler/network_error_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package reconciler

import (
"context"

"github.com/pkg/errors"
"knative.dev/pkg/controller"
)
Expand Down
5 changes: 3 additions & 2 deletions pkg/reconciler/network_error_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ package reconciler

import (
"context"
"testing"

"github.com/pkg/errors"
"github.com/sclevine/spec"
"github.com/stretchr/testify/require"
"testing"
)

const (
networkErrorMsg = "some network problem"
otherErrorMsg = "some other error"
otherErrorMsg = "some other error"
)

func TestNetworkErrorReconciler(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/reconciler/testhelpers/listers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package testhelpers

import (
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
"k8s.io/apimachinery/pkg/runtime"
fakekubeclientset "k8s.io/client-go/kubernetes/fake"
corev1listers "k8s.io/client-go/listers/core/v1"
storagev1listers "k8s.io/client-go/listers/storage/v1"
"k8s.io/client-go/tools/cache"
"knative.dev/pkg/reconciler/testing"

Expand Down Expand Up @@ -83,6 +85,10 @@ func (l *Listers) GetPersistentVolumeClaimLister() corev1listers.PersistentVolum
return corev1listers.NewPersistentVolumeClaimLister(l.indexerFor(&corev1.PersistentVolumeClaim{}))
}

func (l *Listers) GetStorageClassLister() storagev1listers.StorageClassLister {
return storagev1listers.NewStorageClassLister(l.indexerFor(&storagev1.StorageClass{}))
}

func (l *Listers) GetPodLister() corev1listers.PodLister {
return corev1listers.NewPodLister(l.indexerFor(&corev1.Pod{}))
}
Expand Down

0 comments on commit daedd68

Please sign in to comment.