diff --git a/changelogs/unreleased/3708-sseago b/changelogs/unreleased/3708-sseago new file mode 100644 index 000000000..96c9f3cef --- /dev/null +++ b/changelogs/unreleased/3708-sseago @@ -0,0 +1 @@ +Handle namespace mapping for PVs without snapshots on restore diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index c874dd7c7..6e55a2a34 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -1102,6 +1102,12 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso default: ctx.log.Infof("Restoring persistent volume as-is because it doesn't have a snapshot and its reclaim policy is not Delete.") + // Check to see if the claimRef.namespace field needs to be remapped, and do so if necessary. + _, err = remapClaimRefNS(ctx, obj) + if err != nil { + errs.Add(namespace, err) + return warnings, errs + } obj = resetVolumeBindingInfo(obj) // We call the pvRestorer here to clear out the PV's claimRef.UID, // so it can be re-claimed when its PVC is restored and gets a new UID. diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 274397862..04fc2f5db 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -1811,6 +1811,8 @@ func TestRestorePersistentVolumes(t *testing.T) { volumeSnapshotLocations []*velerov1api.VolumeSnapshotLocation volumeSnapshotterGetter volumeSnapshotterGetter want []*test.APIResource + wantError bool + wantWarning bool }{ { name: "when a PV with a reclaim policy of delete has no snapshot and does not exist in-cluster, it does not get restored, and its PVC gets reset for dynamic provisioning", @@ -2191,6 +2193,95 @@ func TestRestorePersistentVolumes(t *testing.T) { ), }, }, + { + name: "when a PV without a snapshot is used by a PVC in a namespace that's being remapped, and the original PV exists in-cluster, the PV is not replaced and there is a restore warning", + restore: defaultRestore().NamespaceMappings("source-ns", "target-ns").Result(), + backup: defaultBackup().Result(), + tarball: test.NewTarWriter(t). + AddItems( + "persistentvolumes", + builder.ForPersistentVolume("source-pv"). + //ReclaimPolicy(corev1api.PersistentVolumeReclaimRetain). + AWSEBSVolumeID("source-volume"). + ClaimRef("source-ns", "pvc-1"). + Result(), + ). + AddItems( + "persistentvolumeclaims", + builder.ForPersistentVolumeClaim("source-ns", "pvc-1").VolumeName("source-pv").Result(), + ). + Done(), + apiResources: []*test.APIResource{ + test.PVs( + builder.ForPersistentVolume("source-pv"). + //ReclaimPolicy(corev1api.PersistentVolumeReclaimRetain). + AWSEBSVolumeID("source-volume"). + ClaimRef("source-ns", "pvc-1"). + Result(), + ), + test.PVCs(), + }, + want: []*test.APIResource{ + test.PVs( + builder.ForPersistentVolume("source-pv"). + AWSEBSVolumeID("source-volume"). + ClaimRef("source-ns", "pvc-1"). + Result(), + ), + test.PVCs( + builder.ForPersistentVolumeClaim("target-ns", "pvc-1"). + ObjectMeta( + builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), + ). + VolumeName("source-pv"). + Result(), + ), + }, + wantWarning: true, + }, + { + name: "when a PV without a snapshot is used by a PVC in a namespace that's being remapped, and the original PV does not exist in-cluster, the PV is not renamed", + restore: defaultRestore().NamespaceMappings("source-ns", "target-ns").Result(), + backup: defaultBackup().Result(), + tarball: test.NewTarWriter(t). + AddItems( + "persistentvolumes", + builder.ForPersistentVolume("source-pv"). + AWSEBSVolumeID("source-volume"). + ClaimRef("source-ns", "pvc-1"). + Result(), + ). + AddItems( + "persistentvolumeclaims", + builder.ForPersistentVolumeClaim("source-ns", "pvc-1").VolumeName("source-pv").Result(), + ). + Done(), + apiResources: []*test.APIResource{ + test.PVs(), + test.PVCs(), + }, + want: []*test.APIResource{ + test.PVs( + builder.ForPersistentVolume("source-pv"). + //ReclaimPolicy(corev1api.PersistentVolumeReclaimRetain). + ObjectMeta( + builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), + ). + // the namespace for this PV's claimRef should be the one that the PVC was remapped into. + ClaimRef("target-ns", "pvc-1"). + AWSEBSVolumeID("source-volume"). + Result(), + ), + test.PVCs( + builder.ForPersistentVolumeClaim("target-ns", "pvc-1"). + ObjectMeta( + builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), + ). + VolumeName("source-pv"). + Result(), + ), + }, + }, { name: "when a PV is renamed and the original PV does not exist in-cluster, the PV should be renamed", restore: defaultRestore().NamespaceMappings("source-ns", "target-ns").Result(), @@ -2423,7 +2514,16 @@ func TestRestorePersistentVolumes(t *testing.T) { tc.volumeSnapshotterGetter, ) - assertEmptyResults(t, warnings, errs) + if tc.wantWarning { + assertNonEmptyResults(t, "warning", warnings) + } else { + assertEmptyResults(t, warnings) + } + if tc.wantError { + assertNonEmptyResults(t, "error", errs) + } else { + assertEmptyResults(t, errs) + } assertAPIContents(t, h, wantIDs) assertRestoredItems(t, h, tc.want) }) @@ -2804,6 +2904,17 @@ func assertEmptyResults(t *testing.T, res ...Result) { } } +func assertNonEmptyResults(t *testing.T, typeMsg string, res ...Result) { + t.Helper() + total := 0 + for _, r := range res { + total += len(r.Cluster) + total += len(r.Namespaces) + total += len(r.Velero) + } + assert.Greater(t, total, 0, "Expected at least one "+typeMsg) +} + type harness struct { *test.APIServer