Only get VolumeSnapshotClass when DataUpload exists.

Signed-off-by: Xun Jiang <[email protected]>
vmware-tanzu · Aug 28, 2024 · 1df870a · 1df870a
1 parent 8fde4a0
commit 1df870a
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 69 deletions.
diff --git a/changelogs/unreleased/7974-blackpiglet b/changelogs/unreleased/7974-blackpiglet
@@ -0,0 +1 @@
+Only get VolumeSnapshotClass when DataUpload exists.
diff --git a/internal/volume/volumes_information.go b/internal/volume/volumes_information.go
@@ -36,7 +36,6 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/features"
 	"github.com/vmware-tanzu/velero/pkg/itemoperation"
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
-	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 )
 
 type Method string
@@ -551,95 +550,124 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromPVB() {
 	v.volumeInfos = append(v.volumeInfos, tmpVolumeInfos...)
 }
 
-// generateVolumeInfoFromDataUpload generate BackupVolumeInfo for DataUpload.
-func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
-	if !features.IsEnabled(velerov1api.CSIFeatureFlag) {
-		v.logger.Debug("Skip generating BackupVolumeInfo when the CSI feature is disabled.")
-		return
+func (v *BackupVolumesInformation) getVolumeSnapshotClasses(
+	classes *snapshotv1api.VolumeSnapshotClassList,
+) (*snapshotv1api.VolumeSnapshotClassList, error) {
+	if classes != nil {
+		return classes, nil
 	}
 
-	tmpVolumeInfos := make([]*BackupVolumeInfo, 0)
 	vsClassList := new(snapshotv1api.VolumeSnapshotClassList)
 	if err := v.crClient.List(context.TODO(), vsClassList); err != nil {
-		v.logger.WithError(err).Errorf("cannot list VolumeSnapshotClass %s", err.Error())
+		v.logger.Warnf("Cannot list VolumeSnapshotClass with error %s.", err.Error())
+		return nil, err
+	}
+
+	return vsClassList, nil
+}
+
+// generateVolumeInfoFromDataUpload generate BackupVolumeInfo for DataUpload.
+func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
+	if !features.IsEnabled(velerov1api.CSIFeatureFlag) {
+		v.logger.Debug("Skip generating BackupVolumeInfo when the CSI feature is disabled.")
 		return
 	}
 
+	// Retrieve the operations containing DataUpload.
+	duOperationMap := make(map[kbclient.ObjectKey]*itemoperation.BackupOperation)
 	for _, operation := range v.BackupOperations {
 		if operation.Spec.ResourceIdentifier.GroupResource.String() == kuberesource.PersistentVolumeClaims.String() {
-			var duIdentifier velero.ResourceIdentifier
-
 			for _, identifier := range operation.Spec.PostOperationItems {
 				if identifier.GroupResource.String() == "datauploads.velero.io" {
-					duIdentifier = identifier
+					duOperationMap[kbclient.ObjectKey{
+						Namespace: identifier.Namespace,
+						Name:      identifier.Name,
+					}] = operation
+
+					break
 				}
 			}
-			if duIdentifier.Empty() {
-				v.logger.Warnf("cannot find DataUpload for PVC %s/%s backup async operation",
-					operation.Spec.ResourceIdentifier.Namespace, operation.Spec.ResourceIdentifier.Name)
-				continue
-			}
+		}
+	}
+
+	var vsClassList *snapshotv1api.VolumeSnapshotClassList
+	if len(duOperationMap) > 0 {
+		var err error
+		vsClassList, err = v.getVolumeSnapshotClasses(vsClassList)
+		if err != nil {
+			return
+		}
+	} else {
+		// No DataUpload is found. Return early.
+		return
+	}
 
-			dataUpload := new(velerov2alpha1.DataUpload)
-			err := v.crClient.Get(
-				context.TODO(),
-				kbclient.ObjectKey{
-					Namespace: duIdentifier.Namespace,
-					Name:      duIdentifier.Name},
-				dataUpload,
+	tmpVolumeInfos := make([]*BackupVolumeInfo, 0)
+	for duObjectKey, operation := range duOperationMap {
+		dataUpload := new(velerov2alpha1.DataUpload)
+		err := v.crClient.Get(
+			context.TODO(),
+			duObjectKey,
+			dataUpload,
+		)
+		if err != nil {
+			v.logger.Warnf("Fail to get DataUpload %s: %s",
+				duObjectKey.Namespace+"/"+duObjectKey.Name,
+				err.Error(),
 			)
-			if err != nil {
-				v.logger.Warnf("fail to get DataUpload for operation %s: %s", operation.Spec.OperationID, err.Error())
-				continue
-			}
+			continue
+		}
 
-			driverUsedByVSClass := ""
-			for index := range vsClassList.Items {
-				if vsClassList.Items[index].Name == dataUpload.Spec.CSISnapshot.SnapshotClass {
-					driverUsedByVSClass = vsClassList.Items[index].Driver
-				}
+		driverUsedByVSClass := ""
+		for index := range vsClassList.Items {
+			if vsClassList.Items[index].Name == dataUpload.Spec.CSISnapshot.SnapshotClass {
+				driverUsedByVSClass = vsClassList.Items[index].Driver
 			}
+		}
 
-			if pvcPVInfo := v.pvMap.retrieve("", operation.Spec.ResourceIdentifier.Name, operation.Spec.ResourceIdentifier.Namespace); pvcPVInfo != nil {
-				dataMover := veleroDatamover
-				if dataUpload.Spec.DataMover != "" {
-					dataMover = dataUpload.Spec.DataMover
-				}
-
-				volumeInfo := &BackupVolumeInfo{
-					BackupMethod:      CSISnapshot,
-					PVCName:           pvcPVInfo.PVCName,
-					PVCNamespace:      pvcPVInfo.PVCNamespace,
-					PVName:            pvcPVInfo.PV.Name,
-					SnapshotDataMoved: true,
-					Skipped:           false,
-					CSISnapshotInfo: &CSISnapshotInfo{
-						SnapshotHandle: FieldValueIsUnknown,
-						VSCName:        FieldValueIsUnknown,
-						OperationID:    FieldValueIsUnknown,
-						Driver:         driverUsedByVSClass,
-					},
-					SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
-						DataMover:    dataMover,
-						UploaderType: kopia,
-						OperationID:  operation.Spec.OperationID,
-						Phase:        dataUpload.Status.Phase,
-					},
-					PVInfo: &PVInfo{
-						ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
-						Labels:        pvcPVInfo.PV.Labels,
-					},
-				}
+		if pvcPVInfo := v.pvMap.retrieve(
+			"",
+			operation.Spec.ResourceIdentifier.Name,
+			operation.Spec.ResourceIdentifier.Namespace,
+		); pvcPVInfo != nil {
+			dataMover := veleroDatamover
+			if dataUpload.Spec.DataMover != "" {
+				dataMover = dataUpload.Spec.DataMover
+			}
 
-				if dataUpload.Status.StartTimestamp != nil {
-					volumeInfo.StartTimestamp = dataUpload.Status.StartTimestamp
-				}
+			volumeInfo := &BackupVolumeInfo{
+				BackupMethod:      CSISnapshot,
+				PVCName:           pvcPVInfo.PVCName,
+				PVCNamespace:      pvcPVInfo.PVCNamespace,
+				PVName:            pvcPVInfo.PV.Name,
+				SnapshotDataMoved: true,
+				Skipped:           false,
+				CSISnapshotInfo: &CSISnapshotInfo{
+					SnapshotHandle: FieldValueIsUnknown,
+					VSCName:        FieldValueIsUnknown,
+					OperationID:    FieldValueIsUnknown,
+					Driver:         driverUsedByVSClass,
+				},
+				SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
+					DataMover:    dataMover,
+					UploaderType: kopia,
+					OperationID:  operation.Spec.OperationID,
+					Phase:        dataUpload.Status.Phase,
+				},
+				PVInfo: &PVInfo{
+					ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy),
+					Labels:        pvcPVInfo.PV.Labels,
+				},
+			}
 
-				tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
-			} else {
-				v.logger.Warnf("Cannot find info for PVC %s/%s", operation.Spec.ResourceIdentifier.Namespace, operation.Spec.ResourceIdentifier.Name)
-				continue
+			if dataUpload.Status.StartTimestamp != nil {
+				volumeInfo.StartTimestamp = dataUpload.Status.StartTimestamp
 			}
+
+			tmpVolumeInfos = append(tmpVolumeInfos, volumeInfo)
+		} else {
+			v.logger.Warnf("Cannot find info for PVC %s/%s", operation.Spec.ResourceIdentifier.Namespace, operation.Spec.ResourceIdentifier.Name)
+			continue
 		}
 	}
 

diff --git a/internal/volume/volumes_information_test.go b/internal/volume/volumes_information_test.go
@@ -1310,3 +1310,12 @@ func int64Ptr(val int) *int64 {
 	i := int64(val)
 	return &i
 }
+
+func TestGetVolumeSnapshotClasses(t *testing.T) {
+	volumesInfo := BackupVolumesInformation{}
+	var vsClass snapshotv1api.VolumeSnapshotClassList
+
+	result, err := volumesInfo.getVolumeSnapshotClasses(&vsClass)
+	require.NoError(t, err)
+	require.Equal(t, &vsClass, result)
+}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Only get VolumeSnapshotClass when DataUpload exists.