diff --git a/changelogs/unreleased/1779-skriss b/changelogs/unreleased/1779-skriss new file mode 100644 index 000000000..114f1f1b6 --- /dev/null +++ b/changelogs/unreleased/1779-skriss @@ -0,0 +1 @@ +when using `velero restore create --namespace-mappings ...` to create a second copy of a namespace in a cluster, create copies of the PVs used diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index dbff5b3eb..2f89dc04d 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -29,6 +29,7 @@ import ( "time" "github.com/pkg/errors" + uuid "github.com/satori/go.uuid" "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -93,6 +94,7 @@ type kubernetesRestorer struct { resourceTerminatingTimeout time.Duration resourcePriorities []string fileSystem filesystem.Interface + pvRenamer func(string) string logger logrus.FieldLogger } @@ -174,6 +176,7 @@ func NewKubernetesRestorer( resourceTerminatingTimeout: resourceTerminatingTimeout, resourcePriorities: resourcePriorities, logger: logger, + pvRenamer: func(string) string { return "velero-clone-" + uuid.NewV4().String() }, fileSystem: filesystem.NewFileSystem(), }, nil } @@ -275,6 +278,8 @@ func (kr *kubernetesRestorer) Restore( }, resourceClients: make(map[resourceClientKey]client.Dynamic), restoredItems: make(map[velero.ResourceIdentifier]struct{}), + renamedPVs: make(map[string]string), + pvRenamer: kr.pvRenamer, } return restoreCtx.execute() @@ -366,6 +371,8 @@ type context struct { extractor *backupExtractor resourceClients map[resourceClientKey]client.Dynamic restoredItems map[velero.ResourceIdentifier]struct{} + renamedPVs map[string]string + pvRenamer func(string) string } type resourceClientKey struct { @@ -876,15 +883,31 @@ func (ctx *context) restoreItem(obj *unstructured.Unstructured, groupResource sc if groupResource == kuberesource.PersistentVolumes { switch { case hasSnapshot(name, ctx.volumeSnapshots): - // Check if the PV exists in the cluster before attempting to create - // a volume from the snapshot, in order to avoid orphaned volumes (GH #609) - shouldRestoreSnapshot, err := ctx.shouldRestore(name, resourceClient) + shouldRenamePV, err := shouldRenamePV(ctx, obj, resourceClient) if err != nil { - addToResult(&errs, namespace, errors.Wrapf(err, "error waiting on in-cluster persistentvolume %s", name)) + addToResult(&errs, namespace, err) return warnings, errs } + var shouldRestoreSnapshot bool + if !shouldRenamePV { + // Check if the PV exists in the cluster before attempting to create + // a volume from the snapshot, in order to avoid orphaned volumes (GH #609) + shouldRestoreSnapshot, err = ctx.shouldRestore(name, resourceClient) + if err != nil { + addToResult(&errs, namespace, errors.Wrapf(err, "error waiting on in-cluster persistentvolume %s", name)) + return warnings, errs + } + } else { + // if we're renaming the PV, we're going to give it a new random name, + // so we can assume it doesn't already exist in the cluster and therefore + // we should proceed with restoring from snapshot. + shouldRestoreSnapshot = true + } + if shouldRestoreSnapshot { + // even if we're renaming the PV, obj still has the old name here, because the pvRestorer + // uses the original name to look up metadata about the snapshot. ctx.log.Infof("Restoring persistent volume from snapshot.") updatedObj, err := ctx.pvRestorer.executePVAction(obj) if err != nil { @@ -893,18 +916,38 @@ func (ctx *context) restoreItem(obj *unstructured.Unstructured, groupResource sc } obj = updatedObj } + + if shouldRenamePV { + // give obj a new name, and record the mapping between the old and new names + oldName := obj.GetName() + newName := ctx.pvRenamer(oldName) + + ctx.renamedPVs[oldName] = newName + obj.SetName(newName) + + // add the original PV name as an annotation + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations["velero.io/original-pv-name"] = oldName + obj.SetAnnotations(annotations) + } + case hasResticBackup(obj, ctx): ctx.log.Infof("Dynamically re-provisioning persistent volume because it has a restic backup to be restored.") ctx.pvsToProvision.Insert(name) // return early because we don't want to restore the PV itself, we want to dynamically re-provision it. return warnings, errs + case hasDeleteReclaimPolicy(obj.Object): ctx.log.Infof("Dynamically re-provisioning persistent volume because it doesn't have a snapshot and its reclaim policy is Delete.") ctx.pvsToProvision.Insert(name) // return early because we don't want to restore the PV itself, we want to dynamically re-provision it. return warnings, errs + default: ctx.log.Infof("Restoring persistent volume as-is because it doesn't have a snapshot and its reclaim policy is not Delete.") @@ -1014,6 +1057,14 @@ func (ctx *context) restoreItem(obj *unstructured.Unstructured, groupResource sc delete(annotations, "pv.kubernetes.io/bound-by-controller") obj.SetAnnotations(annotations) } + + if newName, ok := ctx.renamedPVs[pvc.Spec.VolumeName]; ok { + ctx.log.Infof("Updating persistent volume claim %s/%s to reference renamed persistent volume (%s -> %s)", namespace, name, pvc.Spec.VolumeName, newName) + if err := unstructured.SetNestedField(obj.Object, newName, "spec", "volumeName"); err != nil { + addToResult(&errs, namespace, err) + return warnings, errs + } + } } // necessary because we may have remapped the namespace @@ -1103,6 +1154,45 @@ func (ctx *context) restoreItem(obj *unstructured.Unstructured, groupResource sc return warnings, errs } +// shouldRenamePV returns a boolean indicating whether a persistent volume should be given a new name +// before being restored, or an error if this cannot be determined. A persistent volume will be +// given a new name if and only if (a) a PV with the original name already exists in-cluster, and +// (b) in the backup, the PV is claimed by a PVC in a namespace that's being remapped during the +// restore. +func shouldRenamePV(ctx *context, obj *unstructured.Unstructured, client client.Dynamic) (bool, error) { + if len(ctx.restore.Spec.NamespaceMapping) == 0 { + ctx.log.Debugf("Persistent volume does not need to be renamed because restore is not remapping any namespaces") + return false, nil + } + + pv := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, pv); err != nil { + return false, errors.Wrapf(err, "error converting persistent volume to structured") + } + + if pv.Spec.ClaimRef == nil { + ctx.log.Debugf("Persistent volume does not need to be renamed because it's not claimed") + return false, nil + } + + if _, ok := ctx.restore.Spec.NamespaceMapping[pv.Spec.ClaimRef.Namespace]; !ok { + ctx.log.Debugf("Persistent volume does not need to be renamed because it's not claimed by a PVC in a namespace that's being remapped") + return false, nil + } + + _, err := client.Get(pv.Name, metav1.GetOptions{}) + switch { + case apierrors.IsNotFound(err): + ctx.log.Debugf("Persistent volume does not need to be renamed because it does not exist in the cluster") + return false, nil + case err != nil: + return false, errors.Wrapf(err, "error checking if persistent volume exists in the cluster") + } + + // no error returned: the PV was found in-cluster, so we need to rename it + return true, nil +} + // restorePodVolumeBackups restores the PodVolumeBackups for the given restored pod func restorePodVolumeBackups(ctx *context, createdObj *unstructured.Unstructured, originalNamespace string) { if ctx.resticRestorer == nil { diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index a604bb8fa..83f0a6756 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -2000,7 +2000,128 @@ func TestRestorePersistentVolumes(t *testing.T) { ), }, }, - + { + name: "when a PV with a snapshot is used by a PVC in a namespace that's being remapped, and the original PV exists in-cluster, the PV is renamed", + restore: defaultRestore().NamespaceMappings("source-ns", "target-ns").Result(), + backup: defaultBackup().Result(), + tarball: 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( + builder.ForPersistentVolume("source-pv").AWSEBSVolumeID("source-volume").ClaimRef("source-ns", "pvc-1").Result(), + ), + test.PVCs(), + }, + volumeSnapshots: []*volume.Snapshot{ + { + Spec: volume.SnapshotSpec{ + BackupName: "backup-1", + Location: "default", + PersistentVolumeName: "source-pv", + }, + Status: volume.SnapshotStatus{ + Phase: volume.SnapshotPhaseCompleted, + ProviderSnapshotID: "snapshot-1", + }, + }, + }, + volumeSnapshotLocations: []*velerov1api.VolumeSnapshotLocation{ + builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "default").Provider("provider-1").Result(), + }, + volumeSnapshotterGetter: map[string]velero.VolumeSnapshotter{ + "provider-1": &volumeSnapshotter{ + snapshotVolumes: map[string]string{"snapshot-1": "new-volume"}, + }, + }, + want: []*test.APIResource{ + test.PVs( + builder.ForPersistentVolume("source-pv").AWSEBSVolumeID("source-volume").ClaimRef("source-ns", "pvc-1").Result(), + // note that the renamed PV is not expected to have a claimRef in this test; that would be + // added after creation by the Kubernetes PV/PVC controller when it does a bind. + builder.ForPersistentVolume("renamed-source-pv"). + ObjectMeta( + builder.WithAnnotations("velero.io/original-pv-name", "source-pv"), + builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), + ). + AWSEBSVolumeID("new-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("renamed-source-pv"). + Result(), + ), + }, + }, + { + name: "when a PV with 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: 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(), + }, + volumeSnapshots: []*volume.Snapshot{ + { + Spec: volume.SnapshotSpec{ + BackupName: "backup-1", + Location: "default", + PersistentVolumeName: "source-pv", + }, + Status: volume.SnapshotStatus{ + Phase: volume.SnapshotPhaseCompleted, + ProviderSnapshotID: "snapshot-1", + }, + }, + }, + volumeSnapshotLocations: []*velerov1api.VolumeSnapshotLocation{ + builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "default").Provider("provider-1").Result(), + }, + volumeSnapshotterGetter: map[string]velero.VolumeSnapshotter{ + "provider-1": &volumeSnapshotter{ + snapshotVolumes: map[string]string{"snapshot-1": "new-volume"}, + }, + }, + want: []*test.APIResource{ + test.PVs( + builder.ForPersistentVolume("source-pv"). + ObjectMeta( + builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), + ). + AWSEBSVolumeID("new-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 with a reclaim policy of retain has a snapshot and exists in-cluster, neither the snapshot nor the PV are restored", restore: defaultRestore().Result(), @@ -2068,6 +2189,7 @@ func TestRestorePersistentVolumes(t *testing.T) { t.Run(tc.name, func(t *testing.T) { h := newHarness(t) h.restorer.resourcePriorities = []string{"persistentvolumes", "persistentvolumeclaims"} + h.restorer.pvRenamer = func(oldName string) string { return "renamed-" + oldName } // set up the VolumeSnapshotLocation informer/lister and add test data to it vslInformer := velerov1informers.NewSharedInformerFactory(h.VeleroClient, 0).Velero().V1().VolumeSnapshotLocations()