Merge pull request #55 from ashish-amarnath/pvc-size

Set PVC storage requests if restore size is greater

internal/backup/volumesnapshot_action.go

@@ -23,6 +23,7 @@ import (
 	"github.com/sirupsen/logrus"
 
 	snapshotv1beta1api "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -106,12 +107,16 @@ func (p *VolumeSnapshotBackupItemAction) Execute(item runtime.Unstructured, back
 			util.CSIVSCDeletionPolicy: string(vsc.Spec.DeletionPolicy),
 		}
 
-		if vsc.Status != nil && vsc.Status.SnapshotHandle != nil {
-			// Capture storage provider snapshot handle and CSI driver name
-			// to be used on restore to create a static volumesnapshotcontent that will be the source of the volumesnapshot.
-			vals[util.VolumeSnapshotHandleAnnotation] = *vsc.Status.SnapshotHandle
-			vals[util.CSIDriverNameAnnotation] = vsc.Spec.Driver
-
+		if vsc.Status != nil {
+			if vsc.Status.SnapshotHandle != nil {
+				// Capture storage provider snapshot handle and CSI driver name
+				// to be used on restore to create a static volumesnapshotcontent that will be the source of the volumesnapshot.
+				vals[util.VolumeSnapshotHandleAnnotation] = *vsc.Status.SnapshotHandle
+				vals[util.CSIDriverNameAnnotation] = vsc.Spec.Driver
+			}
+			if vsc.Status.RestoreSize != nil {
+				vals[util.VolumeSnapshotRestoreSize] = resource.NewQuantity(*vsc.Status.RestoreSize, resource.BinarySI).String()
+			}
 		}
 		// save newly applied annotations into the backed-up volumesnapshot item
 		util.AddAnnotations(&vs.ObjectMeta, vals)

internal/restore/pvc_action.go

@@ -17,11 +17,15 @@ limitations under the License.
 package restore
 
 import (
+	"fmt"
+
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
 	snapshotv1beta1api "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1"
 	corev1api "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 
@@ -66,6 +70,21 @@ func resetPVCSpec(pvc *corev1api.PersistentVolumeClaim, vsName string) {
 	}
 }
 
+func setPVCStorageResourceRequest(pvc *corev1api.PersistentVolumeClaim, restoreSize resource.Quantity, log logrus.FieldLogger) {
+	{
+		if pvc.Spec.Resources.Requests == nil {
+			pvc.Spec.Resources.Requests = corev1api.ResourceList{}
+		}
+
+		storageReq, exists := pvc.Spec.Resources.Requests[corev1api.ResourceStorage]
+		if !exists || storageReq.Cmp(restoreSize) < 0 {
+			pvc.Spec.Resources.Requests[corev1api.ResourceStorage] = restoreSize
+			rs := pvc.Spec.Resources.Requests[corev1api.ResourceStorage]
+			log.Infof("Resetting storage requests for PVC %s/%s to %s", pvc.Namespace, pvc.Name, rs.String())
+		}
+	}
+}
+
 // Execute modifies the PVC's spec to use the volumesnapshot object as the data source ensuring that the newly provisioned volume
 // can be pre-populated with data from the volumesnapshot.
 func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) {
@@ -84,6 +103,32 @@ func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInp
 			UpdatedItem: input.Item,
 		}, nil
 	}
+
+	_, snapClient, err := util.GetClients()
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+
+	vs, err := snapClient.SnapshotV1beta1().VolumeSnapshots(pvc.Namespace).Get(volumeSnapshotName, metav1.GetOptions{})
+	if err != nil {
+		return nil, errors.Wrapf(err, fmt.Sprintf("Failed to get Volumesnapshot %s/%s to restore PVC %s/%s", pvc.Namespace, volumeSnapshotName, pvc.Namespace, pvc.Name))
+	}
+
+	if _, exists := vs.Annotations[util.VolumeSnapshotRestoreSize]; exists {
+		restoreSize, err := resource.ParseQuantity(vs.Annotations[util.VolumeSnapshotRestoreSize])
+		if err != nil {
+			return nil, errors.Wrapf(err, fmt.Sprintf("Failed to parse %s from annotation on Volumesnapshot %s/%s into restore size",
+				vs.Annotations[util.VolumeSnapshotRestoreSize], vs.Namespace, vs.Name))
+		}
+		// It is possible that the volume provider allocated a larger capacity volume than what was requested in the backed up PVC.
+		// In this scenario the volumesnapshot of the PVC will endup being larger than its requested storage size.
+		// Such a PVC, on restore as-is, will be stuck attempting to use a Volumesnapshot as a data source for a PVC that
+		// is not large enough.
+		// To counter that, here we set the storage request on the PVC to the larger of the PVC's storage request and the size of the
+		// VolumeSnapshot
+		setPVCStorageResourceRequest(&pvc, restoreSize, p.Log)
+	}
+
 	resetPVCSpec(&pvc, volumeSnapshotName)
 
 	pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&pvc)

internal/restore/pvc_action_test.go

@@ -19,6 +19,7 @@ package restore
 import (
 	"testing"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 
 	corev1api "k8s.io/api/core/v1"
@@ -229,3 +230,98 @@ func TestResetPVCSpec(t *testing.T) {
 		})
 	}
 }
+
+func TestResetPVCResourceRequest(t *testing.T) {
+	var storageReq50Mi, storageReq1Gi, cpuQty resource.Quantity
+
+	storageReq50Mi, err := resource.ParseQuantity("50Mi")
+	assert.NoError(t, err)
+	storageReq1Gi, err = resource.ParseQuantity("1Gi")
+	assert.NoError(t, err)
+	cpuQty, err = resource.ParseQuantity("100m")
+	assert.NoError(t, err)
+
+	testCases := []struct {
+		name                      string
+		pvc                       corev1api.PersistentVolumeClaim
+		restoreSize               resource.Quantity
+		expectedStorageRequestQty string
+	}{
+		{
+			name: "should set storage resource request from volumesnapshot, pvc has nil resource requests",
+			pvc: corev1api.PersistentVolumeClaim{
+				Spec: corev1api.PersistentVolumeClaimSpec{
+					Resources: corev1api.ResourceRequirements{
+						Requests: nil,
+					},
+				},
+			},
+			restoreSize:               storageReq50Mi,
+			expectedStorageRequestQty: "50Mi",
+		},
+		{
+			name: "should set storage resource request from volumesnapshot, pvc has empty resource requests",
+			pvc: corev1api.PersistentVolumeClaim{
+				Spec: corev1api.PersistentVolumeClaimSpec{
+					Resources: corev1api.ResourceRequirements{
+						Requests: corev1api.ResourceList{},
+					},
+				},
+			},
+			restoreSize:               storageReq50Mi,
+			expectedStorageRequestQty: "50Mi",
+		},
+		{
+			name: "should merge resource requests from volumesnapshot into pvc with no storage resource requests",
+			pvc: corev1api.PersistentVolumeClaim{
+				Spec: corev1api.PersistentVolumeClaimSpec{
+					Resources: corev1api.ResourceRequirements{
+						Requests: corev1api.ResourceList{
+							corev1api.ResourceCPU: cpuQty,
+						},
+					},
+				},
+			},
+			restoreSize:               storageReq50Mi,
+			expectedStorageRequestQty: "50Mi",
+		},
+		{
+			name: "should set storage resource request from volumesnapshot, pvc requests less storage",
+			pvc: corev1api.PersistentVolumeClaim{
+				Spec: corev1api.PersistentVolumeClaimSpec{
+					Resources: corev1api.ResourceRequirements{
+						Requests: corev1api.ResourceList{
+							corev1api.ResourceStorage: storageReq50Mi,
+						},
+					},
+				},
+			},
+			restoreSize:               storageReq1Gi,
+			expectedStorageRequestQty: "1Gi",
+		},
+		{
+			name: "should not set storage resource request from volumesnapshot, pvc requests more storage",
+			pvc: corev1api.PersistentVolumeClaim{
+				Spec: corev1api.PersistentVolumeClaimSpec{
+					Resources: corev1api.ResourceRequirements{
+						Requests: corev1api.ResourceList{
+							corev1api.ResourceStorage: storageReq1Gi,
+						},
+					},
+				},
+			},
+			restoreSize:               storageReq50Mi,
+			expectedStorageRequestQty: "1Gi",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			log := logrus.New().WithField("unit-test", tc.name)
+			setPVCStorageResourceRequest(&tc.pvc, tc.restoreSize, log)
+			expected, err := resource.ParseQuantity(tc.expectedStorageRequestQty)
+			assert.NoError(t, err)
+			assert.Equal(t, expected, tc.pvc.Spec.Resources.Requests[corev1api.ResourceStorage])
+		})
+	}
+}

internal/util/labels_annotations.go

@@ -19,6 +19,7 @@ package util
 const (
 	VolumeSnapshotLabel              = "velero.io/volume-snapshot-name"
 	VolumeSnapshotHandleAnnotation   = "velero.io/csi-volumesnapshot-handle"
+	VolumeSnapshotRestoreSize        = "velero.io/vsi-volumesnapshot-restore-size"
 	CSIDriverNameAnnotation          = "velero.io/csi-driver-name"
 	CSIDeleteSnapshotSecretName      = "velero.io/csi-deletesnapshotsecret-name"
 	CSIDeleteSnapshotSecretNamespace = "velero.io/csi-deletesnapshotsecret-namespace"