Merge pull request #754 from skriss/namespace-restore-label

update how we label restored objects

docs/about.md

@@ -29,7 +29,7 @@ Scheduled backups are saved with the name `<SCHEDULE NAME>-<TIMESTAMP>`, where `
 
 The **restore** operation allows you to restore all of the objects and persistent volumes from a previously created backup. You can also restore only a filtered subset of objects and persistent volumes. Ark supports multiple namespace remapping--for example, in a single restore, objects in namespace "abc" can be recreated under namespace "def", and the objects in namespace "123" under "456".
 
-The default name of a restore is `<BACKUP NAME>-<TIMESTAMP>`, where `<TIMESTAMP>` is formatted as *YYYYMMDDhhmmss*. You can also specify a custom name. A restored object also includes a label with key `ark-restore` and value `<RESTORE NAME>`.
+The default name of a restore is `<BACKUP NAME>-<TIMESTAMP>`, where `<TIMESTAMP>` is formatted as *YYYYMMDDhhmmss*. You can also specify a custom name. A restored object also includes a label with key `ark.heptio.com/restore-name` and value `<RESTORE NAME>`.
 
 You can also run the Ark server in restore-only mode, which disables backup, schedule, and garbage collection functionality during disaster recovery.
 

pkg/apis/ark/v1/constants.go

@@ -28,6 +28,9 @@ const (
 	// RestoreLabelKey is the label key that's applied to all resources that
 	// are created during a restore. This is applied for ease of identification
 	// of restored resources. The value will be the restore's name.
+	//
+	// This label is DEPRECATED as of v0.10 and will be removed entirely as of
+	// v1.0 and replaced with RestoreNameLabel ("ark.heptio.com/restore-name").
 	RestoreLabelKey = "ark-restore"
 
 	// ClusterScopedDir is the name of the directory containing cluster-scoped

pkg/restore/restore.go

@@ -760,8 +760,10 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a
 			obj.SetNamespace(namespace)
 		}
 
-		// add an ark-restore label to each resource for easy ID
-		addLabel(obj, api.RestoreLabelKey, ctx.restore.Name)
+		// label the resource with the restore's name and the restored backup's name
+		// for easy identification of all cluster resources created by this restore
+		// and which backup they came from
+		addRestoreLabels(obj, ctx.restore.Name, ctx.restore.Spec.BackupName)
 
 		ctx.infof("Restoring %s: %v", obj.GroupVersionKind().Kind, name)
 		createdObj, restoreErr := resourceClient.Create(obj)
@@ -780,10 +782,10 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a
 				continue
 			}
 
-			// We know the cluster won't have the restore name label, so
-			// copy it over from the backup
-			restoreName := obj.GetLabels()[api.RestoreLabelKey]
-			addLabel(fromCluster, api.RestoreLabelKey, restoreName)
+			// We know the object from the cluster won't have the backup/restore name labels, so
+			// copy them from the object we attempted to restore.
+			labels := obj.GetLabels()
+			addRestoreLabels(fromCluster, labels[api.RestoreNameLabel], labels[api.BackupNameLabel])
 
 			if !equality.Semantic.DeepEqual(fromCluster, obj) {
 				switch groupResource {
@@ -997,15 +999,22 @@ func resetMetadataAndStatus(obj *unstructured.Unstructured) (*unstructured.Unstr
 	return obj, nil
 }
 
-// addLabel applies the specified key/value to an object as a label.
-func addLabel(obj *unstructured.Unstructured, key string, val string) {
+// addRestoreLabels labels the provided object with the restore name and
+// the restored backup's name.
+func addRestoreLabels(obj metav1.Object, restoreName, backupName string) {
 	labels := obj.GetLabels()
 
 	if labels == nil {
 		labels = make(map[string]string)
 	}
 
-	labels[key] = val
+	labels[api.BackupNameLabel] = backupName
+	labels[api.RestoreNameLabel] = restoreName
+
+	// TODO(1.0): remove the below line, and remove the `RestoreLabelKey`
+	// constant from the API pkg, since it's been replaced with the
+	// namespaced label above.
+	labels[api.RestoreLabelKey] = restoreName
 
 	obj.SetLabels(labels)
 }

pkg/restore/restore_test.go

@@ -307,11 +307,12 @@ func TestNamespaceRemapping(t *testing.T) {
 					WithFile("bak/resources/configmaps/namespaces/ns-1/cm-1.json", newTestConfigMap().WithNamespace("ns-1").ToJSON()).
 					WithFile("bak/resources/namespaces/cluster/ns-1.json", newTestNamespace("ns-1").ToJSON())
 		expectedNS   = "ns-2"
-		expectedObjs = toUnstructured(newTestConfigMap().WithNamespace("ns-2").WithArkLabel("").ConfigMap)
+		expectedObjs = toUnstructured(newTestConfigMap().WithNamespace("ns-2").ConfigMap)
 	)
 
 	resourceClient := &arktest.FakeDynamicClient{}
 	for i := range expectedObjs {
+		addRestoreLabels(&expectedObjs[i], "", "")
 		resourceClient.On("Create", &expectedObjs[i]).Return(&expectedObjs[i], nil)
 	}
 
@@ -381,8 +382,8 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 				WithFile("configmaps/cm-1.json", newNamedTestConfigMap("cm-1").ToJSON()).
 				WithFile("configmaps/cm-2.json", newNamedTestConfigMap("cm-2").ToJSON()),
 			expectedObjs: toUnstructured(
-				newNamedTestConfigMap("cm-1").WithArkLabel("my-restore").ConfigMap,
-				newNamedTestConfigMap("cm-2").WithArkLabel("my-restore").ConfigMap,
+				newNamedTestConfigMap("cm-1").ConfigMap,
+				newNamedTestConfigMap("cm-2").ConfigMap,
 			),
 		},
 		{
@@ -415,7 +416,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 					"ns-1": {"error decoding \"configmaps/cm-1-invalid.json\": invalid character 'h' in literal true (expecting 'r')"},
 				},
 			},
-			expectedObjs: toUnstructured(newNamedTestConfigMap("cm-2").WithArkLabel("my-restore").ConfigMap),
+			expectedObjs: toUnstructured(newNamedTestConfigMap("cm-2").ConfigMap),
 		},
 		{
 			name:          "matching label selector correctly includes",
@@ -423,7 +424,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			resourcePath:  "configmaps",
 			labelSelector: labels.SelectorFromSet(labels.Set(map[string]string{"foo": "bar"})),
 			fileSystem:    arktest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().WithLabels(map[string]string{"foo": "bar"}).ToJSON()),
-			expectedObjs:  toUnstructured(newTestConfigMap().WithLabels(map[string]string{"foo": "bar"}).WithArkLabel("my-restore").ConfigMap),
+			expectedObjs:  toUnstructured(newTestConfigMap().WithLabels(map[string]string{"foo": "bar"}).ConfigMap),
 		},
 		{
 			name:          "non-matching label selector correctly excludes",
@@ -440,7 +441,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			fileSystem: arktest.NewFakeFileSystem().
 				WithFile("configmaps/cm-1.json", newTestConfigMap().WithControllerOwner().ToJSON()).
 				WithFile("configmaps/cm-2.json", newNamedTestConfigMap("cm-2").ToJSON()),
-			expectedObjs: toUnstructured(newNamedTestConfigMap("cm-2").WithArkLabel("my-restore").ConfigMap),
+			expectedObjs: toUnstructured(newNamedTestConfigMap("cm-2").ConfigMap),
 		},
 		{
 			name:          "namespace is remapped",
@@ -448,7 +449,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			resourcePath:  "configmaps",
 			labelSelector: labels.NewSelector(),
 			fileSystem:    arktest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().WithNamespace("ns-1").ToJSON()),
-			expectedObjs:  toUnstructured(newTestConfigMap().WithNamespace("ns-2").WithArkLabel("my-restore").ConfigMap),
+			expectedObjs:  toUnstructured(newTestConfigMap().WithNamespace("ns-2").ConfigMap),
 		},
 		{
 			name:          "custom restorer is correctly used",
@@ -464,7 +465,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 					selector:                  labels.Everything(),
 				},
 			},
-			expectedObjs: toUnstructured(newTestConfigMap().WithLabels(map[string]string{"fake-restorer": "foo"}).WithArkLabel("my-restore").ConfigMap),
+			expectedObjs: toUnstructured(newTestConfigMap().WithLabels(map[string]string{"fake-restorer": "foo"}).ConfigMap),
 		},
 		{
 			name:          "custom restorer for different group/resource is not used",
@@ -480,7 +481,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 					selector:                  labels.Everything(),
 				},
 			},
-			expectedObjs: toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap),
+			expectedObjs: toUnstructured(newTestConfigMap().ConfigMap),
 		},
 		{
 			name:                    "cluster-scoped resources are skipped when IncludeClusterResources=false",
@@ -497,7 +498,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			labelSelector:           labels.NewSelector(),
 			includeClusterResources: falsePtr,
 			fileSystem:              arktest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()),
-			expectedObjs:            toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap),
+			expectedObjs:            toUnstructured(newTestConfigMap().ConfigMap),
 		},
 		{
 			name:                    "cluster-scoped resources are not skipped when IncludeClusterResources=true",
@@ -506,7 +507,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			labelSelector:           labels.NewSelector(),
 			includeClusterResources: truePtr,
 			fileSystem:              arktest.NewFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestPV().ToJSON()),
-			expectedObjs:            toUnstructured(newTestPV().WithArkLabel("my-restore").PersistentVolume),
+			expectedObjs:            toUnstructured(newTestPV().PersistentVolume),
 		},
 		{
 			name:                    "namespaced resources are not skipped when IncludeClusterResources=true",
@@ -515,7 +516,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			labelSelector:           labels.NewSelector(),
 			includeClusterResources: truePtr,
 			fileSystem:              arktest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()),
-			expectedObjs:            toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap),
+			expectedObjs:            toUnstructured(newTestConfigMap().ConfigMap),
 		},
 		{
 			name:                    "cluster-scoped resources are not skipped when IncludeClusterResources=nil",
@@ -524,7 +525,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			labelSelector:           labels.NewSelector(),
 			includeClusterResources: nil,
 			fileSystem:              arktest.NewFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestPV().ToJSON()),
-			expectedObjs:            toUnstructured(newTestPV().WithArkLabel("my-restore").PersistentVolume),
+			expectedObjs:            toUnstructured(newTestPV().PersistentVolume),
 		},
 		{
 			name:                    "namespaced resources are not skipped when IncludeClusterResources=nil",
@@ -533,7 +534,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			labelSelector:           labels.NewSelector(),
 			includeClusterResources: nil,
 			fileSystem:              arktest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()),
-			expectedObjs:            toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap),
+			expectedObjs:            toUnstructured(newTestConfigMap().ConfigMap),
 		},
 		{
 			name:                    "serviceaccounts are restored",
@@ -542,7 +543,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			labelSelector:           labels.NewSelector(),
 			includeClusterResources: nil,
 			fileSystem:              arktest.NewFakeFileSystem().WithFile("serviceaccounts/sa-1.json", newTestServiceAccount().ToJSON()),
-			expectedObjs:            toUnstructured(newTestServiceAccount().WithArkLabel("my-restore").ServiceAccount),
+			expectedObjs:            toUnstructured(newTestServiceAccount().ServiceAccount),
 		},
 		{
 			name:                    "non-mirror pods are restored",
@@ -566,7 +567,6 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 					WithKind("Pod").
 					WithNamespace("ns-1").
 					WithName("pod1").
-					WithArkLabel("my-restore").
 					Unstructured),
 			},
 		},
@@ -594,6 +594,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			resourceClient := &arktest.FakeDynamicClient{}
 			for i := range test.expectedObjs {
+				addRestoreLabels(&test.expectedObjs[i], "my-restore", "my-backup")
 				resourceClient.On("Create", &test.expectedObjs[i]).Return(&test.expectedObjs[i], nil)
 			}
 
@@ -625,6 +626,7 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 					},
 					Spec: api.RestoreSpec{
 						IncludeClusterResources: test.includeClusterResources,
+						BackupName:              "my-backup",
 					},
 				},
 				backup:     &api.Backup{},
@@ -679,8 +681,7 @@ func TestRestoringExistingServiceAccount(t *testing.T) {
 				m[k] = v
 			}
 			fromBackupWithLabel := &unstructured.Unstructured{Object: m}
-			l := map[string]string{api.RestoreLabelKey: "my-restore"}
-			fromBackupWithLabel.SetLabels(l)
+			addRestoreLabels(fromBackupWithLabel, "my-restore", "my-backup")
 			// resetMetadataAndStatus will strip the creationTimestamp before calling Create
 			fromBackupWithLabel.SetCreationTimestamp(metav1.Time{Time: time.Time{}})
 
@@ -711,6 +712,7 @@ func TestRestoringExistingServiceAccount(t *testing.T) {
 					},
 					Spec: api.RestoreSpec{
 						IncludeClusterResources: nil,
+						BackupName:              "my-backup",
 					},
 				},
 				backup: &api.Backup{},
@@ -922,7 +924,7 @@ status:
 			}
 
 			resetMetadataAndStatus(unstructuredPV)
-			addLabel(unstructuredPV, api.RestoreLabelKey, ctx.restore.Name)
+			addRestoreLabels(unstructuredPV, ctx.restore.Name, ctx.restore.Spec.BackupName)
 			unstructuredPV.Object["foo"] = "bar"
 
 			if test.expectPVCreation {
@@ -964,7 +966,7 @@ status:
 			unstructuredPVC := &unstructured.Unstructured{Object: unstructuredPVCMap}
 
 			resetMetadataAndStatus(unstructuredPVC)
-			addLabel(unstructuredPVC, api.RestoreLabelKey, ctx.restore.Name)
+			addRestoreLabels(unstructuredPVC, ctx.restore.Name, ctx.restore.Spec.BackupName)
 
 			createdPVC := unstructuredPVC.DeepCopy()
 			// just to ensure we have the data flowing correctly
@@ -1431,17 +1433,6 @@ func (obj *testUnstructured) WithName(name string) *testUnstructured {
 	return obj.WithMetadataField("name", name)
 }
 
-func (obj *testUnstructured) WithArkLabel(restoreName string) *testUnstructured {
-	ls := obj.GetLabels()
-	if ls == nil {
-		ls = make(map[string]string)
-	}
-	ls[api.RestoreLabelKey] = restoreName
-	obj.SetLabels(ls)
-
-	return obj
-}
-
 func (obj *testUnstructured) ToJSON() []byte {
 	bytes, err := json.Marshal(obj.Object)
 	if err != nil {
@@ -1523,14 +1514,6 @@ func newTestServiceAccount() *testServiceAccount {
 	}
 }
 
-func (sa *testServiceAccount) WithArkLabel(restoreName string) *testServiceAccount {
-	if sa.Labels == nil {
-		sa.Labels = make(map[string]string)
-	}
-	sa.Labels[api.RestoreLabelKey] = restoreName
-	return sa
-}
-
 func (sa *testServiceAccount) WithImagePullSecret(name string) *testServiceAccount {
 	secret := v1.LocalObjectReference{Name: name}
 	sa.ImagePullSecrets = append(sa.ImagePullSecrets, secret)
@@ -1567,14 +1550,6 @@ func newTestPV() *testPersistentVolume {
 	}
 }
 
-func (pv *testPersistentVolume) WithArkLabel(restoreName string) *testPersistentVolume {
-	if pv.Labels == nil {
-		pv.Labels = make(map[string]string)
-	}
-	pv.Labels[api.RestoreLabelKey] = restoreName
-	return pv
-}
-
 func (pv *testPersistentVolume) ToJSON() []byte {
 	bytes, _ := json.Marshal(pv.PersistentVolume)
 	return bytes
@@ -1629,16 +1604,6 @@ func newNamedTestConfigMap(name string) *testConfigMap {
 	}
 }
 
-func (cm *testConfigMap) WithArkLabel(restoreName string) *testConfigMap {
-	if cm.Labels == nil {
-		cm.Labels = make(map[string]string)
-	}
-
-	cm.Labels[api.RestoreLabelKey] = restoreName
-
-	return cm
-}
-
 func (cm *testConfigMap) WithNamespace(name string) *testConfigMap {
 	cm.Namespace = name
 	return cm