don't error during backup when additional items returned by plugin don't exist (#2595)

* log a warning instead of erroring if additional item can't be found

Signed-off-by: Steve Kriss <krisss@vmware.com>

* always show backup warning/error count in get/describe

Signed-off-by: Steve Kriss <krisss@vmware.com>

* changelog

Signed-off-by: Steve Kriss <krisss@vmware.com>

changelogs/unreleased/2595-skriss

@@ -0,0 +1 @@
+log a warning instead of erroring if an additional item returned from a plugin can't be found in the Kubernetes API

pkg/backup/backup_test.go

@@ -1723,7 +1723,7 @@ func TestBackupActionAdditionalItems(t *testing.T) {
 		},
 
 		{
-			name:   "if there's an error backing up additional items, the item the action was run for isn't backed up",
+			name:   "if additional items aren't found in the API, they're skipped and the original item is still backed up",
 			backup: defaultBackup().Result(),
 			apiResources: []*test.APIResource{
 				test.Pods(
@@ -1746,8 +1746,10 @@ func TestBackupActionAdditionalItems(t *testing.T) {
 				},
 			},
 			want: []string{
+				"resources/pods/namespaces/ns-1/pod-1.json",
 				"resources/pods/namespaces/ns-2/pod-2.json",
 				"resources/pods/namespaces/ns-3/pod-3.json",
+				"resources/pods/v1-preferredversion/namespaces/ns-1/pod-1.json",
 				"resources/pods/v1-preferredversion/namespaces/ns-2/pod-2.json",
 				"resources/pods/v1-preferredversion/namespaces/ns-3/pod-3.json",
 			},

pkg/backup/item_backupper.go

@@ -26,6 +26,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	corev1api "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -339,12 +340,20 @@ func (ib *itemBackupper) executeActions(
 				return nil, err
 			}
 
-			additionalItem, err := client.Get(additionalItem.Name, metav1.GetOptions{})
+			item, err := client.Get(additionalItem.Name, metav1.GetOptions{})
+			if apierrors.IsNotFound(err) {
+				log.WithFields(logrus.Fields{
+					"groupResource": additionalItem.GroupResource,
+					"namespace":     additionalItem.Namespace,
+					"name":          additionalItem.Name,
+				}).Warnf("Additional item was not found in Kubernetes API, can't back it up")
+				continue
+			}
 			if err != nil {
 				return nil, errors.WithStack(err)
 			}
 
-			if _, err = ib.backupItem(log, additionalItem, gvr.GroupResource(), gvr); err != nil {
+			if _, err = ib.backupItem(log, item, gvr.GroupResource(), gvr); err != nil {
 				return nil, err
 			}
 		}

pkg/cmd/util/output/backup_describer.go

@@ -70,11 +70,9 @@ func DescribeBackup(
 			}
 		}
 
-		if status.Phase == velerov1api.BackupPhasePartiallyFailed {
-			d.Println()
-			d.Printf("Errors:\t%d\n", status.Errors)
-			d.Printf("Warnings:\t%d\n", status.Warnings)
-		}
+		d.Println()
+		d.Printf("Errors:\t%d\n", status.Errors)
+		d.Printf("Warnings:\t%d\n", status.Warnings)
 
 		d.Println()
 		DescribeBackupSpec(d, backup.Spec)

pkg/cmd/util/output/backup_printer.go

@@ -35,6 +35,8 @@ var (
 		// https://github.com/kubernetes/kubernetes/blob/v1.15.3/pkg/printers/tableprinter.go#L204
 		{Name: "Name", Type: "string", Format: "name"},
 		{Name: "Status"},
+		{Name: "Errors"},
+		{Name: "Warnings"},
 		{Name: "Created"},
 		{Name: "Expires"},
 		{Name: "Storage Location"},
@@ -58,7 +60,6 @@ func printBackupList(list *velerov1api.BackupList) []metav1.TableRow {
 var timestampSuffix = regexp.MustCompile("-[0-9]{14}$")
 
 func sortBackupsByPrefixAndTimestamp(list *velerov1api.BackupList) {
-
 	sort.Slice(list.Items, func(i, j int) bool {
 		iSuffixIndex := timestampSuffix.FindStringIndex(list.Items[i].Name)
 		jSuffixIndex := timestampSuffix.FindStringIndex(list.Items[j].Name)
@@ -98,18 +99,17 @@ func printBackup(backup *velerov1api.Backup) []metav1.TableRow {
 	if backup.DeletionTimestamp != nil && !backup.DeletionTimestamp.Time.IsZero() {
 		status = "Deleting"
 	}
-	if status == string(velerov1api.BackupPhasePartiallyFailed) {
-		if backup.Status.Errors == 1 {
-			status = fmt.Sprintf("%s (1 error)", status)
-		} else {
-			status = fmt.Sprintf("%s (%d errors)", status, backup.Status.Errors)
-		}
 
-	}
-
-	location := backup.Spec.StorageLocation
-
-	row.Cells = append(row.Cells, backup.Name, status, backup.Status.StartTimestamp, humanReadableTimeFromNow(expiration), location, metav1.FormatLabelSelector(backup.Spec.LabelSelector))
+	row.Cells = append(row.Cells,
+		backup.Name,
+		status,
+		backup.Status.Errors,
+		backup.Status.Warnings,
+		backup.Status.StartTimestamp,
+		humanReadableTimeFromNow(expiration),
+		backup.Spec.StorageLocation,
+		metav1.FormatLabelSelector(backup.Spec.LabelSelector),
+	)
 
 	return []metav1.TableRow{row}
 }

pkg/cmd/util/output/restore_printer.go

@@ -30,8 +30,8 @@ var (
 		{Name: "Name", Type: "string", Format: "name"},
 		{Name: "Backup"},
 		{Name: "Status"},
-		{Name: "Warnings"},
 		{Name: "Errors"},
+		{Name: "Warnings"},
 		{Name: "Created"},
 		{Name: "Selector"},
 	}
@@ -60,8 +60,8 @@ func printRestore(restore *v1.Restore) []metav1.TableRow {
 		restore.Name,
 		restore.Spec.BackupName,
 		status,
-		restore.Status.Warnings,
 		restore.Status.Errors,
+		restore.Status.Warnings,
 		restore.CreationTimestamp.Time,
 		metav1.FormatLabelSelector(restore.Spec.LabelSelector),
 	)