Improve `describe` output

* Move Phase to right under Metadata(name/namespace/label/annotations)
 * Move Validation errors: section right after Phase: section and only
   show it if the item has a phase of FailedValidation
 * For restores move Warnings and Errors under Validation errors. Do not
   show Warnings or Errors if there are none.

Signed-off-by: DheerajSShetty <dheerajs@vmware.com>

Fixes #987

changelogs/unreleased/1248-DheerajSShetty

@@ -0,0 +1,6 @@
+Improve `describe` output
+   * Move Phase to right under Metadata(name/namespace/label/annotations)
+   * Move Validation errors: section right after Phase: section and only
+     show it if the item has a phase of FailedValidation
+   * For restores move Warnings and Errors under Validation errors. Leave
+     their display as is.

pkg/cmd/util/output/backup_describer.go

@@ -49,6 +49,15 @@ func DescribeBackup(
 		}
 		d.Printf("Phase:\t%s\n", phase)
 
+		status := backup.Status
+		if len(status.ValidationErrors) > 0 {
+			d.Println()
+			d.Printf("Validation errors:")
+			for _, ve := range status.ValidationErrors {
+				d.Printf("\t%s\n", ve)
+			}
+		}
+
 		d.Println()
 		DescribeBackupSpec(d, backup.Spec)
 
@@ -201,16 +210,6 @@ func DescribeBackupStatus(d *Describer, backup *velerov1api.Backup, details bool
 	d.Printf("Expiration:\t%s\n", status.Expiration.Time)
 	d.Println()
 
-	d.Printf("Validation errors:")
-	if len(status.ValidationErrors) == 0 {
-		d.Printf("\t<none>\n")
-	} else {
-		for _, ve := range status.ValidationErrors {
-			d.Printf("\t%s\n", ve)
-		}
-	}
-
-	d.Println()
 	if len(status.VolumeBackups) > 0 {
 		// pre-v0.10 backup
 		d.Printf("Persistent Volumes:\n")

pkg/cmd/util/output/restore_describer.go

@@ -33,6 +33,19 @@ func DescribeRestore(restore *v1.Restore, podVolumeRestores []v1.PodVolumeRestor
 	return Describe(func(d *Describer) {
 		d.DescribeMetadata(restore.ObjectMeta)
 
+		d.Println()
+		d.Printf("Phase:\t%s\n", restore.Status.Phase)
+
+		if len(restore.Status.ValidationErrors) > 0 {
+			d.Println()
+			d.Printf("Validation errors:")
+			for _, ve := range restore.Status.ValidationErrors {
+				d.Printf("\t%s\n", ve)
+			}
+		}
+
+		describeRestoreResults(d, restore, veleroClient)
+
 		d.Println()
 		d.Printf("Backup:\t%s\n", restore.Spec.BackupName)
 
@@ -82,22 +95,6 @@ func DescribeRestore(restore *v1.Restore, podVolumeRestores []v1.PodVolumeRestor
 		d.Println()
 		d.Printf("Restore PVs:\t%s\n", BoolPointerString(restore.Spec.RestorePVs, "false", "true", "auto"))
 
-		d.Println()
-		d.Printf("Phase:\t%s\n", restore.Status.Phase)
-
-		d.Println()
-		d.Printf("Validation errors:")
-		if len(restore.Status.ValidationErrors) == 0 {
-			d.Printf("\t<none>\n")
-		} else {
-			for _, ve := range restore.Status.ValidationErrors {
-				d.Printf("\t%s\n", ve)
-			}
-		}
-
-		d.Println()
-		describeRestoreResults(d, restore, veleroClient)
-
 		if len(podVolumeRestores) > 0 {
 			d.Println()
 			describePodVolumeRestores(d, podVolumeRestores, details)
@@ -107,7 +104,6 @@ func DescribeRestore(restore *v1.Restore, podVolumeRestores []v1.PodVolumeRestor
 
 func describeRestoreResults(d *Describer, restore *v1.Restore, veleroClient clientset.Interface) {
 	if restore.Status.Warnings == 0 && restore.Status.Errors == 0 {
-		d.Printf("Warnings:\t<none>\nErrors:\t<none>\n")
 		return
 	}
 
@@ -124,9 +120,14 @@ func describeRestoreResults(d *Describer, restore *v1.Restore, veleroClient clie
 		return
 	}
 
-	describeRestoreResult(d, "Warnings", resultMap["warnings"])
-	d.Println()
-	describeRestoreResult(d, "Errors", resultMap["errors"])
+	if restore.Status.Warnings > 0 {
+		d.Println()
+		describeRestoreResult(d, "Warnings", resultMap["warnings"])
+	}
+	if restore.Status.Errors > 0 {
+		d.Println()
+		describeRestoreResult(d, "Errors", resultMap["errors"])
+	}
 }
 
 func describeRestoreResult(d *Describer, name string, result v1.RestoreResult) {

pkg/cmd/util/output/schedule_describer.go

@@ -26,6 +26,22 @@ func DescribeSchedule(schedule *v1.Schedule) string {
 	return Describe(func(d *Describer) {
 		d.DescribeMetadata(schedule.ObjectMeta)
 
+		d.Println()
+		phase := schedule.Status.Phase
+		if phase == "" {
+			phase = v1.SchedulePhaseNew
+		}
+		d.Printf("Phase:\t%s\n", phase)
+
+		status := schedule.Status
+		if len(status.ValidationErrors) > 0 {
+			d.Println()
+			d.Printf("Validation errors:")
+			for _, ve := range status.ValidationErrors {
+				d.Printf("\t%s\n", ve)
+			}
+		}
+
 		d.Println()
 		DescribeScheduleSpec(d, schedule.Spec)
 
@@ -45,21 +61,6 @@ func DescribeScheduleSpec(d *Describer, spec v1.ScheduleSpec) {
 }
 
 func DescribeScheduleStatus(d *Describer, status v1.ScheduleStatus) {
-	phase := status.Phase
-	if phase == "" {
-		phase = v1.SchedulePhaseNew
-	}
-
-	d.Printf("Validation errors:")
-	if len(status.ValidationErrors) == 0 {
-		d.Printf("\t<none>\n")
-	} else {
-		for _, ve := range status.ValidationErrors {
-			d.Printf("\t%s\n", ve)
-		}
-	}
-
-	d.Println()
 	lastBackup := "<never>"
 	if !status.LastBackup.Time.IsZero() {
 		lastBackup = fmt.Sprintf("%v", status.LastBackup.Time)