Enhance Update Event Notifications

This Commit,
- enhances update event notifications with details of what Spec is getting changed
- adds custom `SpecDiffReporter` and `utils.Diff()` method.
- adds update events in all config.yaml files
- adds unit-test for utils.Diff()
- adds update events for Deployment, Statefulset and Job by default.

config.yaml

@@ -19,13 +19,15 @@ resources:
       - all
     events:
       - create
+      - update
       - delete
       - error
   - name: statefulset
     namespaces:
       - all
     events:
       - create
+      - update
       - delete
       - error
   - name: ingress
@@ -89,9 +91,9 @@ resources:
       - all
     events:
       - create
+      - update
       - delete
       - error
-      - update
   - name: role
     namespaces:
       - all

deploy-all-in-one-tls.yaml

@@ -29,13 +29,15 @@ data:
           - all
         events:
           - create
+          - update
           - delete
           - error
       - name: statefulset
         namespaces:
           - all
         events:
           - create
+          - update
           - delete
           - error
       - name: ingress

deploy-all-in-one.yaml

@@ -29,13 +29,15 @@ data:
           - all
         events:
           - create
+          - update
           - delete
           - error
       - name: statefulset
         namespaces:
           - all
         events:
           - create
+          - update
           - delete
           - error
       - name: ingress

go.mod

@@ -13,6 +13,7 @@ require (
 	github.com/go-sql-driver/mysql v1.4.1 // indirect
 	github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6 // indirect
 	github.com/golang/protobuf v1.3.2 // indirect
+	github.com/google/go-cmp v0.3.0
 	github.com/google/go-github/v27 v27.0.4
 	github.com/googleapis/gnostic v0.3.0 // indirect
 	github.com/gorilla/websocket v1.4.0 // indirect

helm/botkube/values.yaml

@@ -39,13 +39,15 @@ config:
         - all
       events:
         - create
+        - update
         - delete
         - error
     - name: statefulset
       namespaces:
         - all
       events:
         - create
+        - update
         - delete
         - error
     - name: ingress
@@ -109,8 +111,8 @@ config:
         - all
       events:
         - create
-        - delete
         - update
+        - delete
         - error
     - name: role
       namespaces:

pkg/controller/controller.go

@@ -74,7 +74,7 @@ func RegisterInformers(c *config.Config) {
 
 			// If event type is AllowedEventType and configured for the resource
 			if strings.ToLower(eventObj.Type) == config.AllowedEventType.String() {
-				sendEvent(obj, c, kind, config.ErrorEvent)
+				sendEvent(obj, nil, c, kind, config.ErrorEvent)
 			}
 		},
 	})
@@ -95,26 +95,29 @@ func registerEventHandlers(c *config.Config, resourceType string, events []confi
 	for _, event := range events {
 		if event == config.AllEvent || event == config.CreateEvent {
 			handlerFns.AddFunc = func(obj interface{}) {
-				sendEvent(obj, c, resourceType, config.CreateEvent)
+				log.Logger.Debugf("Processing add to %v", resourceType)
+				sendEvent(obj, nil, c, resourceType, config.CreateEvent)
 			}
 		}
 
 		if event == config.AllEvent || event == config.UpdateEvent {
 			handlerFns.UpdateFunc = func(old, new interface{}) {
-				sendEvent(new, c, resourceType, config.UpdateEvent)
+				log.Logger.Debugf("Processing update to %v\n Object: %+v\n", resourceType, new)
+				sendEvent(new, old, c, resourceType, config.UpdateEvent)
 			}
 		}
 
 		if event == config.AllEvent || event == config.DeleteEvent {
 			handlerFns.DeleteFunc = func(obj interface{}) {
-				sendEvent(obj, c, resourceType, config.DeleteEvent)
+				log.Logger.Debugf("Processing delete to %v", resourceType)
+				sendEvent(obj, nil, c, resourceType, config.DeleteEvent)
 			}
 		}
 	}
 	return handlerFns
 }
 
-func sendEvent(obj interface{}, c *config.Config, kind string, eventType config.EventType) {
+func sendEvent(obj, oldObj interface{}, c *config.Config, kind string, eventType config.EventType) {
 	// Filter namespaces
 	objectMeta := utils.GetObjectMetaData(obj)
 	if !utils.AllowedEventKindsMap[utils.EventKind{Resource: kind, Namespace: "all", EventType: eventType}] &&
@@ -140,12 +143,16 @@ func sendEvent(obj interface{}, c *config.Config, kind string, eventType config.
 		}
 	}
 
-	// After resync, Informer gets OnUpdate call, even if nothing changed.
-	// We need to skip update event if that is happened before current time.
-	// As a workaround, we will be ignoring update events older than 5s of current time.
-	if eventType == config.UpdateEvent && time.Now().Sub(event.TimeStamp).Seconds() > 5 {
-		log.Logger.Debug("Skipping older events")
-		return
+	// check for siginificant Update Events in objects
+	if eventType == config.UpdateEvent {
+		updateMsg := utils.Diff(oldObj, obj)
+		if len(updateMsg) > 0 {
+			event.Messages = append(event.Messages, updateMsg)
+		} else {
+			// skipping least significant update
+			log.Logger.Debug("skipping least significant Update event")
+			event.Skip = true
+		}
 	}
 
 	event = filterengine.DefaultFilterEngine.Run(obj, event)

pkg/events/events.go

@@ -109,40 +109,16 @@ func New(object interface{}, eventType config.EventType, kind string) Event {
 		event.TimeStamp = obj.LastTimestamp.Time
 	case *apiV1.Pod:
 		event.Kind = "Pod"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
 	case *apiV1.Node:
 		event.Kind = "Node"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
 	case *apiV1.Namespace:
 		event.Kind = "Namespace"
 	case *apiV1.PersistentVolume:
 		event.Kind = "PersistentVolume"
 	case *apiV1.PersistentVolumeClaim:
 		event.Kind = "PersistentVolumeClaim"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
 	case *apiV1.ReplicationController:
 		event.Kind = "ReplicationController"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
 	case *apiV1.Service:
 		event.Kind = "Service"
 	case *apiV1.Secret:
@@ -157,37 +133,14 @@ func New(object interface{}, eventType config.EventType, kind string) Event {
 		event.Kind = "DaemonSet"
 	case *appsV1.ReplicaSet:
 		event.Kind = "ReplicaSet"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
 	case *appsV1.Deployment:
 		event.Kind = "Deployment"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
 	case *appsV1.StatefulSet:
 		event.Kind = "StatefulSet"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
 
 	case *batchV1.Job:
 		event.Kind = "Job"
-		if eventType == config.UpdateEvent {
-			condLen := len(obj.Status.Conditions)
-			if condLen != 0 {
-				event.TimeStamp = obj.Status.Conditions[condLen-1].LastTransitionTime.Time
-			}
-		}
+
 	case *rbacV1.Role:
 		event.Kind = "Role"
 	case *rbacV1.RoleBinding:
@@ -242,23 +195,45 @@ func (event *Event) Message() (msg string) {
 			)
 		}
 	case config.ErrorEvent:
-		msg = fmt.Sprintf(
-			"Error Occurred in %s: `%s` of cluster `%s`, namespace `%s`:\n```%s``` ",
-			event.Kind,
-			event.Name,
-			event.Cluster,
-			event.Namespace,
-			message,
-		)
+		switch event.Kind {
+		case "Namespace", "Node", "PersistentVolume", "ClusterRole", "ClusterRoleBinding":
+			msg = fmt.Sprintf(
+				"Error Occurred in %s: `%s` of cluster `%s`:\n```%s``` ",
+				event.Kind,
+				event.Name,
+				event.Cluster,
+				message,
+			)
+		default:
+			msg = fmt.Sprintf(
+				"Error Occurred in %s: `%s` of cluster `%s`, namespace `%s`:\n```%s``` ",
+				event.Kind,
+				event.Name,
+				event.Cluster,
+				event.Namespace,
+				message,
+			)
+		}
 	case config.WarningEvent:
-		msg = fmt.Sprintf(
-			"Warning %s: `%s` of cluster `%s`, namespace `%s`:\n```%s``` ",
-			event.Kind,
-			event.Name,
-			event.Cluster,
-			event.Namespace,
-			message,
-		)
+		switch event.Kind {
+		case "Namespace", "Node", "PersistentVolume", "ClusterRole", "ClusterRoleBinding":
+			msg = fmt.Sprintf(
+				"Warning %s: `%s` of cluster `%s`:\n```%s``` ",
+				event.Kind,
+				event.Name,
+				event.Cluster,
+				message,
+			)
+		default:
+			msg = fmt.Sprintf(
+				"Warning %s: `%s` of cluster `%s`, namespace `%s`:\n```%s``` ",
+				event.Kind,
+				event.Name,
+				event.Cluster,
+				event.Namespace,
+				message,
+			)
+		}
 	}
 	return msg
 }

pkg/filterengine/filters/ingress_validator.go

@@ -3,6 +3,7 @@ package filters
 import (
 	"github.com/infracloudio/botkube/pkg/events"
 	"github.com/infracloudio/botkube/pkg/filterengine"
+	log "github.com/infracloudio/botkube/pkg/logging"
 	extV1beta1 "k8s.io/api/extensions/v1beta1"
 )
 
@@ -56,6 +57,7 @@ func (iv IngressValidator) Run(object interface{}, event *events.Event) {
 			event.Recommendations = append(event.Recommendations, "TLS secret "+tls.SecretName+"does not exist")
 		}
 	}
+	log.Logger.Debug("Ingress Validator filter successful!")
 }
 
 // Describe filter

pkg/filterengine/filters/job_status_checker.go

@@ -45,6 +45,8 @@ func (f JobStatusChecker) Run(object interface{}, event *events.Event) {
 	if c.Type == batchV1.JobComplete {
 		event.Messages = []string{"Job succeeded!"}
 		event.TimeStamp = c.LastTransitionTime.Time
+		// overwrite event.Skip in case of Job succeeded (Job update) events
+		event.Skip = false
 	} else {
 		event.Skip = true
 		return

pkg/utils/cmp.go

@@ -0,0 +1,48 @@
+package utils
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+// SpecDiffReporter is a simple custom reporter that only records differences
+// detected in Object Spec during comparison.
+type SpecDiffReporter struct {
+	path  cmp.Path
+	diffs []string
+}
+
+// PushStep custom implements Reporter interface
+func (r *SpecDiffReporter) PushStep(ps cmp.PathStep) {
+	r.path = append(r.path, ps)
+}
+
+// Report custom implements Reporter interface
+func (r *SpecDiffReporter) Report(rs cmp.Result) {
+	if !rs.Equal() {
+		vx, vy := r.path.Last().Values()
+		path := fmt.Sprintf("%#v", r.path)
+		if ok := strings.Contains(path, ".Spec."); ok {
+			r.diffs = append(r.diffs, fmt.Sprintf("%#v:\n\t-: %+v\n\t+: %+v\n", r.path, vx, vy))
+		}
+	}
+}
+
+// PopStep custom implements Reporter interface
+func (r *SpecDiffReporter) PopStep() {
+	r.path = r.path[:len(r.path)-1]
+}
+
+// String custom implements Reporter interface
+func (r *SpecDiffReporter) String() string {
+	return strings.Join(r.diffs, "\n")
+}
+
+// Diff provides differences between two objects spec
+func Diff(x, y interface{}) string {
+	var r SpecDiffReporter
+	cmp.Equal(x, y, cmp.Reporter(&r))
+	return r.String()
+}