Merge pull request #194 from blackpiglet/modify_vs_vsc_bia

Fix #6589

internal/backup/pvc_action.go

@@ -99,6 +99,7 @@ func (p *PVCBackupItemAction) Execute(item runtime.Unstructured, backup *velerov
 	}
 	if pv.Spec.PersistentVolumeSource.CSI == nil {
 		p.Log.Infof("Skipping PVC %s/%s, associated PV %s is not a CSI volume", pvc.Namespace, pvc.Name, pv.Name)
+
 		util.AddAnnotations(&pvc.ObjectMeta, map[string]string{
 			util.SkippedNoCSIPVAnnotation: "true",
 		})
@@ -168,9 +169,6 @@ func (p *PVCBackupItemAction) Execute(item runtime.Unstructured, backup *velerov
 	annotations := labels
 	annotations[util.MustIncludeAdditionalItemAnnotation] = "true"
 
-	util.AddAnnotations(&pvc.ObjectMeta, annotations)
-	util.AddLabels(&pvc.ObjectMeta, labels)
-
 	var additionalItems []velero.ResourceIdentifier
 	operationID := ""
 	var itemToUpdate []velero.ResourceIdentifier
@@ -203,6 +201,9 @@ func (p *PVCBackupItemAction) Execute(item runtime.Unstructured, backup *velerov
 					Name:      dataUpload.Name,
 				},
 			}
+			// Set the DataUploadNameLabel, which is used for restore to let CSI plugin check whether
+			// it should handle the volume. If volume is CSI migration, PVC doesn't have the annotation.
+			annotations[util.DataUploadNameAnnotation] = dataUpload.Namespace + "/" + dataUpload.Name
 
 			dataUploadLog.Info("DataUpload is submitted successfully.")
 		}
@@ -216,6 +217,9 @@ func (p *PVCBackupItemAction) Execute(item runtime.Unstructured, backup *velerov
 		}
 	}
 
+	util.AddAnnotations(&pvc.ObjectMeta, annotations)
+	util.AddLabels(&pvc.ObjectMeta, labels)
+
 	p.Log.Infof("Returning from PVCBackupItemAction with %d additionalItems to backup", len(additionalItems))
 	for _, ai := range additionalItems {
 		p.Log.Debugf("%s: %s", ai.GroupResource.String(), ai.Name)

internal/backup/pvc_action_test.go

@@ -56,6 +56,7 @@ func TestExecute(t *testing.T) {
 		expectedErr        error
 		expectedBackup     *velerov1api.Backup
 		expectedDataUpload *velerov2alpha1.DataUpload
+		expectedPVC        *corev1.PersistentVolumeClaim
 	}{
 		{
 			name:        "Skip PVC handling if SnapshotVolume set to false",
@@ -113,6 +114,21 @@ func TestExecute(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:        "",
+			backup:      builder.ForBackup("velero", "test").SnapshotMoveData(true).Result(),
+			pvc:         builder.ForPersistentVolumeClaim("velero", "testPVC").VolumeName("testPV").StorageClass("testSC").Phase(corev1.ClaimBound).Result(),
+			pv:          builder.ForPersistentVolume("testPV").CSI("hostpath", "testVolume").Result(),
+			sc:          builder.ForStorageClass("testSC").Provisioner("hostpath").Result(),
+			vs:          builder.ForVolumeSnapshot("velero", "testVS").Result(),
+			vsClass:     builder.ForVolumeSnapshotClass("tescVSClass").Driver("hostpath").ObjectMeta(builder.WithLabels(util.VolumeSnapshotClassSelectorLabel, "")).Result(),
+			operationID: ".",
+			expectedErr: nil,
+			expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").
+				ObjectMeta(builder.WithAnnotations(util.MustIncludeAdditionalItemAnnotation, "true", velerov1api.BackupNameLabel, "test", util.DataUploadNameAnnotation, "velero/", util.VolumeSnapshotLabel, ""),
+					builder.WithLabels(util.MustIncludeAdditionalItemAnnotation, "true", velerov1api.BackupNameLabel, "test", util.DataUploadNameAnnotation, "velero/", util.VolumeSnapshotLabel, "")).
+				VolumeName("testPV").StorageClass("testSC").Phase(corev1.ClaimBound).Result(),
+		},
 	}
 
 	for _, tc := range tests {
@@ -150,7 +166,7 @@ func TestExecute(t *testing.T) {
 			pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.pvc)
 			require.NoError(t, err)
 
-			_, _, _, _, err = pvcBIA.Execute(&unstructured.Unstructured{Object: pvcMap}, tc.backup)
+			resultUnstructed, _, _, _, err := pvcBIA.Execute(&unstructured.Unstructured{Object: pvcMap}, tc.backup)
 			if tc.expectedErr != nil {
 				require.Equal(t, err, tc.expectedErr)
 			}
@@ -161,6 +177,12 @@ func TestExecute(t *testing.T) {
 				require.Equal(t, 1, len(dataUploadList.Items))
 				require.Equal(t, *tc.expectedDataUpload, dataUploadList.Items[0])
 			}
+
+			if tc.expectedPVC != nil {
+				resultPVC := new(corev1.PersistentVolumeClaim)
+				runtime.DefaultUnstructuredConverter.FromUnstructured(resultUnstructed.UnstructuredContent(), resultPVC)
+				require.Equal(t, *tc.expectedPVC, *resultPVC)
+			}
 		})
 	}
 }

internal/backup/volumesnapshot_action.go

@@ -184,14 +184,20 @@ func (p *VolumeSnapshotBackupItemAction) Execute(item runtime.Unstructured, back
 		p.Log.Debugf("%s: %s", ai.GroupResource.String(), ai.Name)
 	}
 
-	// The operationID is of the form <namespace>/<volumesnapshot-name>/<started-time>
-	operationID := vs.Namespace + "/" + vs.Name + "/" + time.Now().Format(time.RFC3339)
-	itemToUpdate := []velero.ResourceIdentifier{
-		{
-			GroupResource: kuberesource.VolumeSnapshots,
-			Namespace:     vs.Namespace,
-			Name:          vs.Name,
-		},
+	operationID := ""
+	var itemToUpdate []velero.ResourceIdentifier
+
+	// Only return Async operation for VSC created for this backup.
+	if backupOngoing {
+		// The operationID is of the form <namespace>/<volumesnapshot-name>/<started-time>
+		operationID = vs.Namespace + "/" + vs.Name + "/" + time.Now().Format(time.RFC3339)
+		itemToUpdate = []velero.ResourceIdentifier{
+			{
+				GroupResource: kuberesource.VolumeSnapshots,
+				Namespace:     vs.Namespace,
+				Name:          vs.Name,
+			},
+		}
 	}
 
 	return &unstructured.Unstructured{Object: vsMap}, additionalItems, operationID, itemToUpdate, nil

internal/backup/volumesnapshotcontent_action.go

@@ -33,6 +33,7 @@ import (
 	"github.com/vmware-tanzu/velero-plugin-for-csi/internal/util"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
+	"github.com/vmware-tanzu/velero/pkg/label"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	biav2 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v2"
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
@@ -90,13 +91,21 @@ func (p *VolumeSnapshotContentBackupItemAction) Execute(item runtime.Unstructure
 		return nil, nil, "", nil, errors.WithStack(err)
 	}
 
-	// The operationID is of the form <volumesnapshotcontent-name>/<started-time>
-	operationID := snapCont.Name + "/" + time.Now().Format(time.RFC3339)
-	itemToUpdate := []velero.ResourceIdentifier{
-		{
-			GroupResource: kuberesource.VolumeSnapshotContents,
-			Name:          snapCont.Name,
-		},
+	backupOnGoing := snapCont.GetLabels()[velerov1api.BackupNameLabel] == label.GetValidName(backup.Name)
+	operationID := ""
+	var itemToUpdate []velero.ResourceIdentifier
+
+	// Only return Async operation for VSC created for this backup.
+	if backupOnGoing {
+		// The operationID is of the form <volumesnapshotcontent-name>/<started-time>
+		operationID = snapCont.Name + "/" + time.Now().Format(time.RFC3339)
+		itemToUpdate = []velero.ResourceIdentifier{
+			{
+				GroupResource: kuberesource.VolumeSnapshotContents,
+				Name:          snapCont.Name,
+			},
+		}
+
 	}
 
 	p.Log.Infof("Returning from VolumeSnapshotContentBackupItemAction with %d additionalItems to backup", len(additionalItems))

internal/restore/pvc_action.go

@@ -143,7 +143,8 @@ func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInp
 	operationID := ""
 
 	// remove the volumesnapshot name annotation as well
-	removePVCAnnotations(&pvc, []string{util.VolumeSnapshotLabel})
+	// clean the DataUploadNameLabel for snapshot data mover case.
+	removePVCAnnotations(&pvc, []string{util.VolumeSnapshotLabel, util.DataUploadNameAnnotation})
 
 	if boolptr.IsSetToFalse(input.Restore.Spec.RestorePVs) {
 		logger.Info("Restore did not request for PVs to be restored from snapshot")
@@ -161,6 +162,16 @@ func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInp
 		if boolptr.IsSetToTrue(backup.Spec.SnapshotMoveData) {
 			logger.Info("Start DataMover restore.")
 
+			// If PVC doesn't have a DataUploadNameLabel, which should be created
+			// during backup, then CSI cannot handle the volume during to restore,
+			// so return early to let Velero tries to fall back to Velero native snapshot.
+			if _, ok := pvcFromBackup.Annotations[util.DataUploadNameAnnotation]; !ok {
+				logger.Warnf("PVC doesn't have a DataUpload for data mover. Return.")
+				return &velero.RestoreItemActionExecuteOutput{
+					UpdatedItem: input.Item,
+				}, nil
+			}
+
 			operationID = label.GetValidName(string(velerov1api.AsyncOperationIDPrefixDataDownload) + string(input.Restore.UID) + "." + string(pvcFromBackup.UID))
 			dataDownload, err := restoreFromDataUploadResult(context.Background(), input.Restore, &pvc,
 				operationID, pvcFromBackup.Namespace, p.Client, p.VeleroClient)

internal/restore/pvc_action_test.go

@@ -578,15 +578,15 @@ func TestExecute(t *testing.T) {
 			name:        "DataUploadResult cannot be found",
 			backup:      builder.ForBackup("velero", "testBackup").SnapshotMoveData(true).Result(),
 			restore:     builder.ForRestore("velero", "testRestore").Backup("testBackup").Result(),
-			pvc:         builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(util.VolumeSnapshotRestoreSize, "10Gi")).Result(),
+			pvc:         builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(util.VolumeSnapshotRestoreSize, "10Gi", util.DataUploadNameAnnotation, "velero/")).Result(),
 			expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").Result(),
 			expectedErr: "fail get DataUploadResult for restore: testRestore: no DataUpload result cm found with labels velero.io/pvc-namespace-name=velero.testPVC,velero.io/restore-uid=,velero.io/resource-usage=DataUpload",
 		},
 		{
 			name:             "Restore from DataUploadResult",
 			backup:           builder.ForBackup("velero", "testBackup").SnapshotMoveData(true).Result(),
 			restore:          builder.ForRestore("velero", "testRestore").Backup("testBackup").ObjectMeta(builder.WithUID("uid")).Result(),
-			pvc:              builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(util.VolumeSnapshotRestoreSize, "10Gi")).Result(),
+			pvc:              builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(util.VolumeSnapshotRestoreSize, "10Gi", util.DataUploadNameAnnotation, "velero/")).Result(),
 			dataUploadResult: builder.ForConfigMap("velero", "testCM").Data("uid", "{}").ObjectMeta(builder.WithLabels(velerov1api.RestoreUIDLabel, "uid", velerov1api.PVCNamespaceNameLabel, "velero.testPVC", velerov1api.ResourceUsageLabel, label.GetValidName(string(velerov1api.VeleroResourceUsageDataUploadResult)))).Result(),
 			expectedPVC:      builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations("velero.io/vsi-volumesnapshot-restore-size", "10Gi")).Result(),
 			expectedDataDownload: builder.ForDataDownload("velero", "").TargetVolume(velerov2alpha1.TargetVolumeSpec{PVC: "testPVC", Namespace: "velero"}).
@@ -598,10 +598,16 @@ func TestExecute(t *testing.T) {
 			name:             "Restore from DataUploadResult with long source PVC namespace and name",
 			backup:           builder.ForBackup("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "testBackup").SnapshotMoveData(true).Result(),
 			restore:          builder.ForRestore("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "testRestore").Backup("testBackup").ObjectMeta(builder.WithUID("uid")).Result(),
-			pvc:              builder.ForPersistentVolumeClaim("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "kibishii-data-kibishii-deployment-0").ObjectMeta(builder.WithAnnotations(util.VolumeSnapshotRestoreSize, "10Gi")).Result(),
+			pvc:              builder.ForPersistentVolumeClaim("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "kibishii-data-kibishii-deployment-0").ObjectMeta(builder.WithAnnotations(util.VolumeSnapshotRestoreSize, "10Gi", util.DataUploadNameAnnotation, "velero/")).Result(),
 			dataUploadResult: builder.ForConfigMap("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "testCM").Data("uid", "{}").ObjectMeta(builder.WithLabels(velerov1api.RestoreUIDLabel, "uid", velerov1api.PVCNamespaceNameLabel, "migre209d0da-49c7-45ba-8d5a-3e59fd591ec1.kibishii-data-ki152333", velerov1api.ResourceUsageLabel, label.GetValidName(string(velerov1api.VeleroResourceUsageDataUploadResult)))).Result(),
 			expectedPVC:      builder.ForPersistentVolumeClaim("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "kibishii-data-kibishii-deployment-0").ObjectMeta(builder.WithAnnotations("velero.io/vsi-volumesnapshot-restore-size", "10Gi")).Result(),
 		},
+		{
+			name:    "PVC had no DataUploadNameLabel annotation",
+			backup:  builder.ForBackup("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "testBackup").SnapshotMoveData(true).Result(),
+			restore: builder.ForRestore("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "testRestore").Backup("testBackup").ObjectMeta(builder.WithUID("uid")).Result(),
+			pvc:     builder.ForPersistentVolumeClaim("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1", "kibishii-data-kibishii-deployment-0").ObjectMeta(builder.WithAnnotations(util.VolumeSnapshotRestoreSize, "10Gi")).Result(),
+		},
 	}
 
 	for _, tc := range tests {

internal/util/labels_annotations.go

@@ -49,4 +49,7 @@ const (
 
 	// DynamicPVRestoreLabel is the label key for dynamic PV restore
 	DynamicPVRestoreLabel = "velero.io/dynamic-pv-restore"
+
+	// DataUploadNameAnnotation is the label key for the DataUpload name
+	DataUploadNameAnnotation = "velero.io/data-upload-name"
 )