Update unit and e2e test to check expected values

Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>

.github/workflows/e2e_tests.yml

@@ -49,5 +49,4 @@ jobs:
       - name: Run tests
         run: |
           set -o pipefail
-          kubectl apply -f https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/release-0.52/example/prometheus-operator-crd/monitoring.coreos.com_servicemonitors.yaml
           make test-e2e 2>&1 | tee /tmp/e2e-test.log

controllers/argorollouts_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 
 	rolloutsmanagerv1alpha1 "github.com/argoproj-labs/argo-rollouts-manager/api/v1alpha1"
+	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -127,5 +128,8 @@ func (r *RolloutManagerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	// Watch for changes to RoleBinding sub-resources owned by RolloutManager.
 	bld.Owns(&rbacv1.RoleBinding{})
 
+	// Watch for changes to ServiceMonitor sub-resources owned by RolloutManager.
+	bld.Owns(&monitoringv1.ServiceMonitor{})
+
 	return bld.Complete(r)
 }

controllers/resources.go

@@ -293,12 +293,18 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C
 		},
 	}
 
-	if err := fetchObject(ctx, r.Client, smCRD.Namespace, smCRD.Name, smCRD); err == nil {
-		// Create ServiceMonitor for Rollouts metrics
-		err := r.createServiceMonitorIfAbsent(cr.Namespace, cr, actualSvc.Name, actualSvc.Name)
-		if err != nil {
-			return err
+	if err := fetchObject(ctx, r.Client, smCRD.Namespace, smCRD.Name, smCRD); err != nil {
+		if !apierrors.IsNotFound(err) {
+			return fmt.Errorf("failed to get the ServiceMonitor %s : %s", smCRD.Name, err)
 		}
+
+		return fmt.Errorf("failed to get ServiceMonitor for Rollouts metrics associated with %s : %s", smCRD.Name, err)
+	}
+
+	// Create ServiceMonitor for Rollouts metrics
+	err := r.createServiceMonitorIfAbsent(cr.Namespace, cr, actualSvc.Name, actualSvc.Name)
+	if err != nil {
+		return err
 	}
 
 	return nil
@@ -841,6 +847,20 @@ func (r *RolloutManagerReconciler) createServiceMonitorIfAbsent(namespace string
 	if err == nil {
 		log.Info("A ServiceMonitor instance already exists",
 			"Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name)
+
+		// Check if existing ServiceMonitor matches expected content
+		if !serviceMonitorMatches(existingServiceMonitor, serviceMonitorLabel) {
+			log.Info("Updating existing ServiceMonitor instance",
+				"Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name)
+
+			// Update ServiceMonitor with expected content
+			updateServiceMonitor(existingServiceMonitor, serviceMonitorLabel)
+			if err := r.Client.Update(context.TODO(), existingServiceMonitor); err != nil {
+				log.Error(err, "Error updating existing ServiceMonitor instance",
+					"Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name)
+				return err
+			}
+		}
 		return nil
 	}
 	if apierrors.IsNotFound(err) {
@@ -890,3 +910,36 @@ func newServiceMonitor(namespace, name, matchLabel string) *monitoringv1.Service
 		Spec:       spec,
 	}
 }
+
+func serviceMonitorMatches(sm *monitoringv1.ServiceMonitor, matchLabel string) bool {
+	// Check if labels match
+	labels := sm.Spec.Selector.MatchLabels
+	if val, ok := labels["app.kubernetes.io/name"]; ok {
+		if val != matchLabel {
+			return false
+		}
+	} else {
+		return false
+	}
+
+	// Check if endpoints match
+	if sm.Spec.Endpoints[0].Port != "metrics" {
+		return false
+	}
+
+	return true
+}
+
+func updateServiceMonitor(sm *monitoringv1.ServiceMonitor, matchLabel string) {
+	// Update labels
+	sm.Spec.Selector.MatchLabels = map[string]string{
+		"app.kubernetes.io/name": matchLabel,
+	}
+
+	// Update endpoints
+	sm.Spec.Endpoints = []monitoringv1.Endpoint{
+		{
+			Port: "metrics",
+		},
+	}
+}

controllers/resources_test.go

@@ -155,6 +155,15 @@ var _ = Describe("ReconcileRolloutManager tests", func() {
 					},
 				},
 			},
+			{
+				fmt.Sprintf("ServiceMonitor %s", DefaultArgoRolloutsResourceName),
+				&monitoringv1.ServiceMonitor{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      DefaultArgoRolloutsResourceName,
+						Namespace: a.Namespace,
+					},
+				},
+			},
 		}
 
 		for _, test := range tt {
@@ -187,50 +196,140 @@ var _ = Describe("ReconcileRolloutManager tests", func() {
 	})
 
 	It("Verify whether RolloutManager creating ServiceMonitor", func() {
-		smCRD := &crdv1.CustomResourceDefinition{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "servicemonitors.monitoring.coreos.com",
-				Namespace: req.Namespace,
-			},
-		}
+		smCRD, existingSvc := serviceAndServiceMonitorCRD(req.Namespace)
+		Expect(r.Client.Create(ctx, smCRD)).To(Succeed())
+		Expect(r.Client.Create(ctx, existingSvc)).To(Succeed())
+
+		res, err := r.Reconcile(ctx, req)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(res.Requeue).Should(BeFalse(), "reconcile should not requeue request")
 
+		expectedServiceMonitor := serviceMonitor()
+
+		sm := &monitoringv1.ServiceMonitor{}
+		Expect(r.Client.Get(ctx, types.NamespacedName{
+			Name:      DefaultArgoRolloutsMetricsServiceName,
+			Namespace: testNamespace,
+		}, sm)).To(Succeed())
+
+		Expect(sm.Name).To(Equal(expectedServiceMonitor.Name))
+		Expect(sm.Namespace).To(Equal(expectedServiceMonitor.Namespace))
+		Expect(sm.Spec).To(Equal(expectedServiceMonitor.Spec))
+	})
+
+	It("Verify if ServiceMonitor exists, but has different content than we expect then it should update ServiceMonitor", func() {
+		smCRD, existingSvc := serviceAndServiceMonitorCRD(req.Namespace)
 		Expect(r.Client.Create(ctx, smCRD)).To(Succeed())
+		Expect(r.Client.Create(ctx, existingSvc)).To(Succeed())
 
-		existingSvc := &corev1.Service{
+		existingServiceMonitor := &monitoringv1.ServiceMonitor{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      DefaultArgoRolloutsMetricsServiceName,
-				Namespace: req.Namespace,
-				Labels: map[string]string{
-					"app.kubernetes.io/name":      DefaultArgoRolloutsMetricsServiceName,
-					"app.kubernetes.io/component": "server",
-				},
+				Namespace: testNamespace,
 			},
-			Spec: corev1.ServiceSpec{
-				Ports: []corev1.ServicePort{
-					{
-						Name:       "metrics",
-						Port:       8090,
-						Protocol:   corev1.ProtocolTCP,
-						TargetPort: intstr.FromInt(8090),
+			Spec: monitoringv1.ServiceMonitorSpec{
+				Selector: metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app.kubernetes.io/name": "test-label",
 					},
 				},
-				Selector: map[string]string{
-					DefaultRolloutsSelectorKey: DefaultArgoRolloutsResourceName,
+				Endpoints: []monitoringv1.Endpoint{
+					{
+						Port: "metrics-test",
+					},
 				},
 			},
 		}
 
-		Expect(r.Client.Create(ctx, existingSvc)).To(Succeed())
+		Expect(r.Client.Create(ctx, existingServiceMonitor)).To(Succeed())
 
 		res, err := r.Reconcile(ctx, req)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(res.Requeue).Should(BeFalse(), "reconcile should not requeue request")
 
-		sm := &monitoringv1.ServiceMonitor{}
+		expectedSM := serviceMonitor()
+
 		Expect(r.Client.Get(ctx, types.NamespacedName{
 			Name:      DefaultArgoRolloutsMetricsServiceName,
 			Namespace: testNamespace,
-		}, sm)).To(Succeed())
+		}, existingServiceMonitor)).To(Succeed())
+
+		Expect(existingServiceMonitor.Name).To(Equal(expectedSM.Name))
+		Expect(existingServiceMonitor.Namespace).To(Equal(expectedSM.Namespace))
+		Expect(existingServiceMonitor.Spec).To(Equal(expectedSM.Spec))
+
 	})
 
+	It("Verify ServiceMonitor is not created if the CRD does not exist.", func() {
+		_, existingSvc := serviceAndServiceMonitorCRD(req.Namespace)
+		Expect(r.Client.Create(ctx, existingSvc)).To(Succeed())
+
+		res, err := r.Reconcile(ctx, req)
+		Expect(err).To(HaveOccurred())
+		Expect(res.Requeue).Should(BeFalse(), "reconcile should not requeue request")
+
+		sm := &monitoringv1.ServiceMonitor{}
+		Expect(r.Client.Get(ctx, types.NamespacedName{
+			Name:      DefaultArgoRolloutsMetricsServiceName,
+			Namespace: testNamespace,
+		}, sm)).ToNot(Succeed())
+	})
 })
+
+func serviceMonitor() *monitoringv1.ServiceMonitor {
+	sm := &monitoringv1.ServiceMonitor{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      DefaultArgoRolloutsMetricsServiceName,
+			Namespace: testNamespace,
+		},
+		Spec: monitoringv1.ServiceMonitorSpec{
+			Selector: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"app.kubernetes.io/name": DefaultArgoRolloutsMetricsServiceName,
+				},
+			},
+			Endpoints: []monitoringv1.Endpoint{
+				{
+					Port: "metrics",
+				},
+			},
+		},
+	}
+	return sm
+}
+
+func serviceAndServiceMonitorCRD(namespace string) (*crdv1.CustomResourceDefinition, *corev1.Service) {
+	smCRD := &crdv1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "servicemonitors.monitoring.coreos.com",
+			Namespace: namespace,
+		},
+	}
+
+	existingSvc := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      DefaultArgoRolloutsMetricsServiceName,
+			Namespace: namespace,
+			Labels: map[string]string{
+				"app.kubernetes.io/name":      DefaultArgoRolloutsResourceName,
+				"app.kubernetes.io/component": "server",
+			},
+		},
+		Spec: corev1.ServiceSpec{
+			Ports: []corev1.ServicePort{
+				{
+					Name:       "metrics",
+					Port:       8090,
+					Protocol:   corev1.ProtocolTCP,
+					TargetPort: intstr.FromInt(8090),
+				},
+			},
+			Selector: map[string]string{
+				DefaultRolloutsSelectorKey: DefaultArgoRolloutsResourceName,
+			},
+		},
+	}
+
+	return smCRD, existingSvc
+
+}

hack/run-rollouts-manager-e2e-tests.sh

@@ -8,10 +8,17 @@ SCRIPTPATH="$(
 cd "$SCRIPTPATH/.."
 
 set -o pipefail
-set -ex
 
-go test -v -p=1 -timeout=10m -race -count=1 -coverprofile=coverage.out ./tests/e2e
+# Check if the CRD exists
+kubectl get crd/servicemonitors.monitoring.coreos.com &> /dev/null
+retVal=$?
+if [ $retVal -ne 0 ]; then
+    # If the CRD is not found, apply the CRD YAML
+    kubectl apply -f https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/release-0.52/example/prometheus-operator-crd/monitoring.coreos.com_servicemonitors.yaml
+fi
 
+set -ex
+go test -v -p=1 -timeout=10m -race -count=1 -coverprofile=coverage.out ./tests/e2e
 set +e
 
 # If the output from the E2E operator is available, then check it for errors

hack/run-upstream-argo-rollouts-e2e-tests.sh

@@ -46,6 +46,14 @@ if [ -z "$SKIP_RUN_STEP" ]; then
 
   set +e
 
+  # Check if the ServiceMonitor CRD exists
+  kubectl get crd/servicemonitors.monitoring.coreos.com &> /dev/null
+  retVal=$?
+  if [ $retVal -ne 0 ]; then
+      # If the CRD is not found, apply the CRD YAML
+      kubectl apply -f https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/release-0.52/example/prometheus-operator-crd/monitoring.coreos.com_servicemonitors.yaml
+  fi 
+
   rm -f /tmp/e2e-operator-run.log || true
   go run ./main.go 2>&1 | tee /tmp/e2e-operator-run.log &
 

tests/e2e/rollouts_test.go

@@ -10,13 +10,14 @@ import (
 
 	"github.com/argoproj-labs/argo-rollouts-manager/tests/e2e/fixture/k8s"
 	rolloutManagerFixture "github.com/argoproj-labs/argo-rollouts-manager/tests/e2e/fixture/rolloutmanager"
+	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	rolloutsmanagerv1alpha1 "github.com/argoproj-labs/argo-rollouts-manager/api/v1alpha1"
 
 	controllers "github.com/argoproj-labs/argo-rollouts-manager/controllers"
-	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -181,6 +182,11 @@ var _ = Describe("RolloutManager tests", func() {
 					ObjectMeta: metav1.ObjectMeta{Name: controllers.DefaultRolloutsNotificationSecretName, Namespace: rolloutManager.Namespace},
 				}, "30s", "1s").ShouldNot(k8s.ExistByName(k8sClient))
 
+				By("deleting the serviceMonitor")
+				Eventually(&monitoringv1.ServiceMonitor{
+					ObjectMeta: metav1.ObjectMeta{Name: controllers.DefaultArgoRolloutsMetricsServiceName, Namespace: rolloutManager.Namespace},
+				}, "30s", "1s").ShouldNot(k8s.ExistByName(k8sClient))
+
 				// Make sure the cluster roles have not been deleted
 				By("NOT deleting the three cluster roles")
 				clusterRoleSuffixes := []string{"aggregate-to-admin", "aggregate-to-edit", "aggregate-to-view"}
@@ -299,7 +305,26 @@ var _ = Describe("RolloutManager tests", func() {
 					return deployment.Spec.Template.Spec.Containers[0].Image
 				}, "10s", "1s").Should(Equal(expectedVersion))
 
-				By("verify whether ServiceMonitor is created or not for Rolloutsmanager")
+				expectedServiceMonitor := &monitoringv1.ServiceMonitor{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "argo-rollouts-metrics",
+						Namespace: fixture.TestE2ENamespace,
+					},
+					Spec: monitoringv1.ServiceMonitorSpec{
+						Selector: metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"app.kubernetes.io/name": "argo-rollouts-metrics",
+							},
+						},
+						Endpoints: []monitoringv1.Endpoint{
+							{
+								Port: "metrics",
+							},
+						},
+					},
+				}
+
+				By("verify whether ServiceMonitor is created or not for RolloutManager")
 				sm := &monitoringv1.ServiceMonitor{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "argo-rollouts-metrics",
@@ -308,6 +333,10 @@ var _ = Describe("RolloutManager tests", func() {
 				}
 
 				Eventually(sm, "10s", "1s").Should(k8s.ExistByName(k8sClient))
+				Expect(sm.Name).To(Equal(expectedServiceMonitor.Name))
+				Expect(sm.Namespace).To(Equal(expectedServiceMonitor.Namespace))
+				Expect(sm.Spec).To(Equal(expectedServiceMonitor.Spec))
+
 			})
 		})
 	})