diff --git a/pkg/reconciliation/reconcile_racks.go b/pkg/reconciliation/reconcile_racks.go index 284d8c2b..670b4718 100644 --- a/pkg/reconciliation/reconcile_racks.go +++ b/pkg/reconciliation/reconcile_racks.go @@ -179,8 +179,7 @@ func (rc *ReconciliationContext) CheckPVCResizing() result.ReconcileResult { } for _, pvc := range pvcList { - if isPVCStatusConditionTrue(pvc, corev1.PersistentVolumeClaimResizing) || - isPVCStatusConditionTrue(pvc, corev1.PersistentVolumeClaimFileSystemResizePending) { + if isPVCResizing(pvc) { rc.ReqLogger.Info("Waiting for PVC resize to complete", "pvc", pvc.Name) return result.RequeueSoon(10) @@ -190,6 +189,11 @@ func (rc *ReconciliationContext) CheckPVCResizing() result.ReconcileResult { return result.Continue() } +func isPVCResizing(pvc corev1.PersistentVolumeClaim) bool { + return isPVCStatusConditionTrue(pvc, corev1.PersistentVolumeClaimResizing) || + isPVCStatusConditionTrue(pvc, corev1.PersistentVolumeClaimFileSystemResizePending) +} + func isPVCStatusConditionTrue(pvc corev1.PersistentVolumeClaim, conditionType corev1.PersistentVolumeClaimConditionType) bool { for _, condition := range pvc.Status.Conditions { if condition.Type == conditionType && condition.Status == corev1.ConditionTrue { @@ -274,13 +278,16 @@ func (rc *ReconciliationContext) CheckVolumeClaimSizes(statefulSet, desiredSts * } for _, pvc := range targetPVCs { + if isPVCResizing(pvc) { + return result.RequeueSoon(10) + } patch := client.MergeFrom(pvc.DeepCopy()) pvc.Spec.Resources.Requests[corev1.ResourceStorage] = createdSize if err := rc.Client.Patch(rc.Ctx, &pvc, patch); err != nil { return result.Error(err) } } - return result.RequeueSoon(10) + return result.Continue() } } } @@ -379,7 +386,7 @@ func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult { // copy the stuff that can't be updated - // TODO Careful with this, we probably only want to allow expanding the server-data at first + // TODO Add a annotation check for "experimental-feature" here and if not enabled, use old method if res := rc.CheckVolumeClaimSizes(statefulSet, desiredSts); res.Completed() { return res } diff --git a/pkg/reconciliation/reconcile_racks_test.go b/pkg/reconciliation/reconcile_racks_test.go index 0d850980..be136098 100644 --- a/pkg/reconciliation/reconcile_racks_test.go +++ b/pkg/reconciliation/reconcile_racks_test.go @@ -2227,7 +2227,7 @@ func TestVolumeClaimSizesExpansion(t *testing.T) { desiredStatefulSet, err := newStatefulSetForCassandraDatacenter(nil, "default", rc.Datacenter, 2) require.NoErrorf(err, "error occurred creating statefulset") res = rc.CheckVolumeClaimSizes(originalStatefulSet, desiredStatefulSet) - require.Equal(result.RequeueSoon(10), res, "We made changes to the PVC size, requeue and wait for the PVC to resize") + require.Equal(result.Continue(), res, "We made changes to the PVC size") cond, found := rc.Datacenter.GetCondition(api.DatacenterResizingVolumes) require.True(found) @@ -2315,3 +2315,58 @@ func TestPVCResizingCheck(t *testing.T) { res = rc.CheckPVCResizing() require.Equal(result.Continue(), res, "No resizing in progress, we should simply continue") } + +func TestCheckRackPodTemplateWithVolumeExpansion(t *testing.T) { + require := require.New(t) + rc, _, cleanpMockSrc := setupTest() + defer cleanpMockSrc() + + require.NoError(rc.CalculateRackInformation()) + res := rc.CheckRackCreation() + require.False(res.Completed(), "CheckRackCreation did not complete as expected") + + require.Equal(result.Continue(), rc.CheckRackPodTemplate()) + + // Get the current StS + sts := &appsv1.StatefulSet{} + nsName := newNamespacedNameForStatefulSet(rc.Datacenter, "default") + require.NoError(rc.Client.Get(rc.Ctx, nsName, sts)) + require.Equal(resource.MustParse("1Gi"), sts.Spec.VolumeClaimTemplates[0].Spec.Resources.Requests[corev1.ResourceStorage]) + + // Create the PVCs for the StatefulSet + for i := 0; i < int(*sts.Spec.Replicas); i++ { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("server-data-%s-%d", sts.Name, i), + Namespace: "default", + }, + } + pvc.Spec = sts.Spec.VolumeClaimTemplates[0].Spec + pvc.Labels = sts.Spec.VolumeClaimTemplates[0].Labels + require.NoError(rc.Client.Create(rc.Ctx, pvc)) + } + + require.Equal(result.Continue(), rc.CheckRackPodTemplate()) + + rc.Datacenter.Spec.StorageConfig.CassandraDataVolumeClaimSpec.Resources.Requests = map[corev1.ResourceName]resource.Quantity{"storage": resource.MustParse("2Gi")} + require.NoError(rc.Client.Update(rc.Ctx, rc.Datacenter)) + res = rc.CheckRackPodTemplate() + require.Equal(result.Error(fmt.Errorf("PVC resize requested, but StorageClass does not support expansion")), res, "We should have an error, shrinking is disabled") + + // Mark the StorageClass as allowing expansion + 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)) + + res = rc.CheckRackPodTemplate() + require.Equal(result.Done(), res, "Recreating StS should throw us to silence period") + + require.NoError(rc.Client.Get(rc.Ctx, nsName, sts)) + require.Equal(resource.MustParse("2Gi"), sts.Spec.VolumeClaimTemplates[0].Spec.Resources.Requests[corev1.ResourceStorage]) + + // The fakeClient behavior does not prevent us from modifying the StS fields, so this test behaves unlike real world in that sense + res = rc.CheckRackPodTemplate() + require.Equal(result.Continue(), res, "Recreating StS should throw us to silence period") + +}