Remove dependency with VolumeSnapshotClass in DataUpload. (#9040)

Don't add VSClass in the additionalItems when it's empty.

Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
Signed-off-by: xun.jiang <xun.jiang@broadcom.com>

changelogs/unreleased/9040-blackpiglet

@@ -0,0 +1 @@
+Remove dependency with VolumeSnapshotClass in DataUpload.

config/crd/v2alpha1/bases/velero.io_datauploads.yaml

@@ -87,6 +87,9 @@ spec:
                   of the CSI snapshot.
                 nullable: true
                 properties:
+                  driver:
+                    description: Driver is the driver used by the VolumeSnapshotContent
+                    type: string
                   snapshotClass:
                     description: SnapshotClass is the name of the snapshot class that
                       the volume snapshot is created with

internal/volume/volumes_information.go

@@ -402,7 +402,6 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() {
 	tmpVolumeInfos := make([]*BackupVolumeInfo, 0)
 
 	for _, volumeSnapshot := range v.volumeSnapshots {
-		var volumeSnapshotClass *snapshotv1api.VolumeSnapshotClass
 		var volumeSnapshotContent *snapshotv1api.VolumeSnapshotContent
 
 		// This is protective logic. The passed-in VS should be all related
@@ -411,11 +410,6 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() {
 			continue
 		}
 
-		if volumeSnapshot.Spec.VolumeSnapshotClassName == nil {
-			v.logger.Warnf("Cannot find VolumeSnapshotClass for VolumeSnapshot %s/%s", volumeSnapshot.Namespace, volumeSnapshot.Name)
-			continue
-		}
-
 		if volumeSnapshot.Status == nil || volumeSnapshot.Status.BoundVolumeSnapshotContentName == nil {
 			v.logger.Warnf("Cannot fine VolumeSnapshotContent for VolumeSnapshot %s/%s", volumeSnapshot.Namespace, volumeSnapshot.Name)
 			continue
@@ -426,20 +420,14 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() {
 			continue
 		}
 
-		for index := range v.volumeSnapshotClasses {
-			if *volumeSnapshot.Spec.VolumeSnapshotClassName == v.volumeSnapshotClasses[index].Name {
-				volumeSnapshotClass = &v.volumeSnapshotClasses[index]
-			}
-		}
-
 		for index := range v.volumeSnapshotContents {
 			if *volumeSnapshot.Status.BoundVolumeSnapshotContentName == v.volumeSnapshotContents[index].Name {
 				volumeSnapshotContent = &v.volumeSnapshotContents[index]
 			}
 		}
 
-		if volumeSnapshotClass == nil || volumeSnapshotContent == nil {
-			v.logger.Warnf("fail to get VolumeSnapshotContent or VolumeSnapshotClass for VolumeSnapshot: %s/%s",
+		if volumeSnapshotContent == nil {
+			v.logger.Warnf("fail to get VolumeSnapshotContent for VolumeSnapshot: %s/%s",
 				volumeSnapshot.Namespace, volumeSnapshot.Name)
 			continue
 		}
@@ -473,7 +461,7 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() {
 				CSISnapshotInfo: &CSISnapshotInfo{
 					VSCName:        *volumeSnapshot.Status.BoundVolumeSnapshotContentName,
 					Size:           size,
-					Driver:         volumeSnapshotClass.Driver,
+					Driver:         volumeSnapshotContent.Spec.Driver,
 					SnapshotHandle: snapshotHandle,
 					OperationID:    operation.Spec.OperationID,
 					ReadyToUse:     volumeSnapshot.Status.ReadyToUse,
@@ -581,14 +569,7 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
 		}
 	}
 
-	var vsClassList []snapshotv1api.VolumeSnapshotClass
-	if len(duOperationMap) > 0 {
-		var err error
-		vsClassList, err = v.getVolumeSnapshotClasses()
-		if err != nil {
-			return
-		}
-	} else {
+	if len(duOperationMap) <= 0 {
 		// No DataUpload is found. Return early.
 		return
 	}
@@ -609,13 +590,6 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
 			continue
 		}
 
-		driverUsedByVSClass := ""
-		for index := range vsClassList {
-			if vsClassList[index].Name == dataUpload.Spec.CSISnapshot.SnapshotClass {
-				driverUsedByVSClass = vsClassList[index].Driver
-			}
-		}
-
 		if pvcPVInfo := v.pvMap.retrieve(
 			"",
 			operation.Spec.ResourceIdentifier.Name,
@@ -637,7 +611,7 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
 					SnapshotHandle: FieldValueIsUnknown,
 					VSCName:        FieldValueIsUnknown,
 					OperationID:    FieldValueIsUnknown,
-					Driver:         driverUsedByVSClass,
+					Driver:         dataUpload.Spec.CSISnapshot.Driver,
 				},
 				SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
 					DataMover:    dataMover,

internal/volume/volumes_information_test.go

@@ -29,6 +29,7 @@ import (
 	corev1api "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
@@ -415,7 +416,6 @@ func TestGenerateVolumeInfoForCSIVolumeSnapshot(t *testing.T) {
 					BoundVolumeSnapshotContentName: stringPtr("testContent"),
 				},
 			},
-			volumeSnapshotClass:   *builder.ForVolumeSnapshotClass("testClass").Driver("pd.csi.storage.gke.io").Result(),
 			volumeSnapshotContent: *builder.ForVolumeSnapshotContent("testContent").Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: stringPtr("testSnapshotHandle")}).Result(),
 			expectedVolumeInfos:   []*BackupVolumeInfo{},
 		},
@@ -440,8 +440,7 @@ func TestGenerateVolumeInfoForCSIVolumeSnapshot(t *testing.T) {
 					ReadyToUse:                     &readyToUse,
 				},
 			},
-			volumeSnapshotClass:   *builder.ForVolumeSnapshotClass("testClass").Driver("pd.csi.storage.gke.io").Result(),
-			volumeSnapshotContent: *builder.ForVolumeSnapshotContent("testContent").Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: stringPtr("testSnapshotHandle")}).Result(),
+			volumeSnapshotContent: *builder.ForVolumeSnapshotContent("testContent").Driver("pd.csi.storage.gke.io").Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: stringPtr("testSnapshotHandle")}).Result(),
 			pvMap: map[string]pvcPvInfo{
 				"testPV": {
 					PVCName:      "testPVC",
@@ -758,7 +757,8 @@ func TestGenerateVolumeInfoFromDataUpload(t *testing.T) {
 	defer features.Disable(velerov1api.CSIFeatureFlag)
 	tests := []struct {
 		name                string
-		volumeSnapshotClass *snapshotv1api.VolumeSnapshotClass
+		vs                  *snapshotv1api.VolumeSnapshot
+		vsc                 *snapshotv1api.VolumeSnapshotContent
 		dataUpload          *velerov2alpha1.DataUpload
 		operation           *itemoperation.BackupOperation
 		pvMap               map[string]pvcPvInfo
@@ -829,87 +829,20 @@ func TestGenerateVolumeInfoFromDataUpload(t *testing.T) {
 			},
 			expectedVolumeInfos: []*BackupVolumeInfo{},
 		},
-		{
-			name: "VolumeSnapshotClass cannot be found for operation",
-			dataUpload: builder.ForDataUpload("velero", "testDU").DataMover("velero").CSISnapshot(&velerov2alpha1.CSISnapshotSpec{
-				VolumeSnapshot: "testVS",
-			}).SnapshotID("testSnapshotHandle").StartTimestamp(&now).Result(),
-			operation: &itemoperation.BackupOperation{
-				Spec: itemoperation.BackupOperationSpec{
-					OperationID: "testOperation",
-					ResourceIdentifier: velero.ResourceIdentifier{
-						GroupResource: schema.GroupResource{
-							Group:    "",
-							Resource: "persistentvolumeclaims",
-						},
-						Namespace: "velero",
-						Name:      "testPVC",
-					},
-					PostOperationItems: []velero.ResourceIdentifier{
-						{
-							GroupResource: schema.GroupResource{
-								Group:    "velero.io",
-								Resource: "datauploads",
-							},
-							Namespace: "velero",
-							Name:      "testDU",
-						},
-					},
-				},
-			},
-			pvMap: map[string]pvcPvInfo{
-				"testPV": {
-					PVCName:      "testPVC",
-					PVCNamespace: "velero",
-					PV: corev1api.PersistentVolume{
-						ObjectMeta: metav1.ObjectMeta{
-							Name:   "testPV",
-							Labels: map[string]string{"a": "b"},
-						},
-						Spec: corev1api.PersistentVolumeSpec{
-							PersistentVolumeReclaimPolicy: corev1api.PersistentVolumeReclaimDelete,
-						},
-					},
-				},
-			},
-			expectedVolumeInfos: []*BackupVolumeInfo{
-				{
-					PVCName:           "testPVC",
-					PVCNamespace:      "velero",
-					PVName:            "testPV",
-					BackupMethod:      CSISnapshot,
-					SnapshotDataMoved: true,
-					StartTimestamp:    &now,
-					CSISnapshotInfo: &CSISnapshotInfo{
-						SnapshotHandle: FieldValueIsUnknown,
-						VSCName:        FieldValueIsUnknown,
-						OperationID:    FieldValueIsUnknown,
-						Size:           0,
-					},
-					SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
-						DataMover:    "velero",
-						UploaderType: "kopia",
-						OperationID:  "testOperation",
-					},
-					PVInfo: &PVInfo{
-						ReclaimPolicy: string(corev1api.PersistentVolumeReclaimDelete),
-						Labels:        map[string]string{"a": "b"},
-					},
-				},
-			},
-		},
 		{
 			name: "Normal DataUpload case",
 			dataUpload: builder.ForDataUpload("velero", "testDU").
 				DataMover("velero").
 				CSISnapshot(&velerov2alpha1.CSISnapshotSpec{
-					VolumeSnapshot: "testVS",
+					VolumeSnapshot: "vs-01",
 					SnapshotClass:  "testClass",
+					Driver:         "pd.csi.storage.gke.io",
 				}).SnapshotID("testSnapshotHandle").
 				StartTimestamp(&now).
 				Phase(velerov2alpha1.DataUploadPhaseCompleted).
 				Result(),
-			volumeSnapshotClass: builder.ForVolumeSnapshotClass("testClass").Driver("pd.csi.storage.gke.io").Result(),
+			vs:  builder.ForVolumeSnapshot(velerov1api.DefaultNamespace, "vs-01").Status().BoundVolumeSnapshotContentName("vsc-01").Result(),
+			vsc: builder.ForVolumeSnapshotContent("vsc-01").Driver("pd.csi.storage.gke.io").Result(),
 			operation: &itemoperation.BackupOperation{
 				Spec: itemoperation.BackupOperationSpec{
 					OperationID: "testOperation",
@@ -995,14 +928,17 @@ func TestGenerateVolumeInfoFromDataUpload(t *testing.T) {
 				}
 			}
 
-			volumesInfo.crClient = velerotest.NewFakeControllerRuntimeClient(t)
+			objects := make([]runtime.Object, 0)
 			if tc.dataUpload != nil {
-				volumesInfo.crClient.Create(context.TODO(), tc.dataUpload)
+				objects = append(objects, tc.dataUpload)
 			}
-
-			if tc.volumeSnapshotClass != nil {
-				volumesInfo.crClient.Create(context.TODO(), tc.volumeSnapshotClass)
+			if tc.vs != nil {
+				objects = append(objects, tc.vs)
 			}
+			if tc.vsc != nil {
+				objects = append(objects, tc.vsc)
+			}
+			volumesInfo.crClient = velerotest.NewFakeControllerRuntimeClient(t, objects...)
 
 			volumesInfo.logger = logging.DefaultLogger(logrus.DebugLevel, logging.FormatJSON)
 

pkg/apis/velero/v2alpha1/data_upload_types.go

@@ -79,6 +79,10 @@ type CSISnapshotSpec struct {
 	// SnapshotClass is the name of the snapshot class that the volume snapshot is created with
 	// +optional
 	SnapshotClass string `json:"snapshotClass"`
+
+	// Driver is the driver used by the VolumeSnapshotContent
+	// +optional
+	Driver string `json:"driver,omitempty"`
 }
 
 // DataUploadPhase represents the lifecycle phase of a DataUpload.

pkg/backup/actions/csi/pvc_action.go

@@ -281,7 +281,7 @@ func (p *pvcBackupItemAction) Execute(
 
 		// Wait until VS associated VSC snapshot handle created before
 		// returning with the Async operation for data mover.
-		_, err := csi.WaitUntilVSCHandleIsReady(
+		vsc, err := csi.WaitUntilVSCHandleIsReady(
 			vs,
 			p.crClient,
 			p.log,
@@ -305,6 +305,7 @@ func (p *pvcBackupItemAction) Execute(
 			vs,
 			&pvc,
 			operationID,
+			vsc,
 		)
 		if err != nil {
 			dataUploadLog.WithError(err).Error("failed to submit DataUpload")
@@ -441,6 +442,7 @@ func newDataUpload(
 	vs *snapshotv1api.VolumeSnapshot,
 	pvc *corev1api.PersistentVolumeClaim,
 	operationID string,
+	vsc *snapshotv1api.VolumeSnapshotContent,
 ) *velerov2alpha1.DataUpload {
 	dataUpload := &velerov2alpha1.DataUpload{
 		TypeMeta: metav1.TypeMeta{
@@ -471,7 +473,7 @@ func newDataUpload(
 			CSISnapshot: &velerov2alpha1.CSISnapshotSpec{
 				VolumeSnapshot: vs.Name,
 				StorageClass:   *pvc.Spec.StorageClassName,
-				SnapshotClass:  *vs.Spec.VolumeSnapshotClassName,
+				Driver:         vsc.Spec.Driver,
 			},
 			SourcePVC:             pvc.Name,
 			DataMover:             backup.Spec.DataMover,
@@ -481,6 +483,10 @@ func newDataUpload(
 		},
 	}
 
+	if vs.Spec.VolumeSnapshotClassName != nil {
+		dataUpload.Spec.CSISnapshot.SnapshotClass = *vs.Spec.VolumeSnapshotClassName
+	}
+
 	if backup.Spec.UploaderConfig != nil &&
 		backup.Spec.UploaderConfig.ParallelFilesUpload > 0 {
 		dataUpload.Spec.DataMoverConfig = make(map[string]string)
@@ -497,8 +503,9 @@ func createDataUpload(
 	vs *snapshotv1api.VolumeSnapshot,
 	pvc *corev1api.PersistentVolumeClaim,
 	operationID string,
+	vsc *snapshotv1api.VolumeSnapshotContent,
 ) (*velerov2alpha1.DataUpload, error) {
-	dataUpload := newDataUpload(backup, vs, pvc, operationID)
+	dataUpload := newDataUpload(backup, vs, pvc, operationID, vsc)
 
 	err := crClient.Create(ctx, dataUpload)
 	if err != nil {

pkg/backup/actions/csi/pvc_action_test.go

@@ -142,26 +142,27 @@ func TestExecute(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(*testing.T) {
-			crClient := velerotest.NewFakeControllerRuntimeClient(t)
 			logger := logrus.New()
 			logger.Level = logrus.DebugLevel
-
+			objects := make([]runtime.Object, 0)
 			if tc.pvc != nil {
-				require.NoError(t, crClient.Create(context.Background(), tc.pvc))
+				objects = append(objects, tc.pvc)
 			}
 			if tc.pv != nil {
-				require.NoError(t, crClient.Create(context.Background(), tc.pv))
+				objects = append(objects, tc.pv)
 			}
 			if tc.sc != nil {
-				require.NoError(t, crClient.Create(context.Background(), tc.sc))
+				objects = append(objects, tc.sc)
 			}
 			if tc.vsClass != nil {
-				require.NoError(t, crClient.Create(context.Background(), tc.vsClass))
+				objects = append(objects, tc.vsClass)
 			}
 			if tc.resourcePolicy != nil {
-				require.NoError(t, crClient.Create(context.Background(), tc.resourcePolicy))
+				objects = append(objects, tc.resourcePolicy)
 			}
 
+			crClient := velerotest.NewFakeControllerRuntimeClient(t, objects...)
+
 			pvcBIA := pvcBackupItemAction{
 				log:      logger,
 				crClient: crClient,

pkg/backup/actions/csi/volumesnapshot_action.go

@@ -84,16 +84,15 @@ func (p *volumeSnapshotBackupItemAction) Execute(
 		return nil, nil, "", nil, errors.WithStack(err)
 	}
 
-	volumeSnapshotClassName := ""
+	additionalItems := make([]velero.ResourceIdentifier, 0)
 	if vs.Spec.VolumeSnapshotClassName != nil {
-		volumeSnapshotClassName = *vs.Spec.VolumeSnapshotClassName
-	}
-
-	additionalItems := []velero.ResourceIdentifier{
-		{
+		additionalItems = append(
+			additionalItems,
+			velero.ResourceIdentifier{
 				GroupResource: kuberesource.VolumeSnapshotClasses,
-			Name:          volumeSnapshotClassName,
+				Name:          *vs.Spec.VolumeSnapshotClassName,
 			},
+		)
 	}
 
 	p.log.Infof("Getting VolumesnapshotContent for Volumesnapshot %s/%s",

pkg/backup/actions/csi/volumesnapshot_action_test.go

@@ -86,6 +86,39 @@ func TestVSExecute(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "VS not have VSClass",
+			backup: builder.ForBackup("velero", "backup").
+				Phase(velerov1api.BackupPhaseInProgress).Result(),
+			vs: builder.ForVolumeSnapshot("velero", "vs").
+				ObjectMeta(builder.WithLabels(
+					velerov1api.BackupNameLabel, "backup")).
+				Status().
+				BoundVolumeSnapshotContentName("vsc").Result(),
+			vsc: builder.ForVolumeSnapshotContent("vsc").Status(
+				&snapshotv1api.VolumeSnapshotContentStatus{
+					SnapshotHandle: &snapshotHandle,
+				},
+			).Result(),
+			expectedErr: "",
+			expectedAdditionalItems: []velero.ResourceIdentifier{
+				{
+					GroupResource: kuberesource.VolumeSnapshotContents,
+					Name:          "vsc",
+				},
+			},
+			expectedItemToUpdate: []velero.ResourceIdentifier{
+				{
+					GroupResource: kuberesource.VolumeSnapshots,
+					Namespace:     "velero",
+					Name:          "vs",
+				},
+				{
+					GroupResource: kuberesource.VolumeSnapshotContents,
+					Name:          "vsc",
+				},
+			},
+		},
 	}
 
 	for _, tc := range tests {