From ac788de041221e35c291da6409bce83376d481b9 Mon Sep 17 00:00:00 2001 From: Zvi Cahana Date: Mon, 9 Nov 2020 18:29:53 +0200 Subject: [PATCH 1/2] Cleanup some code leftovers from a previous refactor (#1118) --- pkg/predicate/predicate.go | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/pkg/predicate/predicate.go b/pkg/predicate/predicate.go index 079ef23df0..cc03fb8beb 100644 --- a/pkg/predicate/predicate.go +++ b/pkg/predicate/predicate.go @@ -122,21 +122,14 @@ type ResourceVersionChangedPredicate struct { // Update implements default UpdateEvent filter for validating resource version change func (ResourceVersionChangedPredicate) Update(e event.UpdateEvent) bool { if e.ObjectOld == nil { - log.Error(nil, "UpdateEvent has no old metadata", "event", e) - return false - } - if e.ObjectOld == nil { - log.Error(nil, "GenericEvent has no old runtime object to update", "event", e) + log.Error(nil, "Update event has no old object to update", "event", e) return false } if e.ObjectNew == nil { - log.Error(nil, "GenericEvent has no new runtime object for update", "event", e) - return false - } - if e.ObjectNew == nil { - log.Error(nil, "UpdateEvent has no new metadata", "event", e) + log.Error(nil, "Update event has no new object to update", "event", e) return false } + return e.ObjectNew.GetResourceVersion() != e.ObjectOld.GetResourceVersion() } @@ -163,21 +156,14 @@ type GenerationChangedPredicate struct { // Update implements default UpdateEvent filter for validating generation change func (GenerationChangedPredicate) Update(e event.UpdateEvent) bool { if e.ObjectOld == nil { - log.Error(nil, "Update event has no old metadata", "event", e) - return false - } - if e.ObjectOld == nil { - log.Error(nil, "Update event has no old runtime object to update", "event", e) + log.Error(nil, "Update event has no old object to update", "event", e) return false } if e.ObjectNew == nil { - log.Error(nil, "Update event has no new runtime object for update", "event", e) - return false - } - if e.ObjectNew == nil { - log.Error(nil, "Update event has no new metadata", "event", e) + log.Error(nil, "Update event has no new object for update", "event", e) return false } + return e.ObjectNew.GetGeneration() != e.ObjectOld.GetGeneration() } From a07ed0d417d30f349b4772dabef5fe418009c3b9 Mon Sep 17 00:00:00 2001 From: Zvi Cahana Date: Mon, 9 Nov 2020 21:32:27 +0200 Subject: [PATCH 2/2] Add predicate for annotations change on update event Signed-off-by: Zvi Cahana --- pkg/predicate/predicate.go | 33 ++++++ pkg/predicate/predicate_test.go | 196 ++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) diff --git a/pkg/predicate/predicate.go b/pkg/predicate/predicate.go index cc03fb8beb..cdc4d8efdb 100644 --- a/pkg/predicate/predicate.go +++ b/pkg/predicate/predicate.go @@ -17,6 +17,8 @@ limitations under the License. package predicate import ( + "reflect" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" @@ -44,6 +46,7 @@ type Predicate interface { var _ Predicate = Funcs{} var _ Predicate = ResourceVersionChangedPredicate{} var _ Predicate = GenerationChangedPredicate{} +var _ Predicate = AnnotationChangedPredicate{} var _ Predicate = or{} var _ Predicate = and{} @@ -167,6 +170,36 @@ func (GenerationChangedPredicate) Update(e event.UpdateEvent) bool { return e.ObjectNew.GetGeneration() != e.ObjectOld.GetGeneration() } +// AnnotationChangedPredicate implements a default update predicate function on annotation change. +// +// This predicate will skip update events that have no change in the object's annotation. +// It is intended to be used in conjunction with the GenerationChangedPredicate, as in the following example: +// +// Controller.Watch( +// &source.Kind{Type: v1.MyCustomKind}, +// &handler.EnqueueRequestForObject{}, +// predicate.Or(predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{})) +// +// This is mostly useful for controllers that needs to trigger both when the resource's generation is incremented +// (i.e., when the resource' .spec changes), or an annotation changes (e.g., for a staging/alpha API). +type AnnotationChangedPredicate struct { + Funcs +} + +// Update implements default UpdateEvent filter for validating annotation change +func (AnnotationChangedPredicate) Update(e event.UpdateEvent) bool { + if e.ObjectOld == nil { + log.Error(nil, "Update event has no old object to update", "event", e) + return false + } + if e.ObjectNew == nil { + log.Error(nil, "Update event has no new object for update", "event", e) + return false + } + + return !reflect.DeepEqual(e.ObjectNew.GetAnnotations(), e.ObjectOld.GetAnnotations()) +} + // And returns a composite predicate that implements a logical AND of the predicates passed to it. func And(predicates ...Predicate) Predicate { return and{predicates} diff --git a/pkg/predicate/predicate_test.go b/pkg/predicate/predicate_test.go index c4419a6435..5465a0ba32 100644 --- a/pkg/predicate/predicate_test.go +++ b/pkg/predicate/predicate_test.go @@ -414,6 +414,202 @@ var _ = Describe("Predicate", func() { }) + Describe("When checking an AnnotationChangedPredicate", func() { + instance := predicate.AnnotationChangedPredicate{} + Context("Where the old object is missing", func() { + It("should return false", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + }, + }} + + failEvnt := event.UpdateEvent{ + ObjectNew: new, + } + Expect(instance.Create(event.CreateEvent{})).To(BeTrue()) + Expect(instance.Delete(event.DeleteEvent{})).To(BeTrue()) + Expect(instance.Generic(event.GenericEvent{})).To(BeTrue()) + Expect(instance.Update(failEvnt)).To(BeFalse()) + }) + }) + + Context("Where the new object is missing", func() { + It("should return false", func() { + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + }, + }} + + failEvnt := event.UpdateEvent{ + ObjectOld: old, + } + Expect(instance.Create(event.CreateEvent{})).To(BeTrue()) + Expect(instance.Delete(event.DeleteEvent{})).To(BeTrue()) + Expect(instance.Generic(event.GenericEvent{})).To(BeTrue()) + Expect(instance.Update(failEvnt)).To(BeFalse()) + }) + }) + + Context("Where the annotations are empty", func() { + It("should return false", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + }} + + failEvnt := event.UpdateEvent{ + ObjectOld: old, + ObjectNew: new, + } + Expect(instance.Create(event.CreateEvent{})).To(BeTrue()) + Expect(instance.Delete(event.DeleteEvent{})).To(BeTrue()) + Expect(instance.Generic(event.GenericEvent{})).To(BeTrue()) + Expect(instance.Update(failEvnt)).To(BeFalse()) + }) + }) + + Context("Where the annotations haven't changed", func() { + It("should return false", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + }, + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + }, + }} + + failEvnt := event.UpdateEvent{ + ObjectOld: old, + ObjectNew: new, + } + Expect(instance.Create(event.CreateEvent{})).To(BeTrue()) + Expect(instance.Delete(event.DeleteEvent{})).To(BeTrue()) + Expect(instance.Generic(event.GenericEvent{})).To(BeTrue()) + Expect(instance.Update(failEvnt)).To(BeFalse()) + }) + }) + + Context("Where an annotation value has changed", func() { + It("should return true", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + }, + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "weez", + }, + }} + + passEvt := event.UpdateEvent{ + ObjectOld: old, + ObjectNew: new, + } + Expect(instance.Create(event.CreateEvent{})).To(BeTrue()) + Expect(instance.Delete(event.DeleteEvent{})).To(BeTrue()) + Expect(instance.Generic(event.GenericEvent{})).To(BeTrue()) + Expect(instance.Update(passEvt)).To(BeTrue()) + }) + }) + + Context("Where an annotation has been added", func() { + It("should return true", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + }, + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + "zooz": "qooz", + }, + }} + + passEvt := event.UpdateEvent{ + ObjectOld: old, + ObjectNew: new, + } + Expect(instance.Create(event.CreateEvent{})).To(BeTrue()) + Expect(instance.Delete(event.DeleteEvent{})).To(BeTrue()) + Expect(instance.Generic(event.GenericEvent{})).To(BeTrue()) + Expect(instance.Update(passEvt)).To(BeTrue()) + }) + }) + + Context("Where an annotation has been removed", func() { + It("should return true", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + "zooz": "qooz", + }, + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Annotations: map[string]string{ + "booz": "wooz", + }, + }} + + passEvt := event.UpdateEvent{ + ObjectOld: old, + ObjectNew: new, + } + Expect(instance.Create(event.CreateEvent{})).To(BeTrue()) + Expect(instance.Delete(event.DeleteEvent{})).To(BeTrue()) + Expect(instance.Generic(event.GenericEvent{})).To(BeTrue()) + Expect(instance.Update(passEvt)).To(BeTrue()) + }) + }) + }) + Context("With a boolean predicate", func() { funcs := func(pass bool) predicate.Funcs { return predicate.Funcs{