Repository: elastic/cloud-on-k8s

Issue:
Question: Flapping ArgoCD reconciliation when spec.remoteClusters is defined
ECK version: 2.14.0
Elasticsearch version: 8.18.3 Enterprise Orchestration License
GitOps tool: Argo CD
Observed behavior: Continuous flapping between Synced and OutOfSync in Argo CD when spec.remoteClusters is defined in the Elasticsearch manifest.

Problem Description
We are deploying Elasticsearch clusters using ECK and managing the manifests via GitOps with Argo CD. 
We have a custom "helm chart" for our complete stack but we use helm template to generate a plain yaml manifest that we deploy with ArgoCD. 
In a multi-cluster setup, we define the spec.remoteClusters block directly in our Git-managed manifests to establish cross-cluster trust.

While the trust setup works, we are experiencing persistent reconciliation flapping in Argo CD. 
The elastic cluster is green and useable, the desired remote cluster is available, connected and green andall stack configuration policies applied.
The ArgoCD application constantly switches between Synced and OutOfSync, despite no visible differences being reported by argocd app diff with following messages:

```
Could not update remote clusters in Elasticsearch settings, re-queuing
Operation cannot be fulfilled on elasticsearches.elasticsearch.k8s.elastic.co "cluster-name": the object has been modified; please apply your changes to the latest version and try again
```
Already tried to ignore some of the specs I saw modified by the ECK Operator
 ```
resource.customizations: |
  elasticsearch.k8s.elastic.co/Elasticsearch:
    ignoreDifferences: |
      jsonPointers:
        - /spec/remoteClusters
        - /metadata/annotations/elasticsearch.k8s.elastic.co~1managed-remote-clusters
```
but to no avail

We are looking for one of the following:

Is anyone already running a conflict-free method to manage remoteClusters declaratively through Git?

If so, what resources/mutations can be ignored on the ArgoCD application side to break the OutOfSync loop?
A recommendation for how to structure ownership boundaries between ECK and GitOps tools like Argo CD when using trust configuration.
Any hints or inputs would be greatly appreciated!
Thanks!

Discussion:
@barkbay: Not an ArgoCD expert but we can see the generation incremented for each "sync" attempt, I don't think this is expected.

This can be reproduced using something like `argocd app create test-remote-cluster --repo https://github.com/elastic/cloud-on-k8s.git --path config/recipes/remoteclusters --dest-server https://kubernetes.default.svc --dest-namespace default`

I'm trying to understand what ArgoCD is updating while the ES spec does not change.@barkbay: This is the change applied by ArgoCD in my case, it does not seem related to `spec.remoteClusters` so maybe this is not a good way to reproduce the issue:

```diff
12c12
<   generation: 4
---
>   generation: 5
15c15
<   resourceVersion: "1764061501126303018"
---
>   resourceVersion: "1764061688545951018"
33,37d32
<     podTemplate:
<       metadata:
<         creationTimestamp: null
<       spec:
<         containers: null
```@barkbay: So I think part of the problem is that we are always updating an annotation without checking if it is necessary: https://github.com/elastic/cloud-on-k8s/blob/cb0d001d077a3779c04d203642f76dd99f189ebd/pkg/controller/elasticsearch/remotecluster/annotations.go#L64@barkbay: > So I think part of the problem is that we are always updating an annotation without checking if it is necessary:

https://github.com/elastic/cloud-on-k8s/pull/8932 should help. I still want to understand why the operator is setting empty values when ArgoCD wants to remove them.@barkbay: I'm closing as I think https://github.com/elastic/cloud-on-k8s/pull/8932 should help in the next release. Let's reopen it if it's not the case.

Solution patch:
diff --git a/pkg/controller/elasticsearch/remotecluster/annotations.go b/pkg/controller/elasticsearch/remotecluster/annotations.go
index 44c320f26..07c43a33b 100644
--- a/pkg/controller/elasticsearch/remotecluster/annotations.go
+++ b/pkg/controller/elasticsearch/remotecluster/annotations.go
@@ -59,7 +59,12 @@ func annotateWithCreatedRemoteClusters(ctx context.Context, c k8s.Client, es esv
 	}
 
 	sort.Strings(annotation)
-	es.Annotations[ManagedRemoteClustersAnnotationName] = strings.Join(annotation, ",")
+	expected := strings.Join(annotation, ",")
+	current, ok := es.Annotations[ManagedRemoteClustersAnnotationName]
 
-	return c.Update(ctx, &es)
+	if !ok || current != expected {
+		es.Annotations[ManagedRemoteClustersAnnotationName] = expected
+		return c.Update(ctx, &es)
+	}
+	return nil
 }



Test patch:
diff --git a/pkg/controller/elasticsearch/remotecluster/elasticsearch_test.go b/pkg/controller/elasticsearch/remotecluster/elasticsearch_test.go
index 6ce3476a8..515413213 100644
--- a/pkg/controller/elasticsearch/remotecluster/elasticsearch_test.go
+++ b/pkg/controller/elasticsearch/remotecluster/elasticsearch_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	commonv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1"
 	esv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/elasticsearch/v1"
@@ -121,6 +122,7 @@ func TestUpdateSettings(t *testing.T) {
 		name                                  string
 		args                                  args
 		wantAnnotation                        string
+		wantKubernetesAPIUpdateCalled         int
 		wantRequeue                           bool
 		wantErr                               bool
 		wantGetRemoteClusterSettingsCalled    bool
@@ -139,6 +141,7 @@ func TestUpdateSettings(t *testing.T) {
 				),
 			},
 			wantRequeue:                           false,
+			wantKubernetesAPIUpdateCalled:         0,
 			wantGetRemoteClusterSettingsCalled:    false,
 			wantUpdateRemoteClusterSettingsCalled: false,
 		},
@@ -153,6 +156,7 @@ func TestUpdateSettings(t *testing.T) {
 					map[string]string{"foo": "bar", ManagedRemoteClustersAnnotationName: ""},
 				),
 			},
+			wantKubernetesAPIUpdateCalled:         1, // Annotation should be removed
 			wantRequeue:                           false,
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: false,
@@ -168,6 +172,7 @@ func TestUpdateSettings(t *testing.T) {
 					map[string]string{ManagedRemoteClustersAnnotationName: "ns2-es2"},
 				),
 			},
+			wantKubernetesAPIUpdateCalled:         1,
 			wantRequeue:                           false,
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: false,
@@ -188,6 +193,7 @@ func TestUpdateSettings(t *testing.T) {
 				),
 			},
 			wantAnnotation:                        "ns2-es2",
+			wantKubernetesAPIUpdateCalled:         1,
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: true,
 			wantSettings: esclient.RemoteClustersSettings{
@@ -219,6 +225,7 @@ func TestUpdateSettings(t *testing.T) {
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: true,
 			wantAnnotation:                        "ns1-es2",
+			wantKubernetesAPIUpdateCalled:         1,
 			wantSettings: esclient.RemoteClustersSettings{
 				PersistentSettings: &esclient.SettingsGroup{
 					Cluster: esclient.RemoteClusters{
@@ -259,6 +266,7 @@ func TestUpdateSettings(t *testing.T) {
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: true,
 			wantAnnotation:                        "ns2-es2",
+			wantKubernetesAPIUpdateCalled:         0,
 			wantSettings: esclient.RemoteClustersSettings{
 				PersistentSettings: &esclient.SettingsGroup{
 					Cluster: esclient.RemoteClusters{
@@ -298,6 +306,7 @@ func TestUpdateSettings(t *testing.T) {
 			},
 			wantRequeue:                           false,
 			wantAnnotation:                        "ns2-es2",
+			wantKubernetesAPIUpdateCalled:         1,
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: true,
 			wantSettings: esclient.RemoteClustersSettings{
@@ -343,6 +352,7 @@ func TestUpdateSettings(t *testing.T) {
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: true,
 			wantAnnotation:                        "ns1-es2,ns1-es3",
+			wantKubernetesAPIUpdateCalled:         1, // Add ns1-es3
 			wantSettings: esclient.RemoteClustersSettings{
 				PersistentSettings: &esclient.SettingsGroup{
 					Cluster: esclient.RemoteClusters{
@@ -466,6 +476,7 @@ func TestUpdateSettings(t *testing.T) {
 			wantGetRemoteClusterSettingsCalled:    true,
 			wantUpdateRemoteClusterSettingsCalled: true,
 			wantAnnotation:                        "ns1-es2,ns1-es4,ns1-es5",
+			wantKubernetesAPIUpdateCalled:         1,
 			wantSettings: esclient.RemoteClustersSettings{
 				PersistentSettings: &esclient.SettingsGroup{
 					Cluster: esclient.RemoteClusters{
@@ -481,7 +492,7 @@ func TestUpdateSettings(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			client := k8s.NewFakeClient(tt.args.es)
+			client := trackUpdateCalls(k8s.NewFakeClient(tt.args.es))
 			shouldRequeue, err := UpdateSettings(
 				context.Background(),
 				client,
@@ -504,6 +515,8 @@ func TestUpdateSettings(t *testing.T) {
 				assert.False(t, annotationExists)
 			}
 
+			assert.Equal(t, tt.wantKubernetesAPIUpdateCalled, client.updateCallCount, "Kubernetes API Update call count mismatch")
+
 			// Check the requeue result
 			assert.Equal(t, tt.wantRequeue, shouldRequeue)
 			assert.Equal(t, tt.wantGetRemoteClusterSettingsCalled, tt.args.esClient.getRemoteClusterSettingsCalled)
@@ -515,3 +528,17 @@ func TestUpdateSettings(t *testing.T) {
 		})
 	}
 }
+
+func trackUpdateCalls(c client.Client) *trackUpdateCallsClient {
+	return &trackUpdateCallsClient{Client: c}
+}
+
+type trackUpdateCallsClient struct {
+	client.Client
+	updateCallCount int
+}
+
+func (t *trackUpdateCallsClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
+	t.updateCallCount++
+	return t.Client.Update(ctx, obj, opts...)
+}