feat: better error message for sync operations (#336)

feat: better error message for sync operations (#336)

Signed-off-by: pashavictorovich <[email protected]>

pkg/sync/sync_context.go

@@ -450,10 +450,12 @@ func (sc *syncContext) Sync() {
 	// syncFailTasks only run during failure, so separate them from regular tasks
 	syncFailTasks, tasks := tasks.Split(func(t *syncTask) bool { return t.phase == common.SyncPhaseSyncFail })
 
+	syncFailedTasks, _ := tasks.Split(func(t *syncTask) bool { return t.syncStatus == common.ResultCodeSyncFailed })
+
 	// if there are any completed but unsuccessful tasks, sync is a failure.
 	if tasks.Any(func(t *syncTask) bool { return t.completed() && !t.successful() }) {
 		sc.deleteHooks(hooksPendingDeletionFailed)
-		sc.setOperationFailed(syncFailTasks, "one or more synchronization tasks completed unsuccessfully")
+		sc.setOperationFailed(syncFailTasks, syncFailedTasks, "one or more synchronization tasks completed unsuccessfully")
 		return
 	}
 
@@ -504,8 +506,9 @@ func (sc *syncContext) Sync() {
 
 	switch runState {
 	case failed:
+		syncFailedTasks, _ := tasks.Split(func(t *syncTask) bool { return t.syncStatus == common.ResultCodeSyncFailed })
 		sc.deleteHooks(hooksPendingDeletionFailed)
-		sc.setOperationFailed(syncFailTasks, "one or more objects failed to apply")
+		sc.setOperationFailed(syncFailTasks, syncFailedTasks, "one or more objects failed to apply")
 	case successful:
 		if remainingTasks.Len() == 0 {
 			// delete all completed hooks which have appropriate delete policy
@@ -556,21 +559,33 @@ func (sc *syncContext) GetState() (common.OperationPhase, string, []common.Resou
 	return sc.phase, sc.message, resourceRes
 }
 
-func (sc *syncContext) setOperationFailed(syncFailTasks syncTasks, message string) {
+func (sc *syncContext) setOperationFailed(syncFailTasks, syncFailedTasks syncTasks, message string) {
+	errorMessageFactory := func(tasks []*syncTask, message string) string {
+		messages := syncFailedTasks.Map(func(task *syncTask) string {
+			return task.message
+		})
+		if len(messages) > 0 {
+			return fmt.Sprintf("%s, reason: %s", message, strings.Join(messages, ","))
+		}
+		return message
+	}
+
+	errorMessage := errorMessageFactory(syncFailedTasks, message)
+
 	if len(syncFailTasks) > 0 {
 		// if all the failure hooks are completed, don't run them again, and mark the sync as failed
 		if syncFailTasks.All(func(task *syncTask) bool { return task.completed() }) {
-			sc.setOperationPhase(common.OperationFailed, message)
+			sc.setOperationPhase(common.OperationFailed, errorMessage)
 			return
 		}
 		// otherwise, we need to start the failure hooks, and then return without setting
 		// the phase, so we make sure we have at least one more sync
 		sc.log.WithValues("syncFailTasks", syncFailTasks).V(1).Info("Running sync fail tasks")
 		if sc.runTasks(syncFailTasks, false) == failed {
-			sc.setOperationPhase(common.OperationFailed, message)
+			sc.setOperationPhase(common.OperationFailed, errorMessage)
 		}
 	} else {
-		sc.setOperationPhase(common.OperationFailed, message)
+		sc.setOperationPhase(common.OperationFailed, errorMessage)
 	}
 }
 

pkg/sync/sync_context_test.go

@@ -1266,3 +1266,40 @@ func TestSyncContext_GetDeleteOptions_WithPrunePropagationPolicy(t *testing.T) {
 	opts := sc.getDeleteOptions()
 	assert.Equal(t, v1.DeletePropagationBackground, *opts.PropagationPolicy)
 }
+
+func TestSetOperationFailed(t *testing.T) {
+	sc := syncContext{}
+	sc.log = klogr.New().WithValues("application", "fake-app")
+
+	tasks := make([]*syncTask, 0)
+	tasks = append(tasks, &syncTask{message: "namespace not found"})
+
+	sc.setOperationFailed(nil, tasks, "one or more objects failed to apply")
+
+	assert.Equal(t, sc.message, "one or more objects failed to apply, reason: namespace not found")
+
+}
+
+func TestSetOperationFailedDuplicatedMessages(t *testing.T) {
+	sc := syncContext{}
+	sc.log = klogr.New().WithValues("application", "fake-app")
+
+	tasks := make([]*syncTask, 0)
+	tasks = append(tasks, &syncTask{message: "namespace not found"})
+	tasks = append(tasks, &syncTask{message: "namespace not found"})
+
+	sc.setOperationFailed(nil, tasks, "one or more objects failed to apply")
+
+	assert.Equal(t, sc.message, "one or more objects failed to apply, reason: namespace not found")
+
+}
+
+func TestSetOperationFailedNoTasks(t *testing.T) {
+	sc := syncContext{}
+	sc.log = klogr.New().WithValues("application", "fake-app")
+
+	sc.setOperationFailed(nil, nil, "one or more objects failed to apply")
+
+	assert.Equal(t, sc.message, "one or more objects failed to apply")
+
+}

pkg/sync/sync_tasks.go

@@ -196,6 +196,18 @@ func (s syncTasks) Split(predicate func(task *syncTask) bool) (trueTasks, falseT
 	return trueTasks, falseTasks
 }
 
+func (s syncTasks) Map(predicate func(task *syncTask) string) []string {
+	messagesMap := make(map[string]interface{})
+	for _, task := range s {
+		messagesMap[predicate(task)] = nil
+	}
+	messages := make([]string, 0)
+	for key := range messagesMap {
+		messages = append(messages, key)
+	}
+	return messages
+}
+
 func (s syncTasks) All(predicate func(task *syncTask) bool) bool {
 	for _, task := range s {
 		if !predicate(task) {