Merge pull request #27 from avi-biton/monitoring

chore(RHTAPWATCH-1207):Add metrics for total and failed notifications

internal/controller/metrics.go

@@ -0,0 +1,26 @@
+package controller
+
+import (
+	"github.com/prometheus/client_golang/prometheus"
+	"sigs.k8s.io/controller-runtime/pkg/metrics"
+)
+
+var (
+	notifications = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "notification_controller_notifications_total",
+			Help: "Number of total notification actions",
+		},
+	)
+	notificationsFailures = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "notification_controller_notifications_failures_total",
+			Help: "Number of failed notifications",
+		},
+	)
+)
+
+func init() {
+	// Register custom metrics with the global prometheus registry
+	metrics.Registry.MustRegister(notifications, notificationsFailures)
+}

internal/controller/notificationservice_controller.go

@@ -78,8 +78,10 @@ func (r *NotificationServiceReconciler) Reconcile(ctx context.Context, req ctrl.
 				return ctrl.Result{}, err
 			}
 			err = r.Notifier.Notify(ctx, string(results))
+			notifications.Inc()
 			if err != nil {
 				logger.Error(err, "Failed to Notify")
+				notificationsFailures.Inc()
 				return ctrl.Result{}, err
 			}
 			logger.Info("SNS Notified", "pipelinerun", pipelineRun.Name, "namespace", pipelineRun.Namespace)

internal/controller/notificationservice_controller_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/konflux-ci/operator-toolkit/metadata"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -33,7 +34,8 @@ import (
 
 var _ = Describe("NotificationService Controller", func() {
 	var (
-		pushPipelineRun, pullRequestPipelineRun *tektonv1.PipelineRun
+		pushPipelineRun, pullRequestPipelineRun            *tektonv1.PipelineRun
+		notificationsCounter, notificationsFailuresCounter float64
 	)
 	const (
 		timeout                    = time.Second * 10
@@ -90,120 +92,198 @@ var _ = Describe("NotificationService Controller", func() {
 				err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
 				return err == nil
 			}, timeout, interval).Should(BeTrue())
+
+			notificationsCounter = testutil.ToFloat64(notifications)
+			notificationsFailuresCounter = testutil.ToFloat64(notificationsFailures)
 		})
 		Context("when a push pipelinerun is created and end successfully", func() {
-			It("should reconcile successfully - Add finalizer, Read the results, add annotation and remove the finalizer", func() {
-				By("Creating a new push pipelinerun and add finalizer")
-
-				// The pipelinerun should be reconciled and the notification finalizer has been added successfully
-				Eventually(func() bool {
-					err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+			It("should reconcile successfully - Add finalizer, Read the results, add annotation and remove the finalizer, update metrics",
+				func() {
+					By("Creating a new push pipelinerun and add finalizer")
+					err := nsr.SetupWithManager(k8sManager)
 					Expect(err).ToNot(HaveOccurred())
-					return controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)
-				}, timeout, interval).Should(BeTrue())
-				Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeTrue())
-				// Check the Notify was not called
-				Expect(mn.Counter).To(BeZero())
+					// The pipelinerun should be reconciled and the notification finalizer has been added successfully
+					Eventually(func() bool {
+						err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+						Expect(err).ToNot(HaveOccurred())
+						return controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)
+					}, timeout, interval).Should(BeTrue())
+					Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeTrue())
+					// Check the Notify was not called
+					Expect(mn.Counter).To(BeZero())
 
-				By("Updating status to completed successfully")
-				createdPipelineRun.Status = tektonv1.PipelineRunStatus{
-					PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{
-						StartTime:      &metav1.Time{Time: time.Now()},
-						CompletionTime: &metav1.Time{Time: time.Now().Add(5 * time.Minute)},
-						Results: []tektonv1.PipelineRunResult{
-							{
-								Name:  "IMAGE_DIGEST",
-								Value: *tektonv1.NewStructuredValues("image_digest_value"),
-							},
-							{
-								Name:  "IMAGE_URL",
-								Value: *tektonv1.NewStructuredValues("image"),
-							},
-							{
-								Name:  "CHAINS-GIT_URL",
-								Value: *tektonv1.NewStructuredValues("git_url_value"),
-							},
-							{
-								Name:  "CHAINS-GIT_COMMIT",
-								Value: *tektonv1.NewStructuredValues("git_commit_value"),
+					By("Updating status to completed successfully")
+					createdPipelineRun.Status = tektonv1.PipelineRunStatus{
+						PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{
+							StartTime:      &metav1.Time{Time: time.Now()},
+							CompletionTime: &metav1.Time{Time: time.Now().Add(5 * time.Minute)},
+							Results: []tektonv1.PipelineRunResult{
+								{
+									Name:  "IMAGE_DIGEST",
+									Value: *tektonv1.NewStructuredValues("image_digest_value"),
+								},
+								{
+									Name:  "IMAGE_URL",
+									Value: *tektonv1.NewStructuredValues("image"),
+								},
+								{
+									Name:  "CHAINS-GIT_URL",
+									Value: *tektonv1.NewStructuredValues("git_url_value"),
+								},
+								{
+									Name:  "CHAINS-GIT_COMMIT",
+									Value: *tektonv1.NewStructuredValues("git_commit_value"),
+								},
 							},
 						},
-					},
-					Status: v1.Status{
-						Conditions: v1.Conditions{
-							apis.Condition{
-								Message: "Tasks Completed: 12 (Failed: 0, Cancelled 0), Skipped: 2",
-								Reason:  "Completed",
-								Status:  "True",
-								Type:    apis.ConditionSucceeded,
+						Status: v1.Status{
+							Conditions: v1.Conditions{
+								apis.Condition{
+									Message: "Tasks Completed: 12 (Failed: 0, Cancelled 0), Skipped: 2",
+									Reason:  "Completed",
+									Status:  "True",
+									Type:    apis.ConditionSucceeded,
+								},
 							},
 						},
-					},
-				}
-				Expect(k8sClient.Status().Update(ctx, createdPipelineRun)).Should(Succeed())
+					}
+					Expect(k8sClient.Status().Update(ctx, createdPipelineRun)).Should(Succeed())
+					// The pipelinerun should be reconciled:
+					// Read the results, add the notification annotation, remove the finalizer
+					Eventually(func() bool {
+						err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+						Expect(err).ToNot(HaveOccurred())
+						return metadata.HasAnnotationWithValue(createdPipelineRun, NotificationPipelineRunAnnotation, NotificationPipelineRunAnnotationValue)
+					}, timeout, interval).Should(BeTrue())
+					Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeFalse())
+					// Check the Notify was called only once
+					Expect(mn.Counter).To(Equal(1))
+					// Check that notifications metric increased by 1
+					Expect(testutil.ToFloat64(notifications)).To(Equal(notificationsCounter + 1))
+					// Check that notificationsFailures metric did not change
+					Expect(testutil.ToFloat64(notificationsFailures)).To(Equal(notificationsFailuresCounter))
+				})
+		})
 
-				// The pipelinerun should be reconciled:
-				// Read the results, add the notification annotation, remove the finalizer
-				Eventually(func() bool {
-					err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+		Context("when a push pipelinerun is created and end successfully, but Notify fails", func() {
+			It("should reconcile successfully - Add finalizer, Fail sending results, Not add annotation, Update metrics",
+				func() {
+					By("Creating a new push pipelinerun and add finalizer")
+					err := fakeErrorNotifyNsr.SetupWithManager(k8sManager)
 					Expect(err).ToNot(HaveOccurred())
-					return metadata.HasAnnotationWithValue(createdPipelineRun, NotificationPipelineRunAnnotation, NotificationPipelineRunAnnotationValue)
-				}, timeout, interval).Should(BeTrue())
-				Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeFalse())
-				// Check the Notify was called once
-				Expect(mn.Counter).To(Equal(1))
-			})
-		})
+					// The pipelinerun should be reconciled and the notification finalizer has been added successfully
+					Eventually(func() bool {
+						err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+						Expect(err).ToNot(HaveOccurred())
+						return controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)
+					}, timeout, interval).Should(BeTrue())
+					Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeTrue())
+					// Check the Notify was not called
+					Expect(mn.Counter).To(BeZero())
 
+					By("Updating status to completed successfully")
+					createdPipelineRun.Status = tektonv1.PipelineRunStatus{
+						PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{
+							StartTime:      &metav1.Time{Time: time.Now()},
+							CompletionTime: &metav1.Time{Time: time.Now().Add(5 * time.Minute)},
+							Results: []tektonv1.PipelineRunResult{
+								{
+									Name:  "IMAGE_DIGEST",
+									Value: *tektonv1.NewStructuredValues("image_digest_value"),
+								},
+								{
+									Name:  "IMAGE_URL",
+									Value: *tektonv1.NewStructuredValues("image"),
+								},
+								{
+									Name:  "CHAINS-GIT_URL",
+									Value: *tektonv1.NewStructuredValues("git_url_value"),
+								},
+								{
+									Name:  "CHAINS-GIT_COMMIT",
+									Value: *tektonv1.NewStructuredValues("git_commit_value"),
+								},
+							},
+						},
+						Status: v1.Status{
+							Conditions: v1.Conditions{
+								apis.Condition{
+									Message: "Tasks Completed: 12 (Failed: 0, Cancelled 0), Skipped: 2",
+									Reason:  "Completed",
+									Status:  "True",
+									Type:    apis.ConditionSucceeded,
+								},
+							},
+						},
+					}
+					Expect(k8sClient.Status().Update(ctx, createdPipelineRun)).Should(Succeed())
+					// Wait for the Notify method to be called
+					Eventually(func() bool {
+						return fakeErrorNotify.Counter != 0
+					}, timeout, interval).Should(BeTrue())
+					Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeTrue())
+					Expect(metadata.HasAnnotationWithValue(createdPipelineRun, NotificationPipelineRunAnnotation, NotificationPipelineRunAnnotationValue)).To(BeFalse())
+					// Since returning an error will reconcile the resource indefinitely, and we cannot track the number of Notify calls and
+					// the number of times the metrics will be updated we can check that the notification and notificationFailures
+					// metrics after at least one failure were increased
+					Expect(testutil.ToFloat64(notifications)).To(BeNumerically(">", notificationsCounter))
+					Expect(testutil.ToFloat64(notificationsFailures)).To(BeNumerically(">", notificationsFailuresCounter))
+				})
+		})
 		Context("when a push pipelinerun is created and end with failure", func() {
-			It("should reconcile successfully - Add finalizer, Not reading the results, Not adding annotation and remove the finalizer", func() {
-				By("Creating a new push pipelinerun and add finalizer")
-
-				// The pipelinerun should be reconciled and the notification finalizer has been added successfully
-				Eventually(func() bool {
-					err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+			It("should reconcile successfully - Add finalizer, Not reading the results, Not adding annotation and remove the finalizer, Not update metrics",
+				func() {
+					By("Creating a new push pipelinerun and add finalizer")
+					err := nsr.SetupWithManager(k8sManager)
 					Expect(err).ToNot(HaveOccurred())
-					return controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)
-				}, timeout, interval).Should(BeTrue())
-				Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeTrue())
+					// The pipelinerun should be reconciled and the notification finalizer has been added successfully
+					Eventually(func() bool {
+						err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+						Expect(err).ToNot(HaveOccurred())
+						return controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)
+					}, timeout, interval).Should(BeTrue())
+					Expect(controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)).To(BeTrue())
 
-				By("Updating status to completed with failure")
-				createdPipelineRun.Status = tektonv1.PipelineRunStatus{
-					PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{
-						StartTime:      &metav1.Time{Time: time.Now()},
-						CompletionTime: &metav1.Time{Time: time.Now().Add(5 * time.Minute)},
-					},
-					Status: v1.Status{
-						Conditions: v1.Conditions{
-							apis.Condition{
-								Message: "Tasks Completed: 12 (Failed: 0, Cancelled 0), Skipped: 2",
-								Reason:  "CouldntGetTask",
-								Status:  "False",
-								Type:    apis.ConditionSucceeded,
+					By("Updating status to completed with failure")
+					createdPipelineRun.Status = tektonv1.PipelineRunStatus{
+						PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{
+							StartTime:      &metav1.Time{Time: time.Now()},
+							CompletionTime: &metav1.Time{Time: time.Now().Add(5 * time.Minute)},
+						},
+						Status: v1.Status{
+							Conditions: v1.Conditions{
+								apis.Condition{
+									Message: "Tasks Completed: 12 (Failed: 0, Cancelled 0), Skipped: 2",
+									Reason:  "CouldntGetTask",
+									Status:  "False",
+									Type:    apis.ConditionSucceeded,
+								},
 							},
 						},
-					},
-				}
-				Expect(k8sClient.Status().Update(ctx, createdPipelineRun)).Should(Succeed())
+					}
+					Expect(k8sClient.Status().Update(ctx, createdPipelineRun)).Should(Succeed())
 
-				// The pipelinerun should be reconciled:
-				// Remove the finalizer
-				Eventually(func() bool {
-					err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
-					Expect(err).ToNot(HaveOccurred())
-					return controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)
-				}, timeout, interval).Should(BeFalse())
-				Expect(metadata.HasAnnotationWithValue(createdPipelineRun, NotificationPipelineRunAnnotation, NotificationPipelineRunAnnotationValue)).To(BeFalse())
-				// Check the Notify was not called
-				Expect(mn.Counter).To(BeZero())
-			})
+					// The pipelinerun should be reconciled:
+					// Remove the finalizer
+					Eventually(func() bool {
+						err := k8sClient.Get(ctx, pushPipelineRunLookupKey, createdPipelineRun)
+						Expect(err).ToNot(HaveOccurred())
+						return controllerutil.ContainsFinalizer(createdPipelineRun, NotificationPipelineRunFinalizer)
+					}, timeout, interval).Should(BeFalse())
+					Expect(metadata.HasAnnotationWithValue(createdPipelineRun, NotificationPipelineRunAnnotation, NotificationPipelineRunAnnotationValue)).To(BeFalse())
+					// Check the Notify was not called
+					Expect(mn.Counter).To(BeZero())
+					Expect(testutil.ToFloat64(notifications)).To(Equal(notificationsCounter))
+					Expect(testutil.ToFloat64(notificationsFailures)).To(Equal(notificationsFailuresCounter))
+				})
 		})
 	})
 
 	Describe("Testing No reconcile with non push pipelinerun", func() {
 		Context("When a non push pipelineRun is created", func() {
 			It("Reconcile should not run", func() {
-
+				err := nsr.SetupWithManager(k8sManager)
+				Expect(err).ToNot(HaveOccurred())
 				// Create a pull_request pipelinerun
 				pullRequestPipelineRun = &tektonv1.PipelineRun{
 					ObjectMeta: metav1.ObjectMeta{
@@ -238,7 +318,7 @@ var _ = Describe("NotificationService Controller", func() {
 						},
 					},
 				}
-				err := k8sClient.Create(ctx, pullRequestPipelineRun)
+				err = k8sClient.Create(ctx, pullRequestPipelineRun)
 				Expect(err).NotTo(HaveOccurred(), "failed to create test Pipelinerun resource")
 
 				// Wait for the resource to be created
@@ -295,6 +375,8 @@ var _ = Describe("NotificationService Controller", func() {
 				Expect(metadata.HasAnnotationWithValue(createdPipelineRun, NotificationPipelineRunAnnotation, NotificationPipelineRunAnnotationValue)).To(BeFalse())
 				// Check the Notify was not called
 				Expect(mn.Counter).To(BeZero())
+				Expect(testutil.ToFloat64(notifications)).To(Equal(notificationsCounter))
+				Expect(testutil.ToFloat64(notificationsFailures)).To(Equal(notificationsFailuresCounter))
 			})
 		})
 	})