Skip to content

Commit

Permalink
Enhance Update Event Notifications
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
codenio committed Aug 18, 2019
1 parent de2d778 commit 4dc0051
Show file tree
Hide file tree
Showing 11 changed files with 150 additions and 61 deletions.
4 changes: 3 additions & 1 deletion config.yaml
Expand Up @@ -19,13 +19,15 @@ resources:
- all
events:
- create
- update
- delete
- error
- name: statefulset
namespaces:
- all
events:
- create
- update
- delete
- error
- name: ingress
Expand Down Expand Up @@ -89,9 +91,9 @@ resources:
- all
events:
- create
- update
- delete
- error
- update
- name: role
namespaces:
- all
Expand Down
2 changes: 2 additions & 0 deletions deploy-all-in-one-tls.yaml
Expand Up @@ -29,13 +29,15 @@ data:
- all
events:
- create
- update
- delete
- error
- name: statefulset
namespaces:
- all
events:
- create
- update
- delete
- error
- name: ingress
Expand Down
2 changes: 2 additions & 0 deletions deploy-all-in-one.yaml
Expand Up @@ -29,13 +29,15 @@ data:
- all
events:
- create
- update
- delete
- error
- name: statefulset
namespaces:
- all
events:
- create
- update
- delete
- error
- name: ingress
Expand Down
1 change: 1 addition & 0 deletions go.mod
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion helm/botkube/values.yaml
Expand Up @@ -39,13 +39,15 @@ config:
- all
events:
- create
- update
- delete
- error
- name: statefulset
namespaces:
- all
events:
- create
- update
- delete
- error
- name: ingress
Expand Down Expand Up @@ -109,8 +111,8 @@ config:
- all
events:
- create
- delete
- update
- delete
- error
- name: role
namespaces:
Expand Down
29 changes: 18 additions & 11 deletions pkg/controller/controller.go
Expand Up @@ -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)
}
},
})
Expand All @@ -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}] &&
Expand All @@ -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)
Expand Down
49 changes: 1 addition & 48 deletions pkg/events/events.go
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions pkg/filterengine/filters/ingress_validator.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/filterengine/filters/job_status_checker.go
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions 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()
}
68 changes: 68 additions & 0 deletions pkg/utils/cmp_test.go
@@ -0,0 +1,68 @@
package utils

import (
"fmt"
"testing"
)

// Object mocks kubernetes objects
type Object struct {
Spec Spec
Other Other
}

// Other mocks fileds like MetaData, Status etc in kubernetes objects
type Other struct {
Foo string
}

// Spec mocks ObjectSpec field in kubernetes object
type Spec struct {
Port int
}

// ExpectedDiff struct to generate expected diff
type ExpectedDiff struct {
Path string
X string
Y string
}

func TestDiff(t *testing.T) {
tests := map[string]struct {
old Object
new Object
expected ExpectedDiff
}{
`Spec Diff`: {
old: Object{Spec: Spec{Port: 81}, Other: Other{Foo: "bar"}},
new: Object{Spec: Spec{Port: 83}, Other: Other{Foo: "bar"}},
expected: ExpectedDiff{
Path: "{utils.Object}.Spec.Port",
X: "81",
Y: "83",
},
},
`Non Spec Diff`: {
old: Object{Spec: Spec{Port: 81}, Other: Other{Foo: "bar"}},
new: Object{Spec: Spec{Port: 81}, Other: Other{Foo: "boo"}},
expected: ExpectedDiff{},
},
}
for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
if actual := Diff(test.old, test.new); actual != test.expected.MockDiff() {
t.Errorf("expected: %+v != actual: %+v\n", test.expected.MockDiff(), actual)
}
})
}
}

// MockDiff mocks utils.Diff
func (e *ExpectedDiff) MockDiff() string {
if e.Path == "" {
return ""
}
return fmt.Sprintf("%+v:\n\t-: %+v\n\t+: %+v\n", e.Path, e.X, e.Y)
}

0 comments on commit 4dc0051

Please sign in to comment.