Fix a bug that would cause a crash if StorageClassName was left to nil (

#688)

* Fix a bug that would cause a crash if StorageClassName was left to nil

* Fix Dockerfiles and wait for rerun of e2e tests

* Update to TopoLVM 15.2.0 chart version and add back the logging in case of failure

* Reorder some code to avoid unnecessary reads to Kubernetes

.github/workflows/kindIntegTest.yml

@@ -311,7 +311,7 @@ jobs:
         with:
           repository: topolvm/topolvm
           path: topolvm
-          ref: topolvm-chart-v15.0.0
+          ref: topolvm-chart-v15.2.0
       - name: Create LVM from TopoLVM's example setup
         run: |
           cd topolvm/example
@@ -367,7 +367,7 @@ jobs:
         run: |
           IMG=${{ steps.load.outputs.operator_img }} LOG_IMG=${{ steps.load.outputs.logger_img }} make integ-test
       - name: Archive k8s logs
-        # if: ${{ failure() }}
+        if: ${{ failure() }}
         uses: actions/upload-artifact@v4
         with:
           name: k8s-logs-topolvm-test-${{ matrix.version }}

CHANGELOG.md

@@ -11,9 +11,11 @@ Changelog for Cass Operator, new PRs should update the `main / unreleased` secti
 
 ## unreleased
 
+* [BUGFIX] [#687](https://github.com/k8ssandra/cass-operator/issues/687) Prevent a crash when when StorageClassName was not set in the CassandraDataVolumeClaimSpec
+
 ## v1.22.0
 
-* [FEATURE] [#263]((https://github.com/k8ssandra/cass-operator/issues/263) Allow increasing the size of CassandraDataVolumeClaimSpec if the selected StorageClass supports it. This feature is currently behind a opt-in feature flag and requires an annotation ``cassandra.datastax.com/allow-storage-changes: true`` to be set in the CassandraDatacenter.
+* [FEATURE] [#263](https://github.com/k8ssandra/cass-operator/issues/263) Allow increasing the size of CassandraDataVolumeClaimSpec if the selected StorageClass supports it. This feature is currently behind a opt-in feature flag and requires an annotation ``cassandra.datastax.com/allow-storage-changes: true`` to be set in the CassandraDatacenter.
 * [FEATURE] [#646](https://github.com/k8ssandra/cass-operator/issues/646) Allow starting multiple parallel pods if they have already previously bootstrapped and not planned for replacement. Set annotation ``cassandra.datastax.com/allow-parallel-starts: true`` to enable this feature.
 * [ENHANCEMENT] [#648](https://github.com/k8ssandra/cass-operator/issues/648) Make MinReadySeconds configurable value in the Spec.
 * [ENHANCEMENT] [#184](https://github.com/k8ssandra/cass-operator/issues/349) Add CassandraDatacenter.Status fields as metrics also

Dockerfile

@@ -1,5 +1,5 @@
 # Build the manager binary
-FROM golang:1.22 as builder
+FROM golang:1.22 AS builder
 ARG TARGETOS
 ARG TARGETARCH
 

logger.Dockerfile

@@ -1,9 +1,9 @@
-FROM redhat/ubi8:latest as builder
+FROM redhat/ubi8:latest AS builder
 ARG VERSION
 ARG TARGETPLATFORM
 
 # Install Vector
-ENV VECTOR_VERSION 0.39.0
+ENV VECTOR_VERSION=0.39.0
 RUN case ${TARGETPLATFORM} in \
          "linux/amd64")  VECTOR_ARCH=x86_64  ;; \
          "linux/arm64")  VECTOR_ARCH=aarch64  ;; \

pkg/reconciliation/reconcile_datacenter.go

@@ -162,14 +162,14 @@ func (rc *ReconciliationContext) listPVCs(selector map[string]string) ([]corev1.
 	return pvcList.Items, nil
 }
 
-func storageClass(ctx context.Context, c client.Client, storageClassName string) (*storagev1.StorageClass, error) {
-	if storageClassName == "" {
+func storageClass(ctx context.Context, c client.Client, storageClassName *string) (*storagev1.StorageClass, error) {
+	if storageClassName == nil || *storageClassName == "" {
 		storageClassList := &storagev1.StorageClassList{}
 		if err := c.List(ctx, storageClassList, client.MatchingLabels{"storageclass.kubernetes.io/is-default-class": "true"}); err != nil {
 			return nil, err
 		}
 
-		if len(storageClassList.Items) > 0 {
+		if len(storageClassList.Items) > 1 {
 			return nil, fmt.Errorf("found multiple default storage classes, please specify StorageClassName in the CassandraDatacenter spec")
 		} else if len(storageClassList.Items) == 0 {
 			return nil, fmt.Errorf("no default storage class found, please specify StorageClassName in the CassandraDatacenter spec")
@@ -179,7 +179,7 @@ func storageClass(ctx context.Context, c client.Client, storageClassName string)
 	}
 
 	storageClass := &storagev1.StorageClass{}
-	if err := c.Get(ctx, types.NamespacedName{Name: storageClassName}, storageClass); err != nil {
+	if err := c.Get(ctx, types.NamespacedName{Name: *storageClassName}, storageClass); err != nil {
 		return nil, err
 	}
 
@@ -188,7 +188,7 @@ func storageClass(ctx context.Context, c client.Client, storageClassName string)
 
 func (rc *ReconciliationContext) storageExpansion() (bool, error) {
 	storageClassName := rc.Datacenter.Spec.StorageConfig.CassandraDataVolumeClaimSpec.StorageClassName
-	storageClass, err := storageClass(rc.Ctx, rc.Client, *storageClassName)
+	storageClass, err := storageClass(rc.Ctx, rc.Client, storageClassName)
 	if err != nil {
 		return false, err
 	}

pkg/reconciliation/reconcile_datacenter_test.go

@@ -9,10 +9,14 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
+	storagev1 "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 
 	"github.com/k8ssandra/cass-operator/pkg/mocks"
 )
@@ -115,3 +119,23 @@ func TestDeletePVCs_FailedToDelete(t *testing.T) {
 
 	assert.EqualError(t, err, "failed to delete")
 }
+
+func TestStorageExpansionNils(t *testing.T) {
+	rc, _, cleanupMockScr := setupTest()
+	defer cleanupMockScr()
+	require := require.New(t)
+
+	rc.Datacenter.Spec.StorageConfig.CassandraDataVolumeClaimSpec.StorageClassName = nil
+	supports, err := rc.storageExpansion()
+	require.NoError(err)
+	require.False(supports)
+
+	storageClass := &storagev1.StorageClass{}
+	require.NoError(rc.Client.Get(rc.Ctx, types.NamespacedName{Name: "standard"}, storageClass))
+	storageClass.AllowVolumeExpansion = ptr.To[bool](true)
+	require.NoError(rc.Client.Update(rc.Ctx, storageClass))
+
+	supports, err = rc.storageExpansion()
+	require.NoError(err)
+	require.True(supports)
+}

pkg/reconciliation/reconcile_racks.go

@@ -210,11 +210,6 @@ func isPVCStatusConditionTrue(pvc *corev1.PersistentVolumeClaim, conditionType c
 func (rc *ReconciliationContext) CheckVolumeClaimSizes(statefulSet, desiredSts *appsv1.StatefulSet) result.ReconcileResult {
 	rc.ReqLogger.Info("reconcile_racks::CheckVolumeClaims")
 
-	supportsExpansion, err := rc.storageExpansion()
-	if err != nil {
-		return result.Error(err)
-	}
-
 	for i, claim := range statefulSet.Spec.VolumeClaimTemplates {
 		// Find the desired one
 		desiredClaim := desiredSts.Spec.VolumeClaimTemplates[i]
@@ -225,8 +220,6 @@ func (rc *ReconciliationContext) CheckVolumeClaimSizes(statefulSet, desiredSts *
 		currentSize := claim.Spec.Resources.Requests[corev1.ResourceStorage]
 		createdSize := desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage]
 
-		// TODO This code is a bit repetitive with all the Status patches. Needs a refactoring in cass-operator since this is a known
-		// 		pattern. https://github.com/k8ssandra/cass-operator/issues/669
 		if currentSize.Cmp(createdSize) > 0 {
 			msg := fmt.Sprintf("shrinking PVC %s is not supported", claim.Name)
 			if err := rc.setCondition(
@@ -248,6 +241,11 @@ func (rc *ReconciliationContext) CheckVolumeClaimSizes(statefulSet, desiredSts *
 				return result.Error(fmt.Errorf(msg))
 			}
 
+			supportsExpansion, err := rc.storageExpansion()
+			if err != nil {
+				return result.Error(err)
+			}
+
 			if !supportsExpansion {
 				msg := fmt.Sprintf("PVC resize requested, but StorageClass %s does not support expansion", *claim.Spec.StorageClassName)
 				rc.Recorder.Eventf(rc.Datacenter, corev1.EventTypeWarning, events.InvalidDatacenterSpec, msg)

pkg/reconciliation/testing.go

@@ -77,7 +77,8 @@ func CreateMockReconciliationContext(
 
 	storageClass := &storagev1.StorageClass{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: storageClassName,
+			Name:   storageClassName,
+			Labels: map[string]string{"storageclass.kubernetes.io/is-default-class": "true"},
 		},
 	}