Skip to content

Commit e1ec4c8

Browse files
sseagoamastbau
authored andcommitted
Summary of Changes:
Move PVC Request Size Patch to Backup CSI Action Shifted the logic that patches the PVC request size (to match the corresponding VolumeSnapshot size) from the CSI Restore action to the CSI Backup action. ✅ This enables restoring a PVC independently using label selectors, without requiring the VolumeSnapshot to be restored first. Include VolumeSnapshot in CSI Additional Items for PVC Added VolumeSnapshot as an additional item in the PVC CSI backup logic to ensure necessary metadata is available during restore. Include VolumeSnapshotContent in CSI Restore Additional Items Added VolumeSnapshotContent to the additional items in the CSI restore action to support a more complete restore workflow. ✅ This to make sure those resources are restored even if filters out by label from the resources list to restore Author: Amos Mastbaum <[email protected]> Signed-off-by: Amos Mastbaum <[email protected]> fixing-after-michal wait-for-vsc.Status.RestoreSize wait-for-vsc.Status.RestoreSize Update pkg/util/csi/volume_snapshot.go Update pkg/util/csi/volume_snapshot.go Update pkg/util/csi/volume_snapshot.go
1 parent b17d7cd commit e1ec4c8

File tree

6 files changed

+207
-227
lines changed

6 files changed

+207
-227
lines changed

pkg/backup/actions/csi/pvc_action.go

Lines changed: 84 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import (
3535
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
3636
crclient "sigs.k8s.io/controller-runtime/pkg/client"
3737

38+
"k8s.io/apimachinery/pkg/api/resource"
39+
3840
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
3941
velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1"
4042
"github.com/vmware-tanzu/velero/pkg/client"
@@ -252,6 +254,24 @@ func (p *pvcBackupItemAction) Execute(
252254
return nil, nil, "", nil, err
253255
}
254256

257+
// Wait until VS associated VSC snapshot handle created before
258+
// continue.we later requier the vsc restore size
259+
vsc, err := csi.WaitUntilVSCHandleIsReady(
260+
vs,
261+
p.crClient,
262+
p.log,
263+
true,
264+
backup.Spec.CSISnapshotTimeout.Duration,
265+
)
266+
if err != nil {
267+
p.log.Errorf(
268+
"Fail to wait VolumeSnapshot turned to ReadyToUse: %s",
269+
err.Error(),
270+
)
271+
csi.CleanupVolumeSnapshot(vs, p.crClient, p.log)
272+
return nil, nil, "", nil, errors.WithStack(err)
273+
}
274+
255275
labels := map[string]string{
256276
velerov1api.VolumeSnapshotLabel: vs.Name,
257277
velerov1api.BackupNameLabel: backup.Name,
@@ -279,24 +299,6 @@ func (p *pvcBackupItemAction) Execute(
279299
"Backup": backup.Name,
280300
})
281301

282-
// Wait until VS associated VSC snapshot handle created before
283-
// returning with the Async operation for data mover.
284-
_, err := csi.WaitUntilVSCHandleIsReady(
285-
vs,
286-
p.crClient,
287-
p.log,
288-
true,
289-
backup.Spec.CSISnapshotTimeout.Duration,
290-
)
291-
if err != nil {
292-
dataUploadLog.Errorf(
293-
"Fail to wait VolumeSnapshot turned to ReadyToUse: %s",
294-
err.Error(),
295-
)
296-
csi.CleanupVolumeSnapshot(vs, p.crClient, p.log)
297-
return nil, nil, "", nil, errors.WithStack(err)
298-
}
299-
300302
dataUploadLog.Info("Starting data upload of backup")
301303

302304
dataUpload, err := createDataUpload(
@@ -340,6 +342,12 @@ func (p *pvcBackupItemAction) Execute(
340342
dataUploadLog.Info("DataUpload is submitted successfully.")
341343
}
342344
} else {
345+
err = setPVCRequestSizeToVSRestoreSize(&pvc, p.crClient, vs.Name, vsc, p.log)
346+
if err != nil {
347+
p.log.Errorf("Failed to set PVC request size: %s", err.Error())
348+
return nil, nil, "", nil, errors.WithStack(err)
349+
}
350+
343351
additionalItems = []velero.ResourceIdentifier{
344352
{
345353
GroupResource: kuberesource.VolumeSnapshots,
@@ -564,3 +572,61 @@ func NewPvcBackupItemAction(f client.Factory) plugincommon.HandlerInitializer {
564572
}, nil
565573
}
566574
}
575+
576+
func setPVCRequestSizeToVSRestoreSize(
577+
pvc *corev1api.PersistentVolumeClaim,
578+
crClient crclient.Client,
579+
volumeSnapshotName string,
580+
vsc *snapshotv1api.VolumeSnapshotContent,
581+
logger logrus.FieldLogger,
582+
) error {
583+
vs := new(snapshotv1api.VolumeSnapshot)
584+
if err := crClient.Get(context.TODO(),
585+
crclient.ObjectKey{
586+
Namespace: pvc.Namespace,
587+
Name: volumeSnapshotName,
588+
},
589+
vs,
590+
); err != nil {
591+
return errors.Wrapf(err, "Failed to get Volumesnapshot %s/%s to restore PVC %s/%s",
592+
pvc.Namespace, volumeSnapshotName, pvc.Namespace, pvc.Name)
593+
}
594+
595+
if vsc.Status.RestoreSize != nil {
596+
logger.Debugf("Patching PVC request size to fit the volumesnapshot restore size %d", vsc.Status.RestoreSize)
597+
restoreSize := *resource.NewQuantity(*vsc.Status.RestoreSize, resource.BinarySI)
598+
599+
// It is possible that the volume provider allocated a larger
600+
// capacity volume than what was requested in the backed up PVC.
601+
// In this scenario the volumesnapshot of the PVC will end being
602+
// larger than its requested storage size. Such a PVC, on restore
603+
// as-is, will be stuck attempting to use a VolumeSnapshot as a
604+
// data source for a PVC that is not large enough.
605+
// To counter that, here we set the storage request on the PVC
606+
// to the larger of the PVC's storage request and the size of the
607+
// VolumeSnapshot
608+
setPVCStorageResourceRequest(pvc, restoreSize, logger)
609+
}
610+
611+
return nil
612+
}
613+
614+
func setPVCStorageResourceRequest(
615+
pvc *corev1api.PersistentVolumeClaim,
616+
restoreSize resource.Quantity,
617+
log logrus.FieldLogger,
618+
) {
619+
{
620+
if pvc.Spec.Resources.Requests == nil {
621+
pvc.Spec.Resources.Requests = corev1api.ResourceList{}
622+
}
623+
624+
storageReq, exists := pvc.Spec.Resources.Requests[corev1api.ResourceStorage]
625+
if !exists || storageReq.Cmp(restoreSize) < 0 {
626+
pvc.Spec.Resources.Requests[corev1api.ResourceStorage] = restoreSize
627+
rs := pvc.Spec.Resources.Requests[corev1api.ResourceStorage]
628+
log.Infof("Resetting storage requests for PVC %s/%s to %s",
629+
pvc.Namespace, pvc.Name, rs.String())
630+
}
631+
}
632+
}

pkg/backup/actions/csi/pvc_action_test.go

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,15 @@ import (
3737
"k8s.io/apimachinery/pkg/util/wait"
3838
crclient "sigs.k8s.io/controller-runtime/pkg/client"
3939

40+
"k8s.io/apimachinery/pkg/api/resource"
41+
4042
"github.com/vmware-tanzu/velero/pkg/apis/velero/shared"
4143
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
4244
velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1"
4345
"github.com/vmware-tanzu/velero/pkg/builder"
4446
factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks"
4547
"github.com/vmware-tanzu/velero/pkg/plugin/velero"
4648
velerotest "github.com/vmware-tanzu/velero/pkg/test"
47-
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
4849
)
4950

5051
func TestExecute(t *testing.T) {
@@ -130,7 +131,7 @@ func TestExecute(t *testing.T) {
130131
},
131132
{
132133
name: "Test ResourcePolicy",
133-
backup: builder.ForBackup("velero", "test").ResourcePolicies("resourcePolicy").SnapshotVolumes(false).Result(),
134+
backup: builder.ForBackup("velero", "test").ResourcePolicies("resourcePolicy").SnapshotVolumes(false).CSISnapshotTimeout(time.Duration(3600) * time.Second).Result(),
134135
resourcePolicy: builder.ForConfigMap("velero", "resourcePolicy").Data("policy", "{\"version\":\"v1\", \"volumePolicies\":[{\"conditions\":{\"csi\": {}},\"action\":{\"type\":\"snapshot\"}}]}").Result(),
135136
pvc: builder.ForPersistentVolumeClaim("velero", "testPVC").VolumeName("testPV").StorageClass("testSC").Phase(corev1.ClaimBound).Result(),
136137
pv: builder.ForPersistentVolume("testPV").CSI("hostpath", "testVolume").Result(),
@@ -170,7 +171,7 @@ func TestExecute(t *testing.T) {
170171
pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.pvc)
171172
require.NoError(t, err)
172173

173-
if boolptr.IsSetToTrue(tc.backup.Spec.SnapshotMoveData) == true {
174+
if tc.pvc != nil {
174175
go func() {
175176
var vsList v1.VolumeSnapshotList
176177
err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 10*time.Second, true, func(ctx context.Context) (bool, error) {
@@ -412,3 +413,72 @@ func TestNewPVCBackupItemAction(t *testing.T) {
412413
_, err1 := plugin1(logger)
413414
require.NoError(t, err1)
414415
}
416+
417+
func TestPVCRequestSize(t *testing.T) {
418+
logger := logrus.New()
419+
420+
tests := []struct {
421+
name string
422+
pvcInitial string // initial storage request on the PVC (e.g. "1Gi" or "3Gi")
423+
restoreSize string // restore size set in VSC.Status.RestoreSize (e.g. "2Gi")
424+
expectedSize string // expected storage request on the PVC after update
425+
}{
426+
{
427+
name: "UpdateRequired: PVC request is lower than restore size",
428+
pvcInitial: "1Gi",
429+
restoreSize: "2Gi",
430+
expectedSize: "2Gi",
431+
},
432+
{
433+
name: "NoUpdateRequired: PVC request is larger than restore size",
434+
pvcInitial: "3Gi",
435+
restoreSize: "2Gi",
436+
expectedSize: "3Gi",
437+
},
438+
}
439+
440+
for _, tc := range tests {
441+
t.Run(tc.name, func(t *testing.T) {
442+
crClient := velerotest.NewFakeControllerRuntimeClient(t)
443+
444+
// Create a PVC with the initial storage request.
445+
pvc := builder.ForPersistentVolumeClaim("velero", "testPVC").
446+
VolumeName("testPV").
447+
StorageClass("testSC").
448+
Result()
449+
pvc.Spec.Resources.Requests = corev1.ResourceList{
450+
corev1.ResourceStorage: resource.MustParse(tc.pvcInitial),
451+
}
452+
453+
// Create a VolumeSnapshot required for the lookup
454+
vs := &snapshotv1api.VolumeSnapshot{
455+
ObjectMeta: metav1.ObjectMeta{
456+
Name: "testVS",
457+
Namespace: "velero",
458+
},
459+
}
460+
require.NoError(t, crClient.Create(context.Background(), vs))
461+
462+
// Create a VolumeSnapshotContent with restore size
463+
rsQty := resource.MustParse(tc.restoreSize)
464+
rsValue := rsQty.Value()
465+
vsc := &snapshotv1api.VolumeSnapshotContent{
466+
ObjectMeta: metav1.ObjectMeta{
467+
Name: "testVSC",
468+
},
469+
Status: &snapshotv1api.VolumeSnapshotContentStatus{
470+
RestoreSize: &rsValue,
471+
},
472+
}
473+
474+
// Call the function under test
475+
err := setPVCRequestSizeToVSRestoreSize(pvc, crClient, "testVS", vsc, logger)
476+
require.NoError(t, err)
477+
478+
// Verify that the PVC storage request is updated as expected.
479+
updatedSize := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
480+
expected := resource.MustParse(tc.expectedSize)
481+
require.Equal(t, 0, updatedSize.Cmp(expected), "PVC storage request should be %s", tc.expectedSize)
482+
})
483+
}
484+
}

0 commit comments

Comments
 (0)