From bc9ce5764fa306f58cf59199a94f6c968c775a2d Mon Sep 17 00:00:00 2001 From: pasha-codefresh Date: Thu, 21 Oct 2021 22:20:28 +0300 Subject: [PATCH] feat: better error message for sync operations (#336) feat: better error message for sync operations (#336) Signed-off-by: pashavictorovich --- pkg/sync/sync_context.go | 27 +++++++++++++++++++------ pkg/sync/sync_context_test.go | 37 +++++++++++++++++++++++++++++++++++ pkg/sync/sync_tasks.go | 12 ++++++++++++ 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 152d4a132..a2692cfa9 100644 --- a/pkg/sync/sync_context.go +++ b/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) } } diff --git a/pkg/sync/sync_context_test.go b/pkg/sync/sync_context_test.go index 08931b9ef..2cd6affc4 100644 --- a/pkg/sync/sync_context_test.go +++ b/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") + +} diff --git a/pkg/sync/sync_tasks.go b/pkg/sync/sync_tasks.go index fac94d65d..fb965f6b4 100644 --- a/pkg/sync/sync_tasks.go +++ b/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) {