Merge branch 'master' into patch-1

controller/controller.go

@@ -463,6 +463,12 @@ func (c *Manager) startLeading(ctx context.Context, rolloutThreadiness, serviceT
 	// Start the informer factories to begin populating the informer caches
 	log.Info("Starting Controllers")
 
+	c.notificationConfigMapInformerFactory.Start(ctx.Done())
+	c.notificationSecretInformerFactory.Start(ctx.Done())
+	if ok := cache.WaitForCacheSync(ctx.Done(), c.configMapSynced, c.secretSynced); !ok {
+		log.Fatalf("failed to wait for configmap/secret caches to sync, exiting")
+	}
+
 	// notice that there is no need to run Start methods in a separate goroutine. (i.e. go kubeInformerFactory.Start(stopCh)
 	// Start method is non-blocking and runs all registered informers in a dedicated goroutine.
 	c.dynamicInformerFactory.Start(ctx.Done())
@@ -471,9 +477,6 @@ func (c *Manager) startLeading(ctx context.Context, rolloutThreadiness, serviceT
 	}
 	c.kubeInformerFactory.Start(ctx.Done())
 
-	c.notificationConfigMapInformerFactory.Start(ctx.Done())
-	c.notificationSecretInformerFactory.Start(ctx.Done())
-
 	c.jobInformerFactory.Start(ctx.Done())
 
 	// Check if Istio installed on cluster before starting dynamicInformerFactory

utils/record/record.go

@@ -218,9 +218,7 @@ func (e *EventRecorderAdapter) defaultEventf(object runtime.Object, warn bool, o
 			err := e.sendNotifications(api, object, opts)
 			if err != nil {
 				logCtx.Errorf("Notifications failed to send for eventReason %s with error: %s", opts.EventReason, err)
-				e.NotificationFailedCounter.WithLabelValues(namespace, name, opts.EventType, opts.EventReason).Inc()
 			}
-			e.NotificationSuccessCounter.WithLabelValues(namespace, name, opts.EventType, opts.EventReason).Inc()
 		}
 	}
 
@@ -248,7 +246,7 @@ func NewAPIFactorySettings() api.Settings {
 }
 
 // Send notifications for triggered event if user is subscribed
-func (e *EventRecorderAdapter) sendNotifications(notificationsAPI api.API, object runtime.Object, opts EventOptions) error {
+func (e *EventRecorderAdapter) sendNotifications(notificationsAPI api.API, object runtime.Object, opts EventOptions) []error {
 	logCtx := logutil.WithObject(object)
 	_, namespace, name := logutil.KindNamespaceName(logCtx)
 	startTime := timeutil.Now()
@@ -259,7 +257,7 @@ func (e *EventRecorderAdapter) sendNotifications(notificationsAPI api.API, objec
 	}()
 
 	if notificationsAPI == nil {
-		return fmt.Errorf("notificationsAPI is nil")
+		return []error{fmt.Errorf("NotificationsAPI is nil")}
 	}
 
 	cfg := notificationsAPI.GetConfig()
@@ -274,39 +272,53 @@ func (e *EventRecorderAdapter) sendNotifications(notificationsAPI api.API, objec
 
 	objMap, err := toObjectMap(object)
 	if err != nil {
-		return err
+		return []error{err}
 	}
 
 	emptyCondition := hash("")
 
+	// We should not return in these loops because we want other configured notifications to still send if they can.
+	errors := []error{}
 	for _, destination := range destinations {
 		res, err := notificationsAPI.RunTrigger(trigger, objMap)
 		if err != nil {
-			log.Errorf("Failed to execute condition of trigger %s: %v", trigger, err)
-			return err
+			log.Errorf("Failed to run trigger, trigger: %s, destination: %s, namespace config: %s : %v",
+				trigger, destination, notificationsAPI.GetConfig().Namespace, err)
+			errors = append(errors, err)
+			continue
 		}
 		log.Infof("Trigger %s result: %v", trigger, res)
 
 		for _, c := range res {
-			log.Infof("Res When Condition hash: %s, Templates: %s", c.Key, c.Templates)
+			log.Infof("Result when condition hash: %s, templates: %s", c.Key, c.Templates)
 			s := strings.Split(c.Key, ".")[1]
 			if s != emptyCondition && c.Triggered == true {
 				err = notificationsAPI.Send(objMap, c.Templates, destination)
 				if err != nil {
-					log.Errorf("notification error: %s", err.Error())
-					return err
+					log.Errorf("Failed to execute the sending of notification on not empty condition, trigger: %s, destination: %s, namespace config: %s : %v",
+						trigger, destination, notificationsAPI.GetConfig().Namespace, err)
+					e.NotificationFailedCounter.WithLabelValues(namespace, name, opts.EventType, opts.EventReason).Inc()
+					errors = append(errors, err)
+					continue
 				}
+				e.NotificationSuccessCounter.WithLabelValues(namespace, name, opts.EventType, opts.EventReason).Inc()
 			} else if s == emptyCondition {
 				err = notificationsAPI.Send(objMap, c.Templates, destination)
 				if err != nil {
-					log.Errorf("notification error: %s", err.Error())
-					return err
+					log.Errorf("Failed to execute the sending of notification on empty condition, trigger: %s, destination: %s, namespace config: %s : %v",
+						trigger, destination, notificationsAPI.GetConfig().Namespace, err)
+					e.NotificationFailedCounter.WithLabelValues(namespace, name, opts.EventType, opts.EventReason).Inc()
+					errors = append(errors, err)
+					continue
 				}
+				e.NotificationSuccessCounter.WithLabelValues(namespace, name, opts.EventType, opts.EventReason).Inc()
 			}
 		}
 	}
-
-	return nil
+	if len(errors) == 0 {
+		return nil
+	}
+	return errors
 }
 
 // This function is copied over from notification engine to make sure we honour emptyCondition

utils/record/record_test.go

@@ -113,7 +113,7 @@ func TestSendNotifications(t *testing.T) {
 	rec.EventRecorderAdapter.apiFactory = apiFactory
 	//ch := make(chan prometheus.HistogramVec, 1)
 	err := rec.sendNotifications(mockAPI, &r, EventOptions{EventReason: "FooReason"})
-	assert.NoError(t, err)
+	assert.Nil(t, err)
 }
 
 func TestSendNotificationsWhenCondition(t *testing.T) {
@@ -140,7 +140,7 @@ func TestSendNotificationsWhenCondition(t *testing.T) {
 	rec.EventRecorderAdapter.apiFactory = apiFactory
 	//ch := make(chan prometheus.HistogramVec, 1)
 	err := rec.sendNotifications(mockAPI, &r, EventOptions{EventReason: "FooReason"})
-	assert.NoError(t, err)
+	assert.Nil(t, err)
 }
 
 func TestSendNotificationsWhenConditionTime(t *testing.T) {
@@ -340,7 +340,7 @@ func TestSendNotificationsFails(t *testing.T) {
 		rec.EventRecorderAdapter.apiFactory = apiFactory
 
 		err := rec.sendNotifications(mockAPI, &r, EventOptions{EventReason: "FooReason"})
-		assert.Error(t, err)
+		assert.Len(t, err, 1)
 	})
 
 	t.Run("GetAPIError", func(t *testing.T) {
@@ -349,7 +349,7 @@ func TestSendNotificationsFails(t *testing.T) {
 		rec.EventRecorderAdapter.apiFactory = apiFactory
 
 		err := rec.sendNotifications(nil, &r, EventOptions{EventReason: "FooReason"})
-		assert.Error(t, err)
+		assert.NotNil(t, err)
 	})
 
 }
@@ -380,7 +380,7 @@ func TestSendNotificationsFailsWithRunTriggerError(t *testing.T) {
 		rec.EventRecorderAdapter.apiFactory = apiFactory
 
 		err := rec.sendNotifications(mockAPI, &r, EventOptions{EventReason: "FooReason"})
-		assert.Error(t, err)
+		assert.Len(t, err, 1)
 	})
 
 	t.Run("GetAPIError", func(t *testing.T) {
@@ -389,7 +389,7 @@ func TestSendNotificationsFailsWithRunTriggerError(t *testing.T) {
 		rec.EventRecorderAdapter.apiFactory = apiFactory
 
 		err := rec.sendNotifications(nil, &r, EventOptions{EventReason: "FooReason"})
-		assert.Error(t, err)
+		assert.NotNil(t, err)
 	})
 
 }
@@ -419,7 +419,7 @@ func TestSendNotificationsNoTrigger(t *testing.T) {
 	rec.EventRecorderAdapter.apiFactory = apiFactory
 
 	err := rec.sendNotifications(mockAPI, &r, EventOptions{EventReason: "MissingReason"})
-	assert.Error(t, err)
+	assert.Len(t, err, 1)
 }
 
 func TestNewAPIFactorySettings(t *testing.T) {