surface root cause in sync failure message and cache discovery errors

Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>

Author: ppapapetrou76

gitops-engine/pkg/sync/sync_context.go

@@ -481,7 +481,23 @@ func (sc *syncContext) Sync() {
 	sc.log.WithValues("skipHooks", sc.skipHooks, "started", sc.started()).Info("Syncing")
 	tasks, ok := sc.getSyncTasks()
 	if !ok {
-		sc.setOperationPhase(common.OperationFailed, "one or more synchronization tasks are not valid")
+		// Collect distinct error messages from failed resource results so that the
+		// operation phase message surfaces the actual root cause (e.g. cluster API
+		// server unreachable) rather than only the generic "not valid" message.
+		seen := make(map[string]bool)
+
+		var errMessages []string
+		for _, res := range sc.syncRes {
+			if res.Status == common.ResultCodeSyncFailed && res.Message != "" && !seen[res.Message] {
+				seen[res.Message] = true
+				errMessages = append(errMessages, res.Message)
+			}
+		}
+		msg := "one or more synchronization tasks are not valid"
+		if len(errMessages) > 0 {
+			msg = fmt.Sprintf("%s: %s", msg, strings.Join(errMessages, "; "))
+		}
+		sc.setOperationPhase(common.OperationFailed, msg)
 		return
 	}
 
@@ -1024,6 +1040,7 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) {
 	isRetryable := apierrors.IsUnauthorized
 
 	serverResCache := make(map[schema.GroupVersionKind]*metav1.APIResource)
+	serverResErrCache := make(map[schema.GroupVersionKind]error)
 
 	// check permissions
 	for _, task := range tasks {
@@ -1033,6 +1050,8 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) {
 		if val, ok := serverResCache[task.groupVersionKind()]; ok {
 			serverRes = val
 			err = nil
+		} else if cachedErr, ok := serverResErrCache[task.groupVersionKind()]; ok {
+			err = cachedErr
 		} else {
 			err = retry.OnError(retry.DefaultRetry, isRetryable, func() error {
 				serverRes, err = kubeutil.ServerResourceForGroupVersionKind(sc.disco, task.groupVersionKind(), "get")
@@ -1041,6 +1060,8 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) {
 			})
 			if serverRes != nil {
 				serverResCache[task.groupVersionKind()] = serverRes
+			} else if err != nil {
+				serverResErrCache[task.groupVersionKind()] = err
 			}
 		}
 

gitops-engine/pkg/sync/sync_context_test.go

@@ -922,6 +922,71 @@ func TestServerResourcesRetry(t *testing.T) {
 	}
 }
 
+func TestSync_getSyncTasks_FailureMessage(t *testing.T) {
+	syncCtx := newTestSyncCtx(nil)
+	testSvc := testingutils.NewService()
+	testSvc.SetName("test-service")
+	testSvc.SetNamespace(testingutils.FakeArgoCDNamespace)
+
+	// Mock discovery failure for this resource
+	fakeDisco := &fakedisco.FakeDiscovery{
+		Fake: &testcore.Fake{
+			Resources: []*metav1.APIResourceList{},
+		},
+	}
+	fakeDisco.Fake.PrependReactor("get", "resource", func(action testcore.Action) (handled bool, ret runtime.Object, err error) {
+		return true, nil, errors.New("discovery failed")
+	})
+	syncCtx.disco = fakeDisco
+
+	syncCtx.resources = groupResources(ReconciliationResult{
+		Live:   []*unstructured.Unstructured{testSvc},
+		Target: []*unstructured.Unstructured{testSvc},
+	})
+
+	syncCtx.Sync()
+	phase, msg, _ := syncCtx.GetState()
+
+	assert.Equal(t, synccommon.OperationFailed, phase)
+	assert.Contains(t, msg, "one or more synchronization tasks are not valid")
+	assert.Contains(t, msg, "discovery failed")
+}
+
+func Test_getSyncTasks_ErrorCaching(t *testing.T) {
+	syncCtx := newTestSyncCtx(nil)
+
+	// Create two services with the same GVK
+	svc1 := testingutils.NewService()
+	svc1.SetName("svc1")
+	svc2 := testingutils.NewService()
+	svc2.SetName("svc2")
+
+	// Mock discovery failure
+	discoveryCalls := 0
+	fakeDisco := &fakedisco.FakeDiscovery{
+		Fake: &testcore.Fake{
+			Resources: []*metav1.APIResourceList{},
+		},
+	}
+	fakeDisco.Fake.PrependReactor("get", "resource", func(action testcore.Action) (handled bool, ret runtime.Object, err error) {
+		discoveryCalls++
+		return true, nil, errors.New("persistent discovery error")
+	})
+	syncCtx.disco = fakeDisco
+
+	syncCtx.resources = groupResources(ReconciliationResult{
+		Live:   []*unstructured.Unstructured{svc1, svc2},
+		Target: []*unstructured.Unstructured{svc1, svc2},
+	})
+
+	tasks, ok := syncCtx.getSyncTasks()
+	assert.False(t, ok)
+	assert.NotNil(t, tasks)
+
+	// discoveryCalls should be 1 if caching works.
+	assert.Equal(t, 1, discoveryCalls, "Discovery should have been called only once due to caching")
+}
+
 func TestDoNotSyncOrPruneHooks(t *testing.T) {
 	syncCtx := newTestSyncCtx(nil, WithOperationSettings(false, false, false, true))
 	targetPod := testingutils.NewPod()