Remove dependency with VolumeSnapshotClass in DataUpload.

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

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

Author: blackpiglet

changelogs/unreleased/9040-blackpiglet

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

internal/volume/volumes_information.go

@@ -557,6 +557,51 @@ func (v *BackupVolumesInformation) getVolumeSnapshotClasses() (
 	return vsClassList.Items, nil
 }
 
+func (v *BackupVolumesInformation) getDriverFromVolumeSnapshot(
+	vsName string,
+	namespace string,
+) (string, error) {
+	if vsName == "" {
+		v.logger.Debugf("vsName is empty. Return early.")
+		return "", nil
+	}
+
+	vs := new(snapshotv1api.VolumeSnapshot)
+	if err := v.crClient.Get(
+		context.TODO(),
+		kbclient.ObjectKey{Namespace: namespace, Name: vsName},
+		vs,
+	); err != nil {
+		v.logger.Infof("Fail to get VolumeSnapshot %s: %s", namespace+"/"+vsName, err.Error())
+		return "", errors.Errorf("fail to get VolumeSnapshot %s: %s",
+			namespace+"/"+vsName, err.Error())
+	}
+
+	if vs.Status == nil || vs.Status.BoundVolumeSnapshotContentName == nil {
+		v.logger.Infof("VolumeSnapshot %s doesn't have a bound VolumeSnapshotContent",
+			namespace+"/"+vsName)
+		return "", errors.Errorf("volumeSnapshot %s doesn't have a bound VolumeSnapshotContent", namespace+"/"+vsName)
+	}
+
+	vsc := new(snapshotv1api.VolumeSnapshotContent)
+	if err := v.crClient.Get(
+		context.TODO(),
+		kbclient.ObjectKey{Name: *vs.Status.BoundVolumeSnapshotContentName},
+		vsc,
+	); err != nil {
+		v.logger.Infof("Fail to get VolumeSnapshotContent %s: %s",
+			*vs.Status.BoundVolumeSnapshotContentName, err.Error())
+		return "",
+			errors.Errorf("fail to get VolumeSnapshotContent %s: %s",
+				*vs.Status.BoundVolumeSnapshotContentName,
+				err.Error(),
+			)
+	}
+
+	v.logger.Debugf("Found driver %s for VolumeSnapshot %s.", vsc.Spec.Driver, namespace+"/"+vsName)
+	return vsc.Spec.Driver, nil
+}
+
 // generateVolumeInfoFromDataUpload generate BackupVolumeInfo for DataUpload.
 func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
 	if !features.IsEnabled(velerov1api.CSIFeatureFlag) {
@@ -581,14 +626,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,11 +647,16 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
 			continue
 		}
 
-		driverUsedByVSClass := ""
-		for index := range vsClassList {
-			if vsClassList[index].Name == dataUpload.Spec.CSISnapshot.SnapshotClass {
-				driverUsedByVSClass = vsClassList[index].Driver
-			}
+		driver, driverErr := v.getDriverFromVolumeSnapshot(
+			dataUpload.Spec.CSISnapshot.VolumeSnapshot,
+			dataUpload.Namespace,
+		)
+		if driverErr != nil {
+			v.logger.Warnf("Fail to get driver for VolumeSnapshot %s: %s",
+				dataUpload.Namespace+"/"+dataUpload.Spec.CSISnapshot.VolumeSnapshot,
+				driverErr.Error(),
+			)
+			continue
 		}
 
 		if pvcPVInfo := v.pvMap.retrieve(
@@ -637,7 +680,7 @@ func (v *BackupVolumesInformation) generateVolumeInfoFromDataUpload() {
 					SnapshotHandle: FieldValueIsUnknown,
 					VSCName:        FieldValueIsUnknown,
 					OperationID:    FieldValueIsUnknown,
-					Driver:         driverUsedByVSClass,
+					Driver:         driver,
 				},
 				SnapshotDataMovementInfo: &SnapshotDataMovementInfo{
 					DataMover:    dataMover,

internal/volume/volumes_information_test.go

@@ -21,6 +21,7 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 
 	snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumesnapshot/v1"
@@ -29,6 +30,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"
@@ -1327,3 +1329,73 @@ func TestGetVolumeSnapshotClasses(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, []snapshotv1api.VolumeSnapshotClass{*class}, result)
 }
+
+func TestGetDriverFromVolumeSnapshot(t *testing.T) {
+	tests := []struct {
+		name           string
+		vsName         string
+		vs             *snapshotv1api.VolumeSnapshot
+		vsc            *snapshotv1api.VolumeSnapshotContent
+		expectedErr    error
+		expectedDriver string
+	}{
+		{
+			name:           "vsName is empty",
+			vsName:         "",
+			expectedErr:    nil,
+			expectedDriver: "",
+		},
+		{
+			name:           "vs is not created",
+			vsName:         "vs-01",
+			expectedErr:    errors.Errorf("fail to get VolumeSnapshot %s: %s", velerov1api.DefaultNamespace+"/"+"vs-01", "volumesnapshots.snapshot.storage.k8s.io \"vs-01\" not found"),
+			expectedDriver: "",
+		},
+		{
+			name:           "vs status is nil",
+			vsName:         "vs-01",
+			vs:             builder.ForVolumeSnapshot(velerov1api.DefaultNamespace, "vs-01").Result(),
+			expectedErr:    errors.Errorf("volumeSnapshot %s doesn't have a bound VolumeSnapshotContent", velerov1api.DefaultNamespace+"/"+"vs-01"),
+			expectedDriver: "",
+		},
+		{
+			name:           "vsc is not created",
+			vsName:         "vs-01",
+			vs:             builder.ForVolumeSnapshot(velerov1api.DefaultNamespace, "vs-01").Status().BoundVolumeSnapshotContentName("vsc-01").Result(),
+			expectedErr:    errors.Errorf("fail to get VolumeSnapshotContent %s: %s", "vsc-01", "volumesnapshotcontents.snapshot.storage.k8s.io \"vsc-01\" not found"),
+			expectedDriver: "",
+		},
+		{
+			name:           "success case",
+			vsName:         "vs-01",
+			vs:             builder.ForVolumeSnapshot(velerov1api.DefaultNamespace, "vs-01").Status().BoundVolumeSnapshotContentName("vsc-01").Result(),
+			vsc:            builder.ForVolumeSnapshotContent("vsc-01").Driver("csi.storage.vsphere.com").Result(),
+			expectedErr:    nil,
+			expectedDriver: "csi.storage.vsphere.com",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			volumesInfo := BackupVolumesInformation{}
+			volumesInfo.Init()
+			volumesInfo.logger = velerotest.NewLogger()
+
+			objects := []runtime.Object{}
+			if tc.vs != nil {
+				objects = append(objects, tc.vs)
+			}
+			if tc.vsc != nil {
+				objects = append(objects, tc.vsc)
+			}
+
+			volumesInfo.crClient = velerotest.NewFakeControllerRuntimeClient(t, objects...)
+
+			driver, err := volumesInfo.getDriverFromVolumeSnapshot(tc.vsName, velerov1api.DefaultNamespace)
+			if err != nil {
+				assert.ErrorAs(t, tc.expectedErr, &err)
+			}
+			assert.Equal(t, tc.expectedDriver, driver)
+		})
+	}
+}

pkg/apis/velero/v2alpha1/data_upload_types.go

@@ -78,7 +78,7 @@ type CSISnapshotSpec struct {
 
 	// SnapshotClass is the name of the snapshot class that the volume snapshot is created with
 	// +optional
-	SnapshotClass string `json:"snapshotClass"`
+	SnapshotClass string `json:"snapshotClass,omitempty"`
 }
 
 // DataUploadPhase represents the lifecycle phase of a DataUpload.

pkg/backup/actions/csi/pvc_action.go

@@ -471,7 +471,6 @@ func newDataUpload(
 			CSISnapshot: &velerov2alpha1.CSISnapshotSpec{
 				VolumeSnapshot: vs.Name,
 				StorageClass:   *pvc.Spec.StorageClassName,
-				SnapshotClass:  *vs.Spec.VolumeSnapshotClassName,
 			},
 			SourcePVC:             pvc.Name,
 			DataMover:             backup.Spec.DataMover,

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{
-		{
-			GroupResource: kuberesource.VolumeSnapshotClasses,
-			Name:          volumeSnapshotClassName,
-		},
+		additionalItems = append(
+			additionalItems,
+			velero.ResourceIdentifier{
+				GroupResource: kuberesource.VolumeSnapshotClasses,
+				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 {