Avoid updating the association status when no association (elastic#4986…

…) (elastic#4987)

This fixes a bug where the association status of the Elasticsearch resource is updated by the EsMonitoring association controller when there is no association and therefore no changes to be made. The `oldStatus` and the `newStatus` appeared to be different because one was nil and the other was empty. The equality tests now supports this situation.

pkg/controller/association/controller/es_monitoring.go

@@ -35,7 +35,11 @@ const (
 // Beats are configured to collect monitoring metrics and logs data of the associated Elasticsearch and send
 // them to the Elasticsearch referenced in the association.
 func AddEsMonitoring(mgr manager.Manager, accessReviewer rbac.AccessReviewer, params operator.Parameters) error {
-	return association.AddAssociationController(mgr, accessReviewer, params, association.AssociationInfo{
+	return association.AddAssociationController(mgr, accessReviewer, params, esMonitoringAssociationInfo())
+}
+
+func esMonitoringAssociationInfo() association.AssociationInfo {
+	return association.AssociationInfo{
 		AssociatedObjTemplate:     func() commonv1.Associated { return &esv1.Elasticsearch{} },
 		ReferencedObjTemplate:     func() client.Object { return &esv1.Elasticsearch{} },
 		ReferencedResourceVersion: referencedElasticsearchStatusVersion,
@@ -64,5 +68,5 @@ func AddEsMonitoring(mgr manager.Manager, accessReviewer rbac.AccessReviewer, pa
 				return user.StackMonitoringUserRole, nil
 			},
 		},
-	})
+	}
 }

pkg/controller/association/controller/es_monitoring_test.go

@@ -0,0 +1,48 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+package controller
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
+	"github.com/elastic/cloud-on-k8s/pkg/controller/association"
+	"github.com/elastic/cloud-on-k8s/pkg/controller/common/annotation"
+	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
+)
+
+var (
+	sampleES = esv1.Elasticsearch{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "esns",
+			Name:      "esname",
+			Annotations: map[string]string{
+				annotation.ControllerVersionAnnotation: "1.5.0",
+			},
+			ResourceVersion: "42"},
+		Status: esv1.ElasticsearchStatus{Version: "7.15.0"},
+	}
+)
+
+// Test_EsMonitoringReconciler_NoAssociation tests that an Elasticsearch resource is not updated by the EsMonitoring
+// reconciler when there is no EsMonitoring association. Covers the bug https://github.com/elastic/cloud-on-k8s/issues/4985.
+func Test_EsMonitoringReconciler_NoAssociation(t *testing.T) {
+	es := sampleES
+	resourceVersion := es.ResourceVersion
+	r := association.NewTestAssociationReconciler(esMonitoringAssociationInfo(), &es)
+	_, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: k8s.ExtractNamespacedName(&es)})
+	require.NoError(t, err)
+	// should not update the Elasticsearch resource
+	var updatedEs esv1.Elasticsearch
+	err = r.Get(context.Background(), k8s.ExtractNamespacedName(&es), &updatedEs)
+	require.NoError(t, err)
+	// resource version should not have changed
+	require.Equal(t, resourceVersion, updatedEs.ResourceVersion)
+}

pkg/controller/association/reconciler.go

@@ -10,16 +10,17 @@ import (
 	"reflect"
 	"time"
 
+	"github.com/go-logr/logr"
 	"go.elastic.co/apm"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	"github.com/go-logr/logr"
-
+	"github.com/elastic/cloud-on-k8s/pkg/about"
 	commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1"
 	esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common"
@@ -409,6 +410,10 @@ func (r *Reconciler) updateStatus(ctx context.Context, associated commonv1.Assoc
 	}
 	newStatus = associated.AssociationStatusMap(r.AssociationType)
 
+	// shortcut if the two maps are nil or empty
+	if len(oldStatus) == 0 && len(newStatus) == 0 {
+		return nil
+	}
 	if !reflect.DeepEqual(oldStatus, newStatus) {
 		if err := r.Status().Update(ctx, associated); err != nil {
 			return err
@@ -446,3 +451,22 @@ func (r *Reconciler) onDelete(ctx context.Context, associated types.NamespacedNa
 		r.log(associated).Error(err, "Error while trying to delete orphaned resources. Continuing.")
 	}
 }
+
+// NewTestAssociationReconciler creates a new AssociationReconciler given an AssociationInfo for testing.
+func NewTestAssociationReconciler(assocInfo AssociationInfo, runtimeObjs ...runtime.Object) Reconciler {
+	return Reconciler{
+		AssociationInfo: assocInfo,
+		Client:          k8s.NewFakeClient(runtimeObjs...),
+		accessReviewer:  rbac.NewPermissiveAccessReviewer(),
+		watches:         watches.NewDynamicWatches(),
+		recorder:        record.NewFakeRecorder(10),
+		Parameters: operator.Parameters{
+			OperatorInfo: about.OperatorInfo{
+				BuildInfo: about.BuildInfo{
+					Version: "1.5.0",
+				},
+			},
+		},
+		logger: log.WithName("test"),
+	}
+}