Skip to content

Commit

Permalink
Let VoluemResize actually just continue instead of waiting for the PV…
Browse files Browse the repository at this point in the history
…C resize, we will do a requeue and check there before recreating the StatefulSet
  • Loading branch information
burmanm committed Jun 27, 2024
1 parent 174240e commit 36ff028
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 5 deletions.
15 changes: 11 additions & 4 deletions pkg/reconciliation/reconcile_racks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
}
}
}
Expand Down Expand Up @@ -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
}
Expand Down
57 changes: 56 additions & 1 deletion pkg/reconciliation/reconcile_racks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")

}

0 comments on commit 36ff028

Please sign in to comment.