From e115e5a191b1fdb5d379b62a35916115e77124a4 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 20 Oct 2020 14:51:30 -0400 Subject: [PATCH] v1.5.2 changelogs and cherry-picks (#3023) * Ensure PVs and PVCs remain bound when doing a restore (#3007) * Only remove the UID from a PV's claimRef The UID is the only part of a claimRef that might prevent it from being rebound correctly on a restore. The namespace and name within the claimRef should be preserved in order to ensure that the PV is claimed by the correct PVC on restore. Signed-off-by: Nolan Brubaker * Remap PVs claimRef.namespace on relevant restores When remapping namespaces, any included PVs need to have their claimRef updated to point remapped namespaces to the new namespace name in order to be bound to the correct PVC. Signed-off-by: Nolan Brubaker * Update tests and ensure claimRef namespace remaps Signed-off-by: Nolan Brubaker * Remove lowercased uid field from unstructured PV Signed-off-by: Nolan Brubaker * Fix issues that prevented PVs from being restored Signed-off-by: Nolan Brubaker * Add changelog Signed-off-by: Nolan Brubaker * Dynamically reprovision volumes without snapshots Signed-off-by: Nolan Brubaker * Update test for lower case uid field Signed-off-by: Nolan Brubaker * Remove stray debugging print statement Signed-off-by: Nolan Brubaker * Fix typo, remove extra code, add tests. Signed-off-by: Nolan Brubaker * restore proper lowercase/plural CRD resource (#2949) * restore proper lowercase/plural CRD resource This commit restores the proper resource string "customresourcedefinitions" for CRD. The prior change to "CustomResourceDefinition" was made because this was being used in another place to populate the CRD "Kind" field in remap_crd_version_action.go -- there, just use the correct Kind string instead of pulling from Resource. Signed-off-by: Scott Seago * add changelog Signed-off-by: Scott Seago * create CRB with velero- (#2886) * create CRB with velero- This will allow creating multiple instances of velero, across two different namespaces Signed-off-by: Alay Patel * add changelog Signed-off-by: Alay Patel * add package var DefaultVeleroNamespace and use it wherever needed Signed-off-by: Alay Patel * Fix BSL controller to avoid invoking init() on all BSLs regardless of ValidationFrequency (#2992) Signed-off-by: Bett, Antony * Fix version cmd getting nil pointer (#2996) Signed-off-by: Carlisia * Changelogs for v1.5.2 Signed-off-by: Nolan Brubaker Co-authored-by: Scott Seago Co-authored-by: Alay Patel Co-authored-by: Antony S Bett Co-authored-by: Carlisia Thompson --- changelogs/CHANGELOG-1.5.md | 21 ++++ pkg/backup/remap_crd_version_action.go | 3 +- pkg/client/factory.go | 4 + .../backupstoragelocation_controller.go | 10 +- pkg/install/resources.go | 7 +- pkg/install/resources_test.go | 15 ++- pkg/kuberesource/kuberesource.go | 2 +- pkg/restore/pv_restorer.go | 14 --- pkg/restore/pv_restorer_test.go | 15 +-- pkg/restore/restore.go | 99 ++++++++++++++++--- pkg/restore/restore_test.go | 59 +++++++++-- 11 files changed, 190 insertions(+), 59 deletions(-) diff --git a/changelogs/CHANGELOG-1.5.md b/changelogs/CHANGELOG-1.5.md index 7013a85e7..324d0f810 100644 --- a/changelogs/CHANGELOG-1.5.md +++ b/changelogs/CHANGELOG-1.5.md @@ -1,3 +1,23 @@ +## v1.5.2 +### 2020-10-20 +### Download +https://github.com/vmware-tanzu/velero/releases/tag/v1.5.2 + +### Container Image +`velero/velero:v1.5.2` + +### Documentation +https://velero.io/docs/v1.5/ + +### Upgrading +https://velero.io/docs/v1.5/upgrade-to-1.5/ + +### All Changes + * Fix BSL controller to avoid invoking init() on all BSLs regardless of ValidationFrequency (#2992, @betta1) + * cli: allow creating multiple instances of Velero across two different namespaces (#2886, @alaypatel07) + * Restore CRD Resource name to fix CRD wait functionality. (#2949, @sseago) + * Ensure that bound PVCs and PVs remain bound on restore. (#3007, @nrb) + ## v1.5.1 ### 2020-09-16 @@ -80,3 +100,4 @@ Displays the Timestamps when issued a print or describe (#2748, @thejasbabu) * when creating new backup from schedule from cli, allow backup name to be automatically generated (#2569, @cblecker) * Convert manifests + BSL api client to kubebuilder (#2561, @carlisia) * backup/restore: reinstantiate backup store just before uploading artifacts to ensure credentials are up-to-date (#2550, @skriss) + diff --git a/pkg/backup/remap_crd_version_action.go b/pkg/backup/remap_crd_version_action.go index eaced726e..b282e876e 100644 --- a/pkg/backup/remap_crd_version_action.go +++ b/pkg/backup/remap_crd_version_action.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "github.com/vmware-tanzu/velero/pkg/kuberesource" "github.com/vmware-tanzu/velero/pkg/plugin/velero" ) @@ -111,7 +110,7 @@ func fetchV1beta1CRD(name string, betaCRDClient apiextv1beta1client.CustomResour // See https://github.com/kubernetes/kubernetes/issues/3030. Unsure why this is happening here and not in main Velero; // probably has to do with List calls and Dynamic client vs typed client // Set these all the time, since they shouldn't ever be different, anyway - betaCRD.Kind = kuberesource.CustomResourceDefinitions.Resource + betaCRD.Kind = "CustomResourceDefinition" betaCRD.APIVersion = apiextv1beta1.SchemeGroupVersion.String() m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&betaCRD) diff --git a/pkg/client/factory.go b/pkg/client/factory.go index e46b81dc1..6ecc63be7 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -155,6 +155,10 @@ func (f *factory) KubebuilderClient() (kbclient.Client, error) { Scheme: scheme, }) + if err != nil { + return nil, err + } + return kubebuilderClient, nil } diff --git a/pkg/controller/backupstoragelocation_controller.go b/pkg/controller/backupstoragelocation_controller.go index fef15545b..8075bf058 100644 --- a/pkg/controller/backupstoragelocation_controller.go +++ b/pkg/controller/backupstoragelocation_controller.go @@ -77,14 +77,14 @@ func (r *BackupStorageLocationReconciler) Reconcile(req ctrl.Request) (ctrl.Resu defaultFound = true } - backupStore, err := r.NewBackupStore(location, pluginManager, log) - if err != nil { - log.WithError(err).Error("Error getting a backup store") + if !storage.IsReadyToValidate(location.Spec.ValidationFrequency, location.Status.LastValidationTime, r.DefaultBackupLocationInfo, log) { + log.Debug("Backup location not ready to be validated") continue } - if !storage.IsReadyToValidate(location.Spec.ValidationFrequency, location.Status.LastValidationTime, r.DefaultBackupLocationInfo, log) { - log.Debug("Backup location not ready to be validated") + backupStore, err := r.NewBackupStore(location, pluginManager, log) + if err != nil { + log.WithError(err).Error("Error getting a backup store") continue } diff --git a/pkg/install/resources.go b/pkg/install/resources.go index cbf78c5a4..9ea7ccbaf 100644 --- a/pkg/install/resources.go +++ b/pkg/install/resources.go @@ -51,6 +51,7 @@ var ( DefaultResticPodMemRequest = "512Mi" DefaultResticPodCPULimit = "1000m" DefaultResticPodMemLimit = "1Gi" + DefaultVeleroNamespace = "velero" ) func labels() map[string]string { @@ -105,8 +106,12 @@ func ServiceAccount(namespace string, annotations map[string]string) *corev1.Ser } func ClusterRoleBinding(namespace string) *rbacv1beta1.ClusterRoleBinding { + crbName := "velero" + if namespace != DefaultVeleroNamespace { + crbName = "velero-" + namespace + } crb := &rbacv1beta1.ClusterRoleBinding{ - ObjectMeta: objectMeta("", "velero"), + ObjectMeta: objectMeta("", crbName), TypeMeta: metav1.TypeMeta{ Kind: "ClusterRoleBinding", APIVersion: rbacv1beta1.SchemeGroupVersion.String(), diff --git a/pkg/install/resources_test.go b/pkg/install/resources_test.go index e2c296493..748d70def 100644 --- a/pkg/install/resources_test.go +++ b/pkg/install/resources_test.go @@ -23,7 +23,7 @@ import ( ) func TestResources(t *testing.T) { - bsl := BackupStorageLocation("velero", "test", "test", "", make(map[string]string), []byte("test")) + bsl := BackupStorageLocation(DefaultVeleroNamespace, "test", "test", "", make(map[string]string), []byte("test")) assert.Equal(t, "velero", bsl.ObjectMeta.Namespace) assert.Equal(t, "test", bsl.Spec.Provider) @@ -31,7 +31,7 @@ func TestResources(t *testing.T) { assert.Equal(t, make(map[string]string), bsl.Spec.Config) assert.Equal(t, []byte("test"), bsl.Spec.ObjectStorage.CACert) - vsl := VolumeSnapshotLocation("velero", "test", make(map[string]string)) + vsl := VolumeSnapshotLocation(DefaultVeleroNamespace, "test", make(map[string]string)) assert.Equal(t, "velero", vsl.ObjectMeta.Namespace) assert.Equal(t, "test", vsl.Spec.Provider) @@ -41,12 +41,19 @@ func TestResources(t *testing.T) { assert.Equal(t, "velero", ns.Name) - crb := ClusterRoleBinding("velero") + crb := ClusterRoleBinding(DefaultVeleroNamespace) // The CRB is a cluster-scoped resource assert.Equal(t, "", crb.ObjectMeta.Namespace) + assert.Equal(t, "velero", crb.ObjectMeta.Name) assert.Equal(t, "velero", crb.Subjects[0].Namespace) - sa := ServiceAccount("velero", map[string]string{"abcd": "cbd"}) + customNamespaceCRB := ClusterRoleBinding("foo") + // The CRB is a cluster-scoped resource + assert.Equal(t, "", customNamespaceCRB.ObjectMeta.Namespace) + assert.Equal(t, "velero-foo", customNamespaceCRB.ObjectMeta.Name) + assert.Equal(t, "foo", customNamespaceCRB.Subjects[0].Namespace) + + sa := ServiceAccount(DefaultVeleroNamespace, map[string]string{"abcd": "cbd"}) assert.Equal(t, "velero", sa.ObjectMeta.Namespace) assert.Equal(t, "cbd", sa.ObjectMeta.Annotations["abcd"]) } diff --git a/pkg/kuberesource/kuberesource.go b/pkg/kuberesource/kuberesource.go index d3daafe35..a515a70ff 100644 --- a/pkg/kuberesource/kuberesource.go +++ b/pkg/kuberesource/kuberesource.go @@ -23,7 +23,7 @@ import ( var ( ClusterRoleBindings = schema.GroupResource{Group: "rbac.authorization.k8s.io", Resource: "clusterrolebindings"} ClusterRoles = schema.GroupResource{Group: "rbac.authorization.k8s.io", Resource: "clusterroles"} - CustomResourceDefinitions = schema.GroupResource{Group: "apiextensions.k8s.io", Resource: "CustomResourceDefinition"} + CustomResourceDefinitions = schema.GroupResource{Group: "apiextensions.k8s.io", Resource: "customresourcedefinitions"} Jobs = schema.GroupResource{Group: "batch", Resource: "jobs"} Namespaces = schema.GroupResource{Group: "", Resource: "namespaces"} PersistentVolumeClaims = schema.GroupResource{Group: "", Resource: "persistentvolumeclaims"} diff --git a/pkg/restore/pv_restorer.go b/pkg/restore/pv_restorer.go index 2a83378c4..ff8d36b34 100644 --- a/pkg/restore/pv_restorer.go +++ b/pkg/restore/pv_restorer.go @@ -47,20 +47,6 @@ func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructu return nil, errors.New("PersistentVolume is missing its name") } - // It's simpler to just access the spec through the unstructured object than to convert - // to structured and back here, especially since the SetVolumeID(...) call below needs - // the unstructured representation (and does a conversion internally). - res, ok := obj.Object["spec"] - if !ok { - return nil, errors.New("spec not found") - } - spec, ok := res.(map[string]interface{}) - if !ok { - return nil, errors.Errorf("spec was of type %T, expected map[string]interface{}", res) - } - - delete(spec, "claimRef") - if boolptr.IsSetToFalse(r.snapshotVolumes) { // The backup had snapshots disabled, so we can return early return obj, nil diff --git a/pkg/restore/pv_restorer_test.go b/pkg/restore/pv_restorer_test.go index 0ac322442..c4fb0a9db 100644 --- a/pkg/restore/pv_restorer_test.go +++ b/pkg/restore/pv_restorer_test.go @@ -56,19 +56,6 @@ func TestExecutePVAction_NoSnapshotRestores(t *testing.T) { restore: builder.ForRestore(api.DefaultNamespace, "").Result(), expectedErr: true, }, - { - name: "no spec should error", - obj: NewTestUnstructured().WithName("pv-1").Unstructured, - restore: builder.ForRestore(api.DefaultNamespace, "").Result(), - expectedErr: true, - }, - { - name: "ensure spec.claimRef is deleted", - obj: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("claimRef", "someOtherField").Unstructured, - restore: builder.ForRestore(api.DefaultNamespace, "").RestorePVs(false).Result(), - backup: defaultBackup().Phase(api.BackupPhaseInProgress).Result(), - expectedRes: NewTestUnstructured().WithAnnotations("a", "b").WithName("pv-1").WithSpec("someOtherField").Unstructured, - }, { name: "ensure spec.storageClassName is retained", obj: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("storageClassName", "someOtherField").Unstructured, @@ -81,7 +68,7 @@ func TestExecutePVAction_NoSnapshotRestores(t *testing.T) { obj: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("claimRef", "storageClassName", "someOtherField").Unstructured, restore: builder.ForRestore(api.DefaultNamespace, "").RestorePVs(true).Result(), backup: defaultBackup().Phase(api.BackupPhaseInProgress).SnapshotVolumes(false).Result(), - expectedRes: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("storageClassName", "someOtherField").Unstructured, + expectedRes: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("claimRef", "storageClassName", "someOtherField").Unstructured, }, { name: "restore.spec.restorePVs=false, return early", diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 243d080e5..7e056af55 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -62,6 +62,14 @@ import ( "github.com/vmware-tanzu/velero/pkg/volume" ) +// These annotations are taken from the Kubernetes persistent volume/persistent volume claim controller. +// They cannot be directly importing because they are part of the kubernetes/kubernetes package, and importing that package is unsupported. +// Their values are well-known and slow changing. They're duplicated here as constants to provide compile-time checking. +// Originals can be found in kubernetes/kubernetes/pkg/controller/volume/persistentvolume/util/util.go. +const KubeAnnBindCompleted = "pv.kubernetes.io/bind-completed" +const KubeAnnBoundByController = "pv.kubernetes.io/bound-by-controller" +const KubeAnnDynamicallyProvisioned = "pv.kubernetes.io/provisioned-by" + type VolumeSnapshotterGetter interface { GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error) } @@ -882,6 +890,13 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso return warnings, errs } + // 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 + } + var shouldRestoreSnapshot bool if !shouldRenamePV { // Check if the PV exists in the cluster before attempting to create @@ -899,6 +914,9 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso } if shouldRestoreSnapshot { + // reset the PV's binding status so that Kubernetes can properly associate it with the restored PVC. + obj = resetVolumeBindingInfo(obj) + // 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.") @@ -958,8 +976,9 @@ 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.") - // we call the pvRestorer here to clear out the PV's claimRef, so it can be re-claimed - // when its PVC is restored. + 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. updatedObj, err := ctx.pvRestorer.executePVAction(obj) if err != nil { errs.Add(namespace, fmt.Errorf("error executing PVAction for %s: %v", resourceID, err)) @@ -1052,17 +1071,16 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso return warnings, errs } - if pvc.Spec.VolumeName != "" && ctx.pvsToProvision.Has(pvc.Spec.VolumeName) { - ctx.log.Infof("Resetting PersistentVolumeClaim %s/%s for dynamic provisioning", namespace, name) + if pvc.Spec.VolumeName != "" { + // This used to only happen with restic volumes, but now always remove this binding metadata + obj = resetVolumeBindingInfo(obj) - // use the unstructured helpers here since we're only deleting and - // the unstructured converter will add back (empty) fields for metadata - // and status that we removed earlier. - unstructured.RemoveNestedField(obj.Object, "spec", "volumeName") - annotations := obj.GetAnnotations() - delete(annotations, "pv.kubernetes.io/bind-completed") - delete(annotations, "pv.kubernetes.io/bound-by-controller") - obj.SetAnnotations(annotations) + // This is the case for restic volumes, where we need to actually have an empty volume created instead of restoring one. + // The assumption is that any PV in pvsToProvision doesn't have an associated snapshot. + if ctx.pvsToProvision.Has(pvc.Spec.VolumeName) { + ctx.log.Infof("Resetting PersistentVolumeClaim %s/%s for dynamic provisioning", namespace, name) + unstructured.RemoveNestedField(obj.Object, "spec", "volumeName") + } } if newName, ok := ctx.renamedPVs[pvc.Spec.VolumeName]; ok { @@ -1215,6 +1233,40 @@ func shouldRenamePV(ctx *restoreContext, obj *unstructured.Unstructured, client return true, nil } +// remapClaimRefNS remaps a PersistentVolume's claimRef.Namespace based on a restore's NamespaceMappings, if necessary. +// Returns true if the namespace was remapped, false if it was not required. +func remapClaimRefNS(ctx *restoreContext, obj *unstructured.Unstructured) (bool, error) { + if len(ctx.restore.Spec.NamespaceMapping) == 0 { + ctx.log.Debug("Persistent volume does not need to have the claimRef.namespace remapped because restore is not remapping any namespaces") + return false, nil + } + + // Conversion to the real type here is more readable than all the error checking involved with reading each field individually. + 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 have the claimRef.namepace remapped because it's not claimed") + return false, nil + } + + targetNS, ok := ctx.restore.Spec.NamespaceMapping[pv.Spec.ClaimRef.Namespace] + + if !ok { + ctx.log.Debugf("Persistent volume does not need to have the claimRef.namespace remapped because it's not claimed by a PVC in a namespace that's being remapped") + return false, nil + } + + err := unstructured.SetNestedField(obj.Object, targetNS, "spec", "claimRef", "namespace") + if err != nil { + return false, err + } + ctx.log.Debug("Persistent volume's namespace was updated") + return true, nil +} + // restorePodVolumeBackups restores the PodVolumeBackups for the given restored pod func restorePodVolumeBackups(ctx *restoreContext, createdObj *unstructured.Unstructured, originalNamespace string) { if ctx.resticRestorer == nil { @@ -1329,6 +1381,29 @@ func hasDeleteReclaimPolicy(obj map[string]interface{}) bool { return policy == string(v1.PersistentVolumeReclaimDelete) } +// resetVolumeBindingInfo clears any necessary metadata out of a PersistentVolume or PersistentVolumeClaim that would make it ineligible to be re-bound by Velero. +func resetVolumeBindingInfo(obj *unstructured.Unstructured) *unstructured.Unstructured { + // Clean out ClaimRef UID and resourceVersion, since this information is highly unique. + unstructured.RemoveNestedField(obj.Object, "spec", "claimRef", "uid") + unstructured.RemoveNestedField(obj.Object, "spec", "claimRef", "resourceVersion") + + // Clear out any annotations used by the Kubernetes PV controllers to track bindings. + annotations := obj.GetAnnotations() + + // Upon restore, this new PV will look like a statically provisioned, manually-bound volume rather than one bound by the controller, so remove the annotation that signals that a controller bound it. + delete(annotations, KubeAnnBindCompleted) + // Remove the annotation that signals that the PV is already bound; we want the PV(C) controller to take the two objects and bind them again. + delete(annotations, KubeAnnBoundByController) + + // Remove the provisioned-by annotation which signals that the persistent volume was dynamically provisioned; it is now statically provisioned. + delete(annotations, KubeAnnDynamicallyProvisioned) + + // GetAnnotations returns a copy, so we have to set them again + obj.SetAnnotations(annotations) + + return obj +} + func resetMetadataAndStatus(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { res, ok := obj.Object["metadata"] if !ok { diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 8e8d3b3fb..a29331076 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -1830,7 +1830,7 @@ func TestRestorePersistentVolumes(t *testing.T) { }, }, { - name: "when a PV with a reclaim policy of retain has no snapshot and does not exist in-cluster, it gets restored, without its claim ref", + name: "when a PV with a reclaim policy of retain has no snapshot and does not exist in-cluster, it gets restored, with its claim ref", restore: defaultRestore().Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). @@ -1849,6 +1849,7 @@ func TestRestorePersistentVolumes(t *testing.T) { ObjectMeta( builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), ). + ClaimRef("ns-1", "pvc-1"). Result(), ), }, @@ -2096,13 +2097,12 @@ func TestRestorePersistentVolumes(t *testing.T) { 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"), - ). + // the namespace for this PV's claimRef should be the one that the PVC was remapped into. + ).ClaimRef("target-ns", "pvc-1"). AWSEBSVolumeID("new-volume"). Result(), ), @@ -2161,6 +2161,7 @@ func TestRestorePersistentVolumes(t *testing.T) { ObjectMeta( builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), ). + ClaimRef("target-ns", "pvc-1"). AWSEBSVolumeID("new-volume"). Result(), ), @@ -2221,6 +2222,7 @@ func TestRestorePersistentVolumes(t *testing.T) { builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"), builder.WithAnnotations("velero.io/original-pv-name", "source-pv"), ). + ClaimRef("target-ns", "pvc-1"). AWSEBSVolumeID("new-pvname"). Result(), ), @@ -2340,13 +2342,12 @@ func TestRestorePersistentVolumes(t *testing.T) { 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("volumesnapshotter-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"), ). + ClaimRef("target-ns", "pvc-1"). AWSEBSVolumeID("new-volume"). Result(), ), @@ -2846,3 +2847,49 @@ func (h *harness) AddItems(t *testing.T, resource *test.APIResource) { require.NoError(t, err) } } + +func Test_resetVolumeBindingInfo(t *testing.T) { + tests := []struct { + name string + obj *unstructured.Unstructured + expected *unstructured.Unstructured + }{ + { + name: "PVs that are bound have their binding and dynamic provisioning annotations removed", + obj: NewTestUnstructured().WithMetadataField("kind", "persistentVolume"). + WithName("pv-1").WithAnnotations( + KubeAnnBindCompleted, + KubeAnnBoundByController, + KubeAnnDynamicallyProvisioned, + ).WithSpecField("claimRef", map[string]interface{}{ + "namespace": "ns-1", + "name": "pvc-1", + "uid": "abc", + "resourceVersion": "1"}).Unstructured, + expected: NewTestUnstructured().WithMetadataField("kind", "persistentVolume"). + WithName("pv-1"). + WithAnnotations(). + WithSpecField("claimRef", map[string]interface{}{ + "namespace": "ns-1", "name": "pvc-1"}).Unstructured, + }, + { + name: "PVCs that are bound have their binding annotations removed, but the volume name stays", + obj: NewTestUnstructured().WithMetadataField("kind", "persistentVolumeClaim"). + WithName("pvc-1").WithAnnotations( + KubeAnnBindCompleted, + KubeAnnBoundByController, + KubeAnnDynamicallyProvisioned, + ).WithSpecField("volumeName", "pv-1").Unstructured, + expected: NewTestUnstructured().WithMetadataField("kind", "persistentVolumeClaim"). + WithName("pvc-1").WithAnnotations(). + WithSpecField("volumeName", "pv-1").Unstructured, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + actual := resetVolumeBindingInfo(tc.obj) + assert.Equal(t, tc.expected, actual) + }) + } +}