handle non-resizable storage classes for build cache

helm/korifi/kpack-image-builder/role.yaml

@@ -124,3 +124,10 @@ rules:
   verbs:
   - get
   - patch
+- apiGroups:
+  - storage.k8s.io
+  resources:
+  - storageclasses
+  verbs:
+  - list
+  - watch

kpack-image-builder/controllers/buildworkload_controller.go

@@ -37,6 +37,7 @@ import (
 	buildv1alpha2 "github.com/pivotal/kpack/pkg/apis/build/v1alpha2"
 	corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
+	storagev1 "k8s.io/api/storage/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -164,6 +165,8 @@ func filterBuildWorkloads(object client.Object) bool {
 //+kubebuilder:rbac:groups=kpack.io,resources=builds,verbs=deletecollection
 //+kubebuilder:rbac:groups=kpack.io,resources=builders,verbs=get;list;watch;create;patch;update
 
+//+kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses,verbs=list;watch
+
 //+kubebuilder:rbac:groups="",resources=serviceaccounts;secrets,verbs=get;list;watch;patch
 //+kubebuilder:rbac:groups="",resources=serviceaccounts/status;secrets/status,verbs=get
 
@@ -651,10 +654,33 @@ func (r *BuildWorkloadReconciler) reconcileKpackImage(
 
 	cacheSize, err := resource.ParseQuantity(fmt.Sprintf("%dMi", r.controllerConfig.BuildCacheMB))
 	if err != nil {
-		log.Error(err, "failed to parse image cache size")
+		log.Info("failed to parse image cache size", "reason", err)
 		return err
 	}
+
+	recreateImage := false
 	_, err = controllerutil.CreateOrPatch(ctx, r.k8sClient, &desiredKpackImage, func() error {
+		if desiredKpackImage.Spec.Cache != nil &&
+			desiredKpackImage.Spec.Cache.Volume != nil &&
+			desiredKpackImage.Spec.Cache.Volume.Size != nil &&
+			!desiredKpackImage.Spec.Cache.Volume.Size.Equal(cacheSize) {
+			// Our new request wants to resize the PV on the existing Image. Fetch the storage class to see if that is allowed.
+			scName := desiredKpackImage.Spec.Cache.Volume.StorageClassName
+			scResizable, err2 := r.isResizable(ctx, log, scName)
+			if err2 != nil {
+				return err2
+			}
+			if !scResizable {
+				// Signal to recreate the Image with an error
+				log.V(1).Info("WARNING: storage class does not support PVC resize. Recreating.",
+					"storageClassName", scName,
+					"desiredSize", cacheSize.String(),
+					"actualSize", desiredKpackImage.Spec.Cache.Volume.Size.String())
+				recreateImage = true
+				return nil
+			}
+		}
+
 		desiredKpackImage.Labels = map[string]string{
 			BuildWorkloadLabelKey: buildWorkload.Name,
 		}
@@ -703,6 +729,17 @@ func (r *BuildWorkloadReconciler) reconcileKpackImage(
 		return err
 	}
 
+	if recreateImage {
+		// note: we don't need to explicitly requeue because we have a watch on the Image, so deleting it will trigger a requeue of its owner
+		log.V(1).Info("removing kpack image and re-reconciling", "imageName", desiredKpackImage.Name, "imageNamespace", desiredKpackImage.Namespace)
+		err = r.k8sClient.Delete(ctx, &desiredKpackImage)
+		if err != nil {
+			log.Info("failed to delete kpack image on cache resize", "reason", err)
+			return err
+		}
+		return nil
+	}
+
 	meta.SetStatusCondition(&buildWorkload.Status.Conditions, metav1.Condition{
 		Type:               korifiv1alpha1.SucceededConditionType,
 		Status:             metav1.ConditionUnknown,
@@ -716,6 +753,26 @@ func (r *BuildWorkloadReconciler) reconcileKpackImage(
 	return nil
 }
 
+func (r *BuildWorkloadReconciler) isResizable(ctx context.Context, log logr.Logger, scName string) (bool, error) {
+	scList := storagev1.StorageClassList{}
+	if listErr := r.k8sClient.List(ctx, &scList); listErr != nil {
+		log.Info("unable to list storage classes", "reason", listErr)
+		return false, listErr
+	}
+	for _, sc := range scList.Items {
+		defaultVal, exists := sc.Annotations["storageclass.kubernetes.io/is-default-class"]
+		if sc.Name == scName || (scName == "" && exists && defaultVal == "true") {
+			if sc.AllowVolumeExpansion == nil {
+				return false, nil
+			}
+			return *sc.AllowVolumeExpansion, nil
+		}
+	}
+
+	log.Info("unable to locate matching or default storage class", "storageClassName", scName)
+	return false, nil
+}
+
 func (r *BuildWorkloadReconciler) generateDropletStatus(ctx context.Context, kpackBuild *buildv1alpha2.Build, imagePullSecrets []corev1.LocalObjectReference) (*korifiv1alpha1.BuildDropletStatus, error) {
 	imageRef := kpackBuild.Status.LatestImage
 
@@ -781,20 +838,20 @@ func (r *BuildWorkloadReconciler) finalize(ctx context.Context, log logr.Logger,
 
 	lastBuildWorkload, err := r.isLastBuildWorkload(ctx, buildWorkload)
 	if err != nil {
-		log.Error(err, "failed to check for last build workloads")
+		log.Info("failed to check for last build workloads", "reason", err)
 		return ctrl.Result{}, err
 	}
 
 	if lastBuildWorkload {
 		err = r.deleteBuildsForBuildWorkload(ctx, buildWorkload)
 		if err != nil {
-			log.Error(err, "failed to delete builds for build workload")
+			log.Info("failed to delete builds for build workload", "reason", err)
 			return ctrl.Result{}, err
 		}
 
 		hasRemainingBuilds, err := r.hasRemainingBuilds(ctx, buildWorkload)
 		if err != nil {
-			log.Error(err, "failed to check for remaining builds for build workload")
+			log.Info("failed to check for remaining builds for build workload", "reason", err)
 			return ctrl.Result{}, err
 		}
 

kpack-image-builder/controllers/buildworkload_controller_test.go

@@ -43,9 +43,11 @@ var _ = Describe("BuildWorkloadReconciler", func() {
 		reconcilerName            string
 		buildpacks                []string
 		imageRepoCreatorCallCount int
+		expectedCacheVolumeSize   string
 	)
 
 	BeforeEach(func() {
+		expectedCacheVolumeSize = "1024Mi"
 		reconcilerName = "kpack-image-builder"
 		namespaceGUID = PrefixedGUID("namespace")
 		Expect(k8sClient.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespaceGUID}})).To(Succeed())
@@ -149,6 +151,8 @@ var _ = Describe("BuildWorkloadReconciler", func() {
 		})
 
 		CheckInitialization := func() {
+			GinkgoHelper()
+
 			It("reconciles the kpack.Image", func() {
 				Eventually(func(g Gomega) {
 					kpackImage := new(buildv1alpha2.Image)
@@ -161,7 +165,7 @@ var _ = Describe("BuildWorkloadReconciler", func() {
 
 					g.Expect(kpackImage.Spec.Builder.Kind).To(Equal("ClusterBuilder"))
 					g.Expect(kpackImage.Spec.Builder.Name).To(Equal("cf-kpack-builder"))
-					g.Expect(kpackImage.Spec.Cache.Volume.Size.Equal(resource.MustParse("1024Mi"))).To(BeTrue())
+					g.Expect(kpackImage.Spec.Cache.Volume.Size.Equal(resource.MustParse(expectedCacheVolumeSize))).To(BeTrue())
 				}).Should(Succeed())
 			})
 
@@ -215,6 +219,54 @@ var _ = Describe("BuildWorkloadReconciler", func() {
 			CheckInitialization()
 		})
 
+		When("kpack image exists with a different cache size and the storage class doesn't allow resize", func() {
+			var originalImageUID types.UID
+			BeforeEach(func() {
+				oldSize := resource.MustParse("512Mi")
+				Expect(k8sClient.Create(ctx, &buildv1alpha2.Image{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "app-guid",
+						Namespace: namespaceGUID,
+						Labels: map[string]string{
+							controllers.BuildWorkloadLabelKey: buildWorkloadGUID,
+						},
+					},
+					Spec: buildv1alpha2.ImageSpec{
+						Tag: "my-tag-string",
+						Builder: corev1.ObjectReference{
+							Name: "my-builder",
+						},
+						ServiceAccountName: "my-service-account",
+						Source: corev1alpha1.SourceConfig{
+							Registry: &corev1alpha1.Registry{
+								Image:            "not-an-image",
+								ImagePullSecrets: nil,
+							},
+						},
+						Cache: &buildv1alpha2.ImageCacheConfig{
+							Volume: &buildv1alpha2.ImagePersistentVolumeCache{Size: &oldSize, StorageClassName: "non-resizable-class"},
+						},
+					},
+				})).To(Succeed())
+				Eventually(func(g Gomega) {
+					kpackImage := new(buildv1alpha2.Image)
+					g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "app-guid", Namespace: namespaceGUID}, kpackImage)).To(Succeed())
+					originalImageUID = kpackImage.UID
+					g.Expect(originalImageUID).NotTo(BeEmpty())
+				}).Should(Succeed())
+
+			})
+
+			It("deletes the kpack image and recreates it", func() {
+				Eventually(func(g Gomega) {
+					kpackImage := new(buildv1alpha2.Image)
+					g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "app-guid", Namespace: namespaceGUID}, kpackImage)).To(Succeed())
+					g.Expect(kpackImage.UID).NotTo(Equal(originalImageUID))
+				}).Should(Succeed())
+			})
+
+			CheckInitialization()
+		})
 		When("the source image pull secret doesn't exist", func() {
 			var nonExistentSecret string
 

kpack-image-builder/controllers/suite_test.go

@@ -22,24 +22,27 @@ import (
 	"testing"
 	"time"
 
+	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
+	"code.cloudfoundry.org/korifi/controllers/config"
+	"code.cloudfoundry.org/korifi/kpack-image-builder/controllers"
+	"code.cloudfoundry.org/korifi/kpack-image-builder/controllers/fake"
+	"code.cloudfoundry.org/korifi/kpack-image-builder/controllers/webhooks/finalizer"
+	"code.cloudfoundry.org/korifi/tools"
+	"code.cloudfoundry.org/korifi/tools/k8s"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	buildv1alpha2 "github.com/pivotal/kpack/pkg/apis/build/v1alpha2"
-	v1 "k8s.io/api/core/v1"
+	"k8s.io/api/core/v1"
+	storagev1 "k8s.io/api/storage/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
-
-	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/config"
-	"code.cloudfoundry.org/korifi/kpack-image-builder/controllers"
-	"code.cloudfoundry.org/korifi/kpack-image-builder/controllers/fake"
-	"code.cloudfoundry.org/korifi/kpack-image-builder/controllers/webhooks/finalizer"
-	"code.cloudfoundry.org/korifi/tools/k8s"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -176,6 +179,15 @@ var _ = BeforeSuite(func() {
 		err = k8sManager.Start(ctx)
 		Expect(err).NotTo(HaveOccurred())
 	}()
+
+	// create a test storage class that can't be resized
+	Expect(k8sClient.Create(ctx, &storagev1.StorageClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "non-resizable-class",
+		},
+		Provisioner:          "some-fancy-provisioner",
+		AllowVolumeExpansion: tools.PtrTo(false),
+	})).To(Succeed())
 })
 
 var _ = AfterSuite(func() {