Mark the existed resource as skipped during restore (#10243)

This commit makes sure the object is marked as "skipped" when there's
object with same name exists in the cluster during restore.  Otherwise,
such object will appeared as "failed" in the "Resource list" in the
output of "velero restore describe xxx --details"

Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>

changelogs/unreleased/10243-reasonerjt

@@ -0,0 +1 @@
+Mark the existed resource as skipped during restore

pkg/restore/restore.go

@@ -1954,6 +1954,8 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
 						e := errors.Errorf("could not restore, %s %q already exists. Warning: the in-cluster version is different than the backed-up version",
 							obj.GetKind(), obj.GetName())
 						warnings.Add(namespace, e)
+						itemStatus.action = ItemRestoreResultSkipped
+						ctx.restoredItems[itemKey] = itemStatus
 						// existingResourcePolicy is set as update, attempt patch on the resource and add warning if it fails
 					} else if resourcePolicy == velerov1api.PolicyTypeUpdate {
 						// processing update as existingResourcePolicy
@@ -1969,6 +1971,8 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
 					// Preserved Velero behavior when existingResourcePolicy is not specified by the user
 					e := errors.Errorf("could not restore, %s:%s already exists. Warning: the in-cluster version is different than the backed-up version",
 						obj.GetKind(), obj.GetName())
+					itemStatus.action = ItemRestoreResultSkipped
+					ctx.restoredItems[itemKey] = itemStatus
 					warnings.Add(namespace, e)
 				}
 			}

pkg/restore/restore_test.go

@@ -1086,6 +1086,7 @@ func TestRestoreItems(t *testing.T) {
 		apiResources         []*test.APIResource
 		tarball              io.Reader
 		want                 []*test.APIResource
+		wantWarnings         Result
 		expectedRestoreItems map[itemKey]restoredItemStatus
 		disableInformer      bool
 	}{
@@ -1328,6 +1329,52 @@ func TestRestoreItems(t *testing.T) {
 				test.Pods(builder.ForPod("ns-1", "sa-1").ObjectMeta(builder.WithLabels("velero.io/backup-name", "foo", "velero.io/restore-name", "bar")).Result()),
 			},
 		},
+		{
+			name:    "mark item as skipped when pod exists in cluster and is different from backed up one, existing resource policy is none",
+			restore: defaultRestore().ExistingResourcePolicy("none").Result(),
+			backup:  defaultBackup().Result(),
+			tarball: test.NewTarWriter(t).
+				AddItems("pods", builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("app", "backed-up")).Result()).
+				Done(),
+			apiResources: []*test.APIResource{
+				test.Pods(builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("app", "in-cluster")).Result()),
+			},
+			want: []*test.APIResource{
+				test.Pods(builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("app", "in-cluster")).Result()),
+			},
+			wantWarnings: Result{
+				Namespaces: map[string][]string{
+					"ns-1": {"could not restore, Pod \"pod-1\" already exists. Warning: the in-cluster version is different than the backed-up version"},
+				},
+			},
+			expectedRestoreItems: map[itemKey]restoredItemStatus{
+				{resource: "v1/Namespace", namespace: "", name: "ns-1"}: {action: "created", itemExists: true, createdName: "ns-1"},
+				{resource: "v1/Pod", namespace: "ns-1", name: "pod-1"}:  {action: "skipped", itemExists: true},
+			},
+		},
+		{
+			name:    "mark item as skipped when pod exists in cluster and is different from backed up one, existing resource policy is not specified",
+			restore: defaultRestore().Result(),
+			backup:  defaultBackup().Result(),
+			tarball: test.NewTarWriter(t).
+				AddItems("pods", builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("app", "backed-up")).Result()).
+				Done(),
+			apiResources: []*test.APIResource{
+				test.Pods(builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("app", "in-cluster")).Result()),
+			},
+			want: []*test.APIResource{
+				test.Pods(builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("app", "in-cluster")).Result()),
+			},
+			wantWarnings: Result{
+				Namespaces: map[string][]string{
+					"ns-1": {"could not restore, Pod:pod-1 already exists. Warning: the in-cluster version is different than the backed-up version"},
+				},
+			},
+			expectedRestoreItems: map[itemKey]restoredItemStatus{
+				{resource: "v1/Namespace", namespace: "", name: "ns-1"}: {action: "created", itemExists: true, createdName: "ns-1"},
+				{resource: "v1/Pod", namespace: "ns-1", name: "pod-1"}:  {action: "skipped", itemExists: true},
+			},
+		},
 		{
 			name:    "service account secrets and image pull secrets are restored when service account already exists in cluster",
 			restore: defaultRestore().Result(),
@@ -1437,7 +1484,12 @@ func TestRestoreItems(t *testing.T) {
 				nil, // volume snapshotter getter
 			)
 
-			assertEmptyResults(t, warnings, errs)
+			if tc.wantWarnings.IsEmpty() {
+				assertEmptyResults(t, warnings)
+			} else {
+				assertWantErrsOrWarnings(t, tc.wantWarnings, warnings)
+			}
+			assertEmptyResults(t, errs)
 			assertRestoredItems(t, h, tc.want)
 			if len(tc.expectedRestoreItems) > 0 {
 				assert.Equal(t, tc.expectedRestoreItems, data.RestoredItems)