Merge pull request #201 from 27149chen/do-not-change-namespace-in-plugin

do not change the namespace of pvc and vs in restore plugin directly

internal/restore/pvc_action.go

@@ -144,8 +144,10 @@ func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInp
 
 	// If cross-namespace restore is configured, change the namespace
 	// for PVC object to be restored
-	if val, ok := input.Restore.Spec.NamespaceMapping[pvc.GetNamespace()]; ok {
-		pvc.SetNamespace(val)
+	newNamespace, ok := input.Restore.Spec.NamespaceMapping[pvc.GetNamespace()]
+	if !ok {
+		// Use original namespace
+		newNamespace = pvc.Namespace
 	}
 
 	operationID := ""
@@ -181,8 +183,8 @@ func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInp
 			}
 
 			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)
+			dataDownload, err := restoreFromDataUploadResult(context.Background(), input.Restore, &pvc, newNamespace,
+				operationID, p.Client, p.VeleroClient)
 			if err != nil {
 				logger.Errorf("Fail to restore from DataUploadResult: %s", err.Error())
 				return nil, errors.WithStack(err)
@@ -197,7 +199,7 @@ func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInp
 					UpdatedItem: input.Item,
 				}, nil
 			}
-			if err := restoreFromVolumeSnapshot(&pvc, p.SnapshotClient, volumeSnapshotName, logger); err != nil {
+			if err := restoreFromVolumeSnapshot(&pvc, newNamespace, p.SnapshotClient, volumeSnapshotName, logger); err != nil {
 				logger.Errorf("Failed to restore PVC from VolumeSnapshot.")
 				return nil, errors.WithStack(err)
 			}
@@ -297,8 +299,8 @@ func (p *PVCRestoreItemAction) AreAdditionalItemsReady(additionalItems []velero.
 }
 
 func getDataUploadResult(ctx context.Context, restore *velerov1api.Restore, pvc *corev1api.PersistentVolumeClaim,
-	sourceNamespace string, kubeClient kubernetes.Interface) (*velerov2alpha1.DataUploadResult, error) {
-	labelSelector := fmt.Sprintf("%s=%s,%s=%s,%s=%s", velerov1api.PVCNamespaceNameLabel, label.GetValidName(sourceNamespace+"."+pvc.Name),
+	kubeClient kubernetes.Interface) (*velerov2alpha1.DataUploadResult, error) {
+	labelSelector := fmt.Sprintf("%s=%s,%s=%s,%s=%s", velerov1api.PVCNamespaceNameLabel, label.GetValidName(pvc.Namespace+"."+pvc.Name),
 		velerov1api.RestoreUIDLabel, label.GetValidName(string(restore.UID)),
 		velerov1api.ResourceUsageLabel, label.GetValidName(string(velerov1api.VeleroResourceUsageDataUploadResult)),
 	)
@@ -374,7 +376,7 @@ func cancelDataDownload(ctx context.Context, veleroClient veleroClientSet.Interf
 }
 
 func newDataDownload(restore *velerov1api.Restore, dataUploadResult *velerov2alpha1.DataUploadResult,
-	pvc *corev1api.PersistentVolumeClaim, operationID string) *velerov2alpha1.DataDownload {
+	pvc *corev1api.PersistentVolumeClaim, newNamespace, operationID string) *velerov2alpha1.DataDownload {
 	dataDownload := &velerov2alpha1.DataDownload{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: velerov2alpha1.SchemeGroupVersion.String(),
@@ -401,7 +403,7 @@ func newDataDownload(restore *velerov1api.Restore, dataUploadResult *velerov2alp
 		Spec: velerov2alpha1.DataDownloadSpec{
 			TargetVolume: velerov2alpha1.TargetVolumeSpec{
 				PVC:       pvc.Name,
-				Namespace: pvc.Namespace,
+				Namespace: newNamespace,
 			},
 			BackupStorageLocation: dataUploadResult.BackupStorageLocation,
 			DataMover:             dataUploadResult.DataMover,
@@ -414,11 +416,11 @@ func newDataDownload(restore *velerov1api.Restore, dataUploadResult *velerov2alp
 	return dataDownload
 }
 
-func restoreFromVolumeSnapshot(pvc *corev1api.PersistentVolumeClaim, snapClient snapshotterClientSet.Interface,
+func restoreFromVolumeSnapshot(pvc *corev1api.PersistentVolumeClaim, newNamespace string, snapClient snapshotterClientSet.Interface,
 	volumeSnapshotName string, logger logrus.FieldLogger) error {
-	vs, err := snapClient.SnapshotV1().VolumeSnapshots(pvc.Namespace).Get(context.TODO(), volumeSnapshotName, metav1.GetOptions{})
+	vs, err := snapClient.SnapshotV1().VolumeSnapshots(newNamespace).Get(context.TODO(), volumeSnapshotName, metav1.GetOptions{})
 	if err != nil {
-		return errors.Wrapf(err, fmt.Sprintf("Failed to get Volumesnapshot %s/%s to restore PVC %s/%s", pvc.Namespace, volumeSnapshotName, pvc.Namespace, pvc.Name))
+		return errors.Wrapf(err, fmt.Sprintf("Failed to get Volumesnapshot %s/%s to restore PVC %s/%s", newNamespace, volumeSnapshotName, newNamespace, pvc.Name))
 	}
 
 	if _, exists := vs.Annotations[util.VolumeSnapshotRestoreSize]; exists {
@@ -442,8 +444,8 @@ func restoreFromVolumeSnapshot(pvc *corev1api.PersistentVolumeClaim, snapClient
 }
 
 func restoreFromDataUploadResult(ctx context.Context, restore *velerov1api.Restore, pvc *corev1api.PersistentVolumeClaim,
-	operationID string, sourceNamespace string, kubeClient kubernetes.Interface, veleroClient veleroClientSet.Interface) (*velerov2alpha1.DataDownload, error) {
-	dataUploadResult, err := getDataUploadResult(ctx, restore, pvc, sourceNamespace, kubeClient)
+	newNamespace, operationID string, kubeClient kubernetes.Interface, veleroClient veleroClientSet.Interface) (*velerov2alpha1.DataDownload, error) {
+	dataUploadResult, err := getDataUploadResult(ctx, restore, pvc, kubeClient)
 	if err != nil {
 		return nil, errors.Wrapf(err, "fail get DataUploadResult for restore: %s", restore.Name)
 	}
@@ -454,9 +456,9 @@ func restoreFromDataUploadResult(ctx context.Context, restore *velerov1api.Resto
 	if pvc.Spec.Selector.MatchLabels == nil {
 		pvc.Spec.Selector.MatchLabels = make(map[string]string)
 	}
-	pvc.Spec.Selector.MatchLabels[util.DynamicPVRestoreLabel] = label.GetValidName(fmt.Sprintf("%s.%s.%s", pvc.Namespace, pvc.Name, utilrand.String(GenerateNameRandomLength)))
+	pvc.Spec.Selector.MatchLabels[util.DynamicPVRestoreLabel] = label.GetValidName(fmt.Sprintf("%s.%s.%s", newNamespace, pvc.Name, utilrand.String(GenerateNameRandomLength)))
 
-	dataDownload := newDataDownload(restore, dataUploadResult, pvc, operationID)
+	dataDownload := newDataDownload(restore, dataUploadResult, pvc, newNamespace, operationID)
 	_, err = veleroClient.VeleroV2alpha1().DataDownloads(restore.Namespace).Create(ctx, dataDownload, metav1.CreateOptions{})
 	if err != nil {
 		return nil, errors.Wrapf(err, "fail to create DataDownload")

internal/restore/volumesnapshot_action.go

@@ -76,16 +76,18 @@ func (p *VolumeSnapshotRestoreItemAction) Execute(input *velero.RestoreItemActio
 
 	// If cross-namespace restore is configured, change the namespace
 	// for VolumeSnapshot object to be restored
-	if val, ok := input.Restore.Spec.NamespaceMapping[vs.GetNamespace()]; ok {
-		vs.SetNamespace(val)
+	newNamespace, ok := input.Restore.Spec.NamespaceMapping[vs.GetNamespace()]
+	if !ok {
+		// Use original namespace
+		newNamespace = vs.Namespace
 	}
 
 	_, snapClient, err := util.GetClients()
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
 
-	if !util.IsVolumeSnapshotExists(&vs, snapClient.SnapshotV1()) {
+	if !util.IsVolumeSnapshotExists(newNamespace, vs.Name, snapClient.SnapshotV1()) {
 		snapHandle, exists := vs.Annotations[util.VolumeSnapshotHandleAnnotation]
 		if !exists {
 			return nil, errors.Errorf("Volumesnapshot %s/%s does not have a %s annotation", vs.Namespace, vs.Name, util.VolumeSnapshotHandleAnnotation)
@@ -97,7 +99,7 @@ func (p *VolumeSnapshotRestoreItemAction) Execute(input *velero.RestoreItemActio
 		}
 
 		p.Log.Debugf("Set VolumeSnapshotContent %s/%s DeletionPolicy to Retain to make sure VS deletion in namespace will not delete Snapshot on cloud provider.",
-			vs.Namespace, vs.Name)
+			newNamespace, vs.Name)
 
 		// TODO: generated name will be like velero-velero-something. Fix that.
 		vsc := snapshotv1api.VolumeSnapshotContent{
@@ -112,7 +114,7 @@ func (p *VolumeSnapshotRestoreItemAction) Execute(input *velero.RestoreItemActio
 				Driver:         csiDriverName,
 				VolumeSnapshotRef: core_v1.ObjectReference{
 					Kind:      util.VolumeSnapshotKindName,
-					Namespace: vs.Namespace,
+					Namespace: newNamespace,
 					Name:      vs.Name,
 				},
 				Source: snapshotv1api.VolumeSnapshotContentSource{
@@ -131,7 +133,7 @@ func (p *VolumeSnapshotRestoreItemAction) Execute(input *velero.RestoreItemActio
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to create volumesnapshotcontents %s", vsc.GenerateName)
 		}
-		p.Log.Infof("Created VolumesnapshotContents %s with static binding to volumesnapshot %s/%s", vscupd, vs.Namespace, vs.Name)
+		p.Log.Infof("Created VolumesnapshotContents %s with static binding to volumesnapshot %s/%s", vscupd, newNamespace, vs.Name)
 
 		// Reset Spec to convert the volumesnapshot from using the dyanamic volumesnapshotcontent to the static one.
 		resetVolumeSnapshotSpecForRestore(&vs, &vscupd.Name)

internal/util/util.go

@@ -358,16 +358,13 @@ func AddLabels(o *metav1.ObjectMeta, vals map[string]string) {
 }
 
 // IsVolumeSnapshotExists returns whether a specific volumesnapshot object exists.
-func IsVolumeSnapshotExists(volSnap *snapshotv1api.VolumeSnapshot, snapshotClient snapshotter.SnapshotV1Interface) bool {
-	exists := false
-	if volSnap != nil {
-		vs, err := snapshotClient.VolumeSnapshots(volSnap.Namespace).Get(context.TODO(), volSnap.Name, metav1.GetOptions{})
-		if err == nil && vs != nil {
-			exists = true
-		}
+func IsVolumeSnapshotExists(ns, name string, snapshotClient snapshotter.SnapshotV1Interface) bool {
+	vs, err := snapshotClient.VolumeSnapshots(ns).Get(context.TODO(), name, metav1.GetOptions{})
+	if err == nil && vs != nil {
+		return true
 	}
 
-	return exists
+	return false
 }
 
 func SetVolumeSnapshotContentDeletionPolicy(vscName string, csiClient snapshotter.SnapshotV1Interface) error {

internal/util/util_test.go

@@ -1560,16 +1560,11 @@ func TestIsVolumeSnapshotExists(t *testing.T) {
 			expected: false,
 			vs:       vsNotExists,
 		},
-		{
-			name:     "should not find a nil VolumeSnapshot object",
-			expected: false,
-			vs:       nil,
-		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			actual := IsVolumeSnapshotExists(tc.vs, fakeClient.SnapshotV1())
+			actual := IsVolumeSnapshotExists(tc.vs.Namespace, tc.vs.Name, fakeClient.SnapshotV1())
 			assert.Equal(t, tc.expected, actual)
 		})
 	}