diff --git a/pkg/predicate/predicate.go b/pkg/predicate/predicate.go index 9542d4a1c0..b5e8d014d3 100644 --- a/pkg/predicate/predicate.go +++ b/pkg/predicate/predicate.go @@ -40,6 +40,7 @@ type Predicate interface { var _ Predicate = Funcs{} var _ Predicate = ResourceVersionChangedPredicate{} +var _ Predicate = GenerationChangedPredicate{} // Funcs is a function that implements Predicate. type Funcs struct { @@ -116,3 +117,47 @@ func (ResourceVersionChangedPredicate) Update(e event.UpdateEvent) bool { } return true } + +// GenerationChangedPredicate implements a default update predicate function on Generation change. +// +// This predicate will skip update events that have no change in the object's metadata.generation field. +// The metadata.generation field of an object is incremented by the API server when writes are made to the spec field of an object. +// This allows a controller to ignore update events where the spec is unchanged, and only the metadata and/or status fields are changed. +// +// For CustomResource objects the Generation is only incremented when the status subresource is enabled. +// +// Caveats: +// +// * The assumption that the Generation is incremented only on writing to the spec does not hold for all APIs. +// E.g For Deployment objects the Generation is also incremented on writes to the metadata.annotations field. +// For object types other than CustomResources be sure to verify which fields will trigger a Generation increment when they are written to. +// +// * With this predicate, any update events with writes only to the status field will not be reconciled. +// So in the event that the status block is overwritten or wiped by someone else the controller will not self-correct to restore the correct status. +type GenerationChangedPredicate struct { + Funcs +} + +// Update implements default UpdateEvent filter for validating generation change +func (GenerationChangedPredicate) Update(e event.UpdateEvent) bool { + if e.MetaOld == 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) + return false + } + if e.ObjectNew == nil { + log.Error(nil, "Update event has no new runtime object for update", "event", e) + return false + } + if e.MetaNew == nil { + log.Error(nil, "Update event has no new metadata", "event", e) + return false + } + if e.MetaNew.GetGeneration() == e.MetaOld.GetGeneration() { + return false + } + return true +} diff --git a/pkg/predicate/predicate_test.go b/pkg/predicate/predicate_test.go index b66c462f4a..c43c88e9b1 100644 --- a/pkg/predicate/predicate_test.go +++ b/pkg/predicate/predicate_test.go @@ -308,4 +308,134 @@ var _ = Describe("Predicate", func() { }) }) + + Describe("When checking a GenerationChangedPredicate", func() { + instance := predicate.GenerationChangedPredicate{} + Context("Where the old object doesn't have a Generation or metadata", func() { + It("should return false", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 1, + }} + + failEvnt := event.UpdateEvent{ + MetaNew: new.GetObjectMeta(), + 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 doesn't have a Generation or metadata", func() { + It("should return false", func() { + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 1, + }} + + failEvnt := event.UpdateEvent{ + MetaOld: old.GetObjectMeta(), + 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 Generation hasn't changed", func() { + It("should return false", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 1, + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 1, + }} + + failEvnt := event.UpdateEvent{ + MetaOld: old.GetObjectMeta(), + ObjectOld: old, + MetaNew: new.GetObjectMeta(), + 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 Generation has changed", func() { + It("should return true", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 1, + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 2, + }} + passEvt := event.UpdateEvent{ + MetaOld: old.GetObjectMeta(), + ObjectOld: old, + MetaNew: new.GetObjectMeta(), + 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 the objects or metadata are missing", func() { + + It("should return false", func() { + new := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 1, + }} + + old := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "biz", + Generation: 1, + }} + + failEvt1 := event.UpdateEvent{MetaOld: old.GetObjectMeta(), ObjectOld: old, MetaNew: new.GetObjectMeta()} + failEvt2 := event.UpdateEvent{MetaOld: old.GetObjectMeta(), MetaNew: new.GetObjectMeta(), ObjectNew: new} + failEvt3 := event.UpdateEvent{MetaOld: old.GetObjectMeta(), 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(failEvt1)).To(BeFalse()) + Expect(instance.Update(failEvt2)).To(BeFalse()) + Expect(instance.Update(failEvt3)).To(BeFalse()) + }) + }) + + }) }) diff --git a/pkg/runtime/log/log.go b/pkg/runtime/log/log.go index 9e873c02c3..c5425fe06d 100644 --- a/pkg/runtime/log/log.go +++ b/pkg/runtime/log/log.go @@ -37,7 +37,7 @@ var ( // ZapLoggerTo returns a new Logger implementation using Zap which logs // to the given destination, instead of stderr. It otherwise behaves like // ZapLogger. - ZapLoggerTo = zap.Logger + ZapLoggerTo = zap.LoggerTo // SetLogger sets a concrete logging implementation for all deferred Loggers. SetLogger = log.SetLogger