From 8c3ddf0f730845157aa5a062653d91052f0d75a3 Mon Sep 17 00:00:00 2001
From: Anshul Ahuja <anshulahuja@microsoft.com>
Date: Thu, 16 Feb 2023 09:57:53 +0530
Subject: [PATCH] Publish backup results extracted from backup logs

Signed-off-by: Anshul Ahuja <anshulahuja@microsoft.com>
---
 changelogs/unreleased/5576-anshulahuja98     |  1 +
 pkg/cmd/util/output/restore_describer.go     |  6 +-
 pkg/controller/backup_controller.go          | 21 +++++-
 pkg/controller/restore_controller.go         |  5 +-
 pkg/controller/restore_controller_test.go    | 11 ++--
 pkg/persistence/object_store.go              |  2 +
 pkg/persistence/object_store_layout.go       |  4 ++
 pkg/restore/restore.go                       |  1 +
 pkg/restore/restore_test.go                  |  1 +
 pkg/util/logging/log_counter_hook.go         | 67 ++++++++++++++++----
 pkg/{restore => util/results}/result.go      | 10 +--
 pkg/{restore => util/results}/result_test.go |  2 +-
 12 files changed, 101 insertions(+), 30 deletions(-)
 create mode 100644 changelogs/unreleased/5576-anshulahuja98
 rename pkg/{restore => util/results}/result.go (89%)
 rename pkg/{restore => util/results}/result_test.go (99%)

diff --git a/changelogs/unreleased/5576-anshulahuja98 b/changelogs/unreleased/5576-anshulahuja98
new file mode 100644
index 0000000000..fd8fc7210b
--- /dev/null
+++ b/changelogs/unreleased/5576-anshulahuja98
@@ -0,0 +1 @@
+Publish backupresults json to enhance error info during backups.
\ No newline at end of file
diff --git a/pkg/cmd/util/output/restore_describer.go b/pkg/cmd/util/output/restore_describer.go
index 631dbb2cb0..de249c969e 100644
--- a/pkg/cmd/util/output/restore_describer.go
+++ b/pkg/cmd/util/output/restore_describer.go
@@ -32,7 +32,7 @@ import (
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest"
 	clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
-	pkgrestore "github.com/vmware-tanzu/velero/pkg/restore"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
 )
 
 func DescribeRestore(ctx context.Context, kbClient kbclient.Client, restore *velerov1api.Restore, podVolumeRestores []velerov1api.PodVolumeRestore, details bool, veleroClient clientset.Interface, insecureSkipTLSVerify bool, caCertFile string) string {
@@ -167,7 +167,7 @@ func describeRestoreResults(ctx context.Context, kbClient kbclient.Client, d *De
 	}
 
 	var buf bytes.Buffer
-	var resultMap map[string]pkgrestore.Result
+	var resultMap map[string]results.Result
 
 	if err := downloadrequest.Stream(ctx, kbClient, restore.Namespace, restore.Name, velerov1api.DownloadTargetKindRestoreResults, &buf, downloadRequestTimeout, insecureSkipTLSVerify, caCertPath); err != nil {
 		d.Printf("Warnings:\t<error getting warnings: %v>\n\nErrors:\t<error getting errors: %v>\n", err, err)
@@ -189,7 +189,7 @@ func describeRestoreResults(ctx context.Context, kbClient kbclient.Client, d *De
 	}
 }
 
-func describeRestoreResult(d *Describer, name string, result pkgrestore.Result) {
+func describeRestoreResult(d *Describer, name string, result results.Result) {
 	d.Printf("%s:\n", name)
 	d.DescribeSlice(1, "Velero", result.Velero)
 	d.DescribeSlice(1, "Cluster", result.Cluster)
diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go
index 2765d076ae..38e19361c4 100644
--- a/pkg/controller/backup_controller.go
+++ b/pkg/controller/backup_controller.go
@@ -43,6 +43,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/tools/cache"
 
+	"github.com/vmware-tanzu/velero/pkg/util/results"
+
 	"github.com/vmware-tanzu/velero/pkg/util/csi"
 
 	snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
@@ -603,7 +605,7 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 	logger := logging.DefaultLogger(c.backupLogLevel, c.formatFlag)
 	logger.Out = io.MultiWriter(os.Stdout, gzippedLogFile)
 
-	logCounter := logging.NewLogCounterHook()
+	logCounter := logging.NewLogHook()
 	logger.Hooks.Add(logCounter)
 
 	backupLog := logger.WithField(Backup, kubeutil.NamespaceAndName(backup))
@@ -725,6 +727,13 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 
 	recordBackupMetrics(backupLog, backup.Backup, backupFile, c.metrics)
 
+	backupWarnings := logCounter.GetEntries(logrus.WarnLevel)
+	backupErrors := logCounter.GetEntries(logrus.ErrorLevel)
+	results := map[string]results.Result{
+		"warnings": backupWarnings,
+		"errors":   backupErrors,
+	}
+
 	if err := gzippedLogFile.Close(); err != nil {
 		c.logger.WithField(Backup, kubeutil.NamespaceAndName(backup)).WithError(err).Error("error closing gzippedLogFile")
 	}
@@ -750,7 +759,7 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 		return err
 	}
 
-	if errs := persistBackup(backup, backupFile, logFile, backupStore, volumeSnapshots, volumeSnapshotContents, volumeSnapshotClasses); len(errs) > 0 {
+	if errs := persistBackup(backup, backupFile, logFile, backupStore, volumeSnapshots, volumeSnapshotContents, volumeSnapshotClasses, results); len(errs) > 0 {
 		fatalErrs = append(fatalErrs, errs...)
 	}
 
@@ -797,6 +806,7 @@ func persistBackup(backup *pkgbackup.Request,
 	csiVolumeSnapshots []snapshotv1api.VolumeSnapshot,
 	csiVolumeSnapshotContents []snapshotv1api.VolumeSnapshotContent,
 	csiVolumesnapshotClasses []snapshotv1api.VolumeSnapshotClass,
+	results map[string]results.Result,
 ) []error {
 	persistErrs := []error{}
 	backupJSON := new(bytes.Buffer)
@@ -835,6 +845,11 @@ func persistBackup(backup *pkgbackup.Request,
 		persistErrs = append(persistErrs, errs...)
 	}
 
+	backupResult, errs := encodeToJSONGzip(results, "backup results")
+	if errs != nil {
+		persistErrs = append(persistErrs, errs...)
+	}
+
 	if len(persistErrs) > 0 {
 		// Don't upload the JSON files or backup tarball if encoding to json fails.
 		backupJSON = nil
@@ -844,6 +859,7 @@ func persistBackup(backup *pkgbackup.Request,
 		csiSnapshotJSON = nil
 		csiSnapshotContentsJSON = nil
 		csiSnapshotClassesJSON = nil
+		backupResult = nil
 	}
 
 	backupInfo := persistence.BackupInfo{
@@ -851,6 +867,7 @@ func persistBackup(backup *pkgbackup.Request,
 		Metadata:                  backupJSON,
 		Contents:                  backupContents,
 		Log:                       backupLog,
+		BackupResults:             backupResult,
 		PodVolumeBackups:          podVolumeBackups,
 		VolumeSnapshots:           nativeVolumeSnapshots,
 		BackupResourceList:        backupResourceList,
diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go
index 038e971095..a560ba8d64 100644
--- a/pkg/controller/restore_controller.go
+++ b/pkg/controller/restore_controller.go
@@ -53,6 +53,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/util/collections"
 	kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
 	"github.com/vmware-tanzu/velero/pkg/util/logging"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -572,7 +573,7 @@ func (c *restoreController) runValidatedRestore(restore *api.Restore, info backu
 		restore.Status.Errors += len(e)
 	}
 
-	m := map[string]pkgrestore.Result{
+	m := map[string]results.Result{
 		"warnings": restoreWarnings,
 		"errors":   restoreErrors,
 	}
@@ -584,7 +585,7 @@ func (c *restoreController) runValidatedRestore(restore *api.Restore, info backu
 	return nil
 }
 
-func putResults(restore *api.Restore, results map[string]pkgrestore.Result, backupStore persistence.BackupStore) error {
+func putResults(restore *api.Restore, results map[string]results.Result, backupStore persistence.BackupStore) error {
 	buf := new(bytes.Buffer)
 	gzw := gzip.NewWriter(buf)
 	defer gzw.Close()
diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go
index fff95340e5..6a752c0d8f 100644
--- a/pkg/controller/restore_controller_test.go
+++ b/pkg/controller/restore_controller_test.go
@@ -51,6 +51,7 @@ import (
 	pkgrestore "github.com/vmware-tanzu/velero/pkg/restore"
 	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 	"github.com/vmware-tanzu/velero/pkg/util/logging"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
 	"github.com/vmware-tanzu/velero/pkg/volume"
 )
 
@@ -506,7 +507,7 @@ func TestProcessQueueItem(t *testing.T) {
 				sharedInformers.Velero().V1().Backups().Informer().GetStore().Add(test.backup)
 			}
 
-			var warnings, errors pkgrestore.Result
+			var warnings, errors results.Result
 			if test.restorerError != nil {
 				errors.Namespaces = map[string][]string{"ns-1": {test.restorerError.Error()}}
 			}
@@ -864,12 +865,12 @@ func (r *fakeRestorer) Restore(
 	actions []riav1.RestoreItemAction,
 	snapshotLocationLister listers.VolumeSnapshotLocationLister,
 	volumeSnapshotterGetter pkgrestore.VolumeSnapshotterGetter,
-) (pkgrestore.Result, pkgrestore.Result) {
+) (results.Result, results.Result) {
 	res := r.Called(info.Log, info.Restore, info.Backup, info.BackupReader, actions)
 
 	r.calledWithArg = *info.Restore
 
-	return res.Get(0).(pkgrestore.Result), res.Get(1).(pkgrestore.Result)
+	return res.Get(0).(results.Result), res.Get(1).(results.Result)
 }
 
 func (r *fakeRestorer) RestoreWithResolvers(req pkgrestore.Request,
@@ -877,11 +878,11 @@ func (r *fakeRestorer) RestoreWithResolvers(req pkgrestore.Request,
 	itemSnapshotterResolver framework.ItemSnapshotterResolver,
 	snapshotLocationLister listers.VolumeSnapshotLocationLister,
 	volumeSnapshotterGetter pkgrestore.VolumeSnapshotterGetter,
-) (pkgrestore.Result, pkgrestore.Result) {
+) (results.Result, results.Result) {
 	res := r.Called(req.Log, req.Restore, req.Backup, req.BackupReader, resolver, itemSnapshotterResolver,
 		snapshotLocationLister, volumeSnapshotterGetter)
 
 	r.calledWithArg = *req.Restore
 
-	return res.Get(0).(pkgrestore.Result), res.Get(1).(pkgrestore.Result)
+	return res.Get(0).(results.Result), res.Get(1).(results.Result)
 }
diff --git a/pkg/persistence/object_store.go b/pkg/persistence/object_store.go
index acda15323a..dc560cd536 100644
--- a/pkg/persistence/object_store.go
+++ b/pkg/persistence/object_store.go
@@ -42,6 +42,7 @@ type BackupInfo struct {
 	Metadata,
 	Contents,
 	Log,
+	BackupResults,
 	PodVolumeBackups,
 	VolumeSnapshots,
 	ItemSnapshots,
@@ -261,6 +262,7 @@ func (s *objectBackupStore) PutBackup(info BackupInfo) error {
 		s.layout.getCSIVolumeSnapshotKey(info.Name):         info.CSIVolumeSnapshots,
 		s.layout.getCSIVolumeSnapshotContentsKey(info.Name): info.CSIVolumeSnapshotContents,
 		s.layout.getCSIVolumeSnapshotClassesKey(info.Name):  info.CSIVolumeSnapshotClasses,
+		s.layout.getBackupResultsKey(info.Name):             info.BackupResults,
 	}
 
 	for key, reader := range backupObjs {
diff --git a/pkg/persistence/object_store_layout.go b/pkg/persistence/object_store_layout.go
index 7042c40b37..12685b15cb 100644
--- a/pkg/persistence/object_store_layout.go
+++ b/pkg/persistence/object_store_layout.go
@@ -116,3 +116,7 @@ func (l *ObjectStoreLayout) getCSIVolumeSnapshotContentsKey(backup string) strin
 func (l *ObjectStoreLayout) getCSIVolumeSnapshotClassesKey(backup string) string {
 	return path.Join(l.subdirs["backups"], backup, fmt.Sprintf("%s-csi-volumesnapshotclasses.json.gz", backup))
 }
+
+func (l *ObjectStoreLayout) getBackupResultsKey(backup string) string {
+	return path.Join(l.subdirs["backups"], backup, fmt.Sprintf("%s-results.gz", backup))
+}
diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go
index bea58fafc4..1e7abd9c95 100644
--- a/pkg/restore/restore.go
+++ b/pkg/restore/restore.go
@@ -67,6 +67,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/util/collections"
 	"github.com/vmware-tanzu/velero/pkg/util/filesystem"
 	"github.com/vmware-tanzu/velero/pkg/util/kube"
+	. "github.com/vmware-tanzu/velero/pkg/util/results"
 	"github.com/vmware-tanzu/velero/pkg/volume"
 )
 
diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
index c8bd37ad50..83e6c64e3b 100644
--- a/pkg/restore/restore_test.go
+++ b/pkg/restore/restore_test.go
@@ -56,6 +56,7 @@ import (
 	testutil "github.com/vmware-tanzu/velero/pkg/test"
 	"github.com/vmware-tanzu/velero/pkg/util/kube"
 	kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
+	. "github.com/vmware-tanzu/velero/pkg/util/results"
 	"github.com/vmware-tanzu/velero/pkg/volume"
 )
 
diff --git a/pkg/util/logging/log_counter_hook.go b/pkg/util/logging/log_counter_hook.go
index d8d53359c1..a14b790f63 100644
--- a/pkg/util/logging/log_counter_hook.go
+++ b/pkg/util/logging/log_counter_hook.go
@@ -17,45 +17,88 @@ limitations under the License.
 package logging
 
 import (
+	"errors"
+	"fmt"
 	"sync"
 
 	"github.com/sirupsen/logrus"
+
+	"github.com/vmware-tanzu/velero/pkg/util/results"
 )
 
-// LogCounterHook is a logrus hook that counts the number of log
-// statements that have been written at each logrus level.
-type LogCounterHook struct {
-	mu     sync.RWMutex
-	counts map[logrus.Level]int
+// LogHook is a logrus hook that counts the number of log
+// statements that have been written at each logrus level. It also
+// maintains log entries at each logrus level in result structure.
+type LogHook struct {
+	mu      sync.RWMutex
+	counts  map[logrus.Level]int
+	entries map[logrus.Level]*results.Result
 }
 
-// NewLogCounterHook returns a pointer to an initialized LogCounterHook.
-func NewLogCounterHook() *LogCounterHook {
-	return &LogCounterHook{
-		counts: make(map[logrus.Level]int),
+// NewLogHook returns a pointer to an initialized LogHook.
+func NewLogHook() *LogHook {
+	return &LogHook{
+		counts:  make(map[logrus.Level]int),
+		entries: make(map[logrus.Level]*results.Result),
 	}
 }
 
 // Levels returns the logrus levels that the hook should be fired for.
-func (h *LogCounterHook) Levels() []logrus.Level {
+func (h *LogHook) Levels() []logrus.Level {
 	return logrus.AllLevels
 }
 
 // Fire executes the hook's logic.
-func (h *LogCounterHook) Fire(entry *logrus.Entry) error {
+func (h *LogHook) Fire(entry *logrus.Entry) error {
 	h.mu.Lock()
 	defer h.mu.Unlock()
 
 	h.counts[entry.Level]++
+	if h.entries[entry.Level] == nil {
+		h.entries[entry.Level] = &results.Result{}
+	}
+
+	namespace, isNamespacePresent := entry.Data["namespace"]
+	errorField, isErrorFieldPresent := entry.Data["error"]
+	resourceField, isResourceFieldPresent := entry.Data["resource"]
+	nameField, isNameFieldPresent := entry.Data["name"]
+
+	entryMessage := ""
+	if isResourceFieldPresent {
+		entryMessage = fmt.Sprintf("%s resource: /%s", entryMessage, resourceField.(string))
+	}
+	if isNameFieldPresent {
+		entryMessage = fmt.Sprintf("%s name: /%s", entryMessage, nameField.(string))
+	}
+	if isErrorFieldPresent {
+		entryMessage = fmt.Sprintf("%s error: /%s", entryMessage, errorField.(error).Error())
+	}
 
+	if isNamespacePresent {
+		h.entries[entry.Level].Add(namespace.(string), errors.New(entryMessage))
+	} else {
+		h.entries[entry.Level].AddVeleroError(errors.New(entryMessage))
+	}
 	return nil
 }
 
 // GetCount returns the number of log statements that have been
 // written at the specific level provided.
-func (h *LogCounterHook) GetCount(level logrus.Level) int {
+func (h *LogHook) GetCount(level logrus.Level) int {
 	h.mu.RLock()
 	defer h.mu.RUnlock()
 
 	return h.counts[level]
 }
+
+// GetEntries returns the log statements that have been
+// written at the specific level provided.
+func (h *LogHook) GetEntries(level logrus.Level) results.Result {
+	h.mu.RLock()
+	defer h.mu.RUnlock()
+	response, isPresent := h.entries[level]
+	if isPresent {
+		return *response
+	}
+	return results.Result{}
+}
diff --git a/pkg/restore/result.go b/pkg/util/results/result.go
similarity index 89%
rename from pkg/restore/result.go
rename to pkg/util/results/result.go
index e8e1e7b30c..f211b8b178 100644
--- a/pkg/restore/result.go
+++ b/pkg/util/results/result.go
@@ -14,10 +14,10 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package restore
+package results
 
 // Result is a collection of messages that were generated during
-// execution of a restore. This will typically store either
+// execution of a backup or restore. This will typically store either
 // warning or error messages.
 type Result struct {
 	// Velero is a slice of messages related to the operation of Velero
@@ -25,12 +25,12 @@ type Result struct {
 	// cloud, reading a backup file, etc.)
 	Velero []string `json:"velero,omitempty"`
 
-	// Cluster is a slice of messages related to restoring cluster-
-	// scoped resources.
+	// Cluster is a slice of messages related to backup or restore of
+	// cluster-scoped resources.
 	Cluster []string `json:"cluster,omitempty"`
 
 	// Namespaces is a map of namespace name to slice of messages
-	// related to restoring namespace-scoped resources.
+	// related to backup or restore namespace-scoped resources.
 	Namespaces map[string][]string `json:"namespaces,omitempty"`
 }
 
diff --git a/pkg/restore/result_test.go b/pkg/util/results/result_test.go
similarity index 99%
rename from pkg/restore/result_test.go
rename to pkg/util/results/result_test.go
index c710372245..f447c9d1e7 100644
--- a/pkg/restore/result_test.go
+++ b/pkg/util/results/result_test.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package restore
+package results
 
 import (
 	"testing"