Do not write elasticsearch.k8s.elastic.co/managed-remote-clusters when not necessary (#8932)

Author: barkbay

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
 }

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...)
+}