Merge pull request vmware-tanzu#5576 from anshulahuja98/backupresults

Publish backup results to enhance error info

changelogs/unreleased/5576-anshulahuja98

@@ -0,0 +1 @@
+Publish backupresults json to enhance error info during backups.

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)

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"
@@ -607,7 +609,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))
@@ -729,6 +731,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")
 	}
@@ -754,7 +763,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...)
 	}
 
@@ -805,6 +814,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)
@@ -843,6 +853,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
@@ -852,13 +867,15 @@ func persistBackup(backup *pkgbackup.Request,
 		csiSnapshotJSON = nil
 		csiSnapshotContentsJSON = nil
 		csiSnapshotClassesJSON = nil
+		backupResult = nil
 	}
 
 	backupInfo := persistence.BackupInfo{
 		Name:                      backup.Name,
 		Metadata:                  backupJSON,
 		Contents:                  backupContents,
 		Log:                       backupLog,
+		BackupResults:             backupResult,
 		PodVolumeBackups:          podVolumeBackups,
 		VolumeSnapshots:           nativeVolumeSnapshots,
 		BackupResourceList:        backupResourceList,

pkg/controller/restore_controller.go

@@ -50,6 +50,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()

pkg/controller/restore_controller_test.go

@@ -47,6 +47,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"
 )
 
@@ -450,7 +451,7 @@ func TestProcessQueueItem(t *testing.T) {
 				require.NoError(t, c.kbClient.Create(context.Background(), test.restore))
 			}
 
-			var warnings, errors pkgrestore.Result
+			var warnings, errors results.Result
 			if test.restorerError != nil {
 				errors.Namespaces = map[string][]string{"ns-1": {test.restorerError.Error()}}
 			}
@@ -794,23 +795,23 @@ func (r *fakeRestorer) Restore(
 	info pkgrestore.Request,
 	actions []riav2.RestoreItemAction,
 	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,
 	resolver framework.RestoreItemActionResolverV2,
 	itemSnapshotterResolver framework.ItemSnapshotterResolver,
 	volumeSnapshotterGetter pkgrestore.VolumeSnapshotterGetter,
-) (pkgrestore.Result, pkgrestore.Result) {
+) (results.Result, results.Result) {
 	res := r.Called(req.Log, req.Restore, req.Backup, req.BackupReader, resolver, itemSnapshotterResolver,
 		r.kbClient, 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)
 }

pkg/persistence/object_store.go

@@ -43,6 +43,7 @@ type BackupInfo struct {
 	Metadata,
 	Contents,
 	Log,
+	BackupResults,
 	PodVolumeBackups,
 	VolumeSnapshots,
 	BackupItemOperations,
@@ -265,6 +266,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 {

pkg/persistence/object_store_layout.go

@@ -120,3 +120,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))
+}

pkg/restore/restore.go

@@ -65,6 +65,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"
 )
 

pkg/restore/restore_test.go

@@ -56,6 +56,7 @@ import (
 	velerotest "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"
 )
 

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{}
+}

pkg/restore/result.go → pkg/util/results/result.go

@@ -14,23 +14,23 @@ 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
 	// itself (for example, messages related to connecting to the
 	// 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"`
 }
 

pkg/restore/result_test.go → 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"