From 830190c1652ec185a21b30a06ec6ebe707638a7c Mon Sep 17 00:00:00 2001 From: lou Date: Wed, 13 Sep 2023 21:19:06 +0800 Subject: [PATCH] do not change the namespace of pvc and vs in restore plugin directly Signed-off-by: lou --- internal/restore/pvc_action.go | 34 ++++++++++++----------- internal/restore/volumesnapshot_action.go | 14 ++++++---- internal/util/util.go | 13 ++++----- internal/util/util_test.go | 7 +---- 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/internal/restore/pvc_action.go b/internal/restore/pvc_action.go index 68e508e2..1211b1c9 100644 --- a/internal/restore/pvc_action.go +++ b/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") diff --git a/internal/restore/volumesnapshot_action.go b/internal/restore/volumesnapshot_action.go index 3c0adedc..a3702756 100644 --- a/internal/restore/volumesnapshot_action.go +++ b/internal/restore/volumesnapshot_action.go @@ -76,8 +76,10 @@ 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() @@ -85,7 +87,7 @@ func (p *VolumeSnapshotRestoreItemAction) Execute(input *velero.RestoreItemActio 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) diff --git a/internal/util/util.go b/internal/util/util.go index 94c68cf7..d75562db 100644 --- a/internal/util/util.go +++ b/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 { diff --git a/internal/util/util_test.go b/internal/util/util_test.go index b381564f..aa65d550 100644 --- a/internal/util/util_test.go +++ b/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) }) }