From 8878ba860e1ddb44e334b8f105078a742059843a Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Thu, 21 Dec 2017 13:23:48 -0800 Subject: [PATCH] don't remove annotations from PVs on restore Signed-off-by: Steve Kriss --- pkg/restore/restore.go | 21 ++++---------- pkg/restore/restore_test.go | 58 +++++++++++++------------------------ 2 files changed, 26 insertions(+), 53 deletions(-) diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 219987393..a0848b2da 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -628,7 +628,7 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a } // clear out non-core metadata fields & status - if obj, err = resetMetadataAndStatus(obj, true); err != nil { + if obj, err = resetMetadataAndStatus(obj); err != nil { addToResult(&errs, namespace, err) continue } @@ -676,15 +676,6 @@ func (ctx *context) executePVAction(obj *unstructured.Unstructured) (*unstructur return nil, err } - metadata, err := collections.GetMap(obj.UnstructuredContent(), "metadata") - if err != nil { - return nil, err - } - - // We need to remove annotations from PVs since they potentially contain - // information about dynamic provisioners which will confuse the controllers. - delete(metadata, "annotations") - delete(spec, "claimRef") delete(spec, "storageClassName") @@ -740,18 +731,18 @@ func isPVReady(obj runtime.Unstructured) bool { return phase == string(v1.VolumeAvailable) } -func resetMetadataAndStatus(obj *unstructured.Unstructured, keepAnnotations bool) (*unstructured.Unstructured, error) { +func resetMetadataAndStatus(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { metadata, err := collections.GetMap(obj.UnstructuredContent(), "metadata") if err != nil { return nil, err } for k := range metadata { - if k == "name" || k == "namespace" || k == "labels" || (k == "annotations" && keepAnnotations) { - continue + switch k { + case "name", "namespace", "labels", "annotations": + default: + delete(metadata, k) } - - delete(metadata, k) } // this should never be backed up anyway, but remove it just diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 5e0891169..c5b309767 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -641,51 +641,33 @@ func TestHasControllerOwner(t *testing.T) { func TestResetMetadataAndStatus(t *testing.T) { tests := []struct { - name string - obj *unstructured.Unstructured - keepAnnotations bool - expectedErr bool - expectedRes *unstructured.Unstructured + name string + obj *unstructured.Unstructured + expectedErr bool + expectedRes *unstructured.Unstructured }{ { - name: "no metadata causes error", - obj: NewTestUnstructured().Unstructured, - keepAnnotations: false, - expectedErr: true, + name: "no metadata causes error", + obj: NewTestUnstructured().Unstructured, + expectedErr: true, }, { - name: "don't keep annotations", - obj: NewTestUnstructured().WithMetadata("name", "namespace", "labels", "annotations").Unstructured, - keepAnnotations: false, - expectedErr: false, - expectedRes: NewTestUnstructured().WithMetadata("name", "namespace", "labels").Unstructured, + name: "keep name, namespace, labels, annotations only", + obj: NewTestUnstructured().WithMetadata("name", "blah", "namespace", "labels", "annotations", "foo").Unstructured, + expectedErr: false, + expectedRes: NewTestUnstructured().WithMetadata("name", "namespace", "labels", "annotations").Unstructured, }, { - name: "keep annotations", - obj: NewTestUnstructured().WithMetadata("name", "namespace", "labels", "annotations").Unstructured, - keepAnnotations: true, - expectedErr: false, - expectedRes: NewTestUnstructured().WithMetadata("name", "namespace", "labels", "annotations").Unstructured, - }, - { - name: "don't keep extraneous metadata", - obj: NewTestUnstructured().WithMetadata("foo").Unstructured, - keepAnnotations: false, - expectedErr: false, - expectedRes: NewTestUnstructured().WithMetadata().Unstructured, - }, - { - name: "don't keep status", - obj: NewTestUnstructured().WithMetadata().WithStatus().Unstructured, - keepAnnotations: false, - expectedErr: false, - expectedRes: NewTestUnstructured().WithMetadata().Unstructured, + name: "don't keep status", + obj: NewTestUnstructured().WithMetadata().WithStatus().Unstructured, + expectedErr: false, + expectedRes: NewTestUnstructured().WithMetadata().Unstructured, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - res, err := resetMetadataAndStatus(test.obj, test.keepAnnotations) + res, err := resetMetadataAndStatus(test.obj) if assert.Equal(t, test.expectedErr, err != nil) { assert.Equal(t, test.expectedRes, res) @@ -722,18 +704,18 @@ func TestExecutePVAction(t *testing.T) { expectedErr: true, }, { - name: "ensure annotations, spec.claimRef, spec.storageClassName are deleted", + name: "ensure spec.claimRef, spec.storageClassName are deleted", obj: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("claimRef", "storageClassName", "someOtherField").Unstructured, restore: arktest.NewDefaultTestRestore().WithRestorePVs(false).Restore, backup: &api.Backup{}, - expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec("someOtherField").Unstructured, + expectedRes: NewTestUnstructured().WithAnnotations("a", "b").WithName("pv-1").WithSpec("someOtherField").Unstructured, }, { name: "if backup.spec.snapshotVolumes is false, ignore restore.spec.restorePVs and return early", obj: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("claimRef", "storageClassName", "someOtherField").Unstructured, restore: arktest.NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: &api.Backup{Spec: api.BackupSpec{SnapshotVolumes: boolptr.False()}}, - expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec("someOtherField").Unstructured, + expectedRes: NewTestUnstructured().WithName("pv-1").WithAnnotations("a", "b").WithSpec("someOtherField").Unstructured, }, { name: "not restoring, return early", @@ -1181,7 +1163,7 @@ func (r *fakeAction) Execute(obj runtime.Unstructured, restore *api.Restore) (ru } // want the baseline functionality too - res, err := resetMetadataAndStatus(unstructuredObj, true) + res, err := resetMetadataAndStatus(unstructuredObj) if err != nil { return nil, nil, err }