diff --git a/pkg/builder/controller.go b/pkg/builder/controller.go index 0760953e02..6d906f6e52 100644 --- a/pkg/builder/controller.go +++ b/pkg/builder/controller.go @@ -163,7 +163,7 @@ func (blder *TypedBuilder[request]) Watches( ) *TypedBuilder[request] { input := WatchesInput[request]{ obj: object, - handler: handler.WithLowPriorityWhenUnchanged(eventHandler), + handler: eventHandler, } for _, opt := range opts { opt.ApplyToWatches(&input) @@ -317,7 +317,7 @@ func (blder *TypedBuilder[request]) doWatch() error { } var hdler handler.TypedEventHandler[client.Object, request] - reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{}))) + reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(&handler.EnqueueRequestForObject{})) allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...) allPredicates = append(allPredicates, blder.forInput.predicates...) src := source.TypedKind(blder.mgr.GetCache(), obj, hdler, allPredicates...) @@ -341,11 +341,11 @@ func (blder *TypedBuilder[request]) doWatch() error { } var hdler handler.TypedEventHandler[client.Object, request] - reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(handler.WithLowPriorityWhenUnchanged(handler.EnqueueRequestForOwner( + reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(handler.EnqueueRequestForOwner( blder.mgr.GetScheme(), blder.mgr.GetRESTMapper(), blder.forInput.object, opts..., - )))) + ))) allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...) allPredicates = append(allPredicates, own.predicates...) src := source.TypedKind(blder.mgr.GetCache(), obj, hdler, allPredicates...) diff --git a/pkg/handler/enqueue.go b/pkg/handler/enqueue.go index 1a1d1ab2f4..64cbe8a4d1 100644 --- a/pkg/handler/enqueue.go +++ b/pkg/handler/enqueue.go @@ -52,25 +52,32 @@ func (e *TypedEnqueueRequestForObject[T]) Create(ctx context.Context, evt event. enqueueLog.Error(nil, "CreateEvent received with no metadata", "event", evt) return } - q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + + item := reconcile.Request{NamespacedName: types.NamespacedName{ Name: evt.Object.GetName(), Namespace: evt.Object.GetNamespace(), - }}) + }} + + addToQueueCreate(q, evt, item) } // Update implements EventHandler. func (e *TypedEnqueueRequestForObject[T]) Update(ctx context.Context, evt event.TypedUpdateEvent[T], q workqueue.TypedRateLimitingInterface[reconcile.Request]) { switch { case !isNil(evt.ObjectNew): - q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + item := reconcile.Request{NamespacedName: types.NamespacedName{ Name: evt.ObjectNew.GetName(), Namespace: evt.ObjectNew.GetNamespace(), - }}) + }} + + addToQueueUpdate(q, evt, item) case !isNil(evt.ObjectOld): - q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + item := reconcile.Request{NamespacedName: types.NamespacedName{ Name: evt.ObjectOld.GetName(), Namespace: evt.ObjectOld.GetNamespace(), - }}) + }} + + addToQueueUpdate(q, evt, item) default: enqueueLog.Error(nil, "UpdateEvent received with no metadata", "event", evt) } diff --git a/pkg/handler/enqueue_mapped.go b/pkg/handler/enqueue_mapped.go index 491bc40c42..be97fa3781 100644 --- a/pkg/handler/enqueue_mapped.go +++ b/pkg/handler/enqueue_mapped.go @@ -21,6 +21,7 @@ import ( "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/priorityqueue" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -63,7 +64,8 @@ func EnqueueRequestsFromMapFunc(fn MapFunc) EventHandler { // TypedEnqueueRequestsFromMapFunc is experimental and subject to future change. func TypedEnqueueRequestsFromMapFunc[object any, request comparable](fn TypedMapFunc[object, request]) TypedEventHandler[object, request] { return &enqueueRequestsFromMapFunc[object, request]{ - toRequests: fn, + toRequests: fn, + objectImplementsClientObject: implementsClientObject[object](), } } @@ -71,7 +73,8 @@ var _ EventHandler = &enqueueRequestsFromMapFunc[client.Object, reconcile.Reques type enqueueRequestsFromMapFunc[object any, request comparable] struct { // Mapper transforms the argument into a slice of keys to be reconciled - toRequests TypedMapFunc[object, request] + toRequests TypedMapFunc[object, request] + objectImplementsClientObject bool } // Create implements EventHandler. @@ -81,7 +84,15 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Create( q workqueue.TypedRateLimitingInterface[request], ) { reqs := map[request]empty{} - e.mapAndEnqueue(ctx, q, evt.Object, reqs) + + var lowPriority bool + if e.objectImplementsClientObject && isPriorityQueue(q) && !isNil(evt.Object) { + clientObjectEvent := event.CreateEvent{Object: any(evt.Object).(client.Object)} + if isObjectUnchanged(clientObjectEvent) { + lowPriority = true + } + } + e.mapAndEnqueue(ctx, q, evt.Object, reqs, lowPriority) } // Update implements EventHandler. @@ -90,9 +101,13 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Update( evt event.TypedUpdateEvent[object], q workqueue.TypedRateLimitingInterface[request], ) { + var lowPriority bool + if e.objectImplementsClientObject && isPriorityQueue(q) && !isNil(evt.ObjectOld) && !isNil(evt.ObjectNew) { + lowPriority = any(evt.ObjectOld).(client.Object).GetResourceVersion() == any(evt.ObjectNew).(client.Object).GetResourceVersion() + } reqs := map[request]empty{} - e.mapAndEnqueue(ctx, q, evt.ObjectOld, reqs) - e.mapAndEnqueue(ctx, q, evt.ObjectNew, reqs) + e.mapAndEnqueue(ctx, q, evt.ObjectOld, reqs, lowPriority) + e.mapAndEnqueue(ctx, q, evt.ObjectNew, reqs, lowPriority) } // Delete implements EventHandler. @@ -102,7 +117,7 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Delete( q workqueue.TypedRateLimitingInterface[request], ) { reqs := map[request]empty{} - e.mapAndEnqueue(ctx, q, evt.Object, reqs) + e.mapAndEnqueue(ctx, q, evt.Object, reqs, false) } // Generic implements EventHandler. @@ -112,14 +127,26 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Generic( q workqueue.TypedRateLimitingInterface[request], ) { reqs := map[request]empty{} - e.mapAndEnqueue(ctx, q, evt.Object, reqs) + e.mapAndEnqueue(ctx, q, evt.Object, reqs, false) } -func (e *enqueueRequestsFromMapFunc[object, request]) mapAndEnqueue(ctx context.Context, q workqueue.TypedRateLimitingInterface[request], o object, reqs map[request]empty) { +func (e *enqueueRequestsFromMapFunc[object, request]) mapAndEnqueue( + ctx context.Context, + q workqueue.TypedRateLimitingInterface[request], + o object, + reqs map[request]empty, + lowPriority bool, +) { for _, req := range e.toRequests(ctx, o) { _, ok := reqs[req] if !ok { - q.Add(req) + if lowPriority { + q.(priorityqueue.PriorityQueue[request]).AddWithOpts(priorityqueue.AddOpts{ + Priority: LowPriority, + }, req) + } else { + q.Add(req) + } reqs[req] = empty{} } } diff --git a/pkg/handler/enqueue_owner.go b/pkg/handler/enqueue_owner.go index 1680043b46..e8fc8eb46e 100644 --- a/pkg/handler/enqueue_owner.go +++ b/pkg/handler/enqueue_owner.go @@ -72,7 +72,7 @@ func TypedEnqueueRequestForOwner[object client.Object](scheme *runtime.Scheme, m for _, opt := range opts { opt(e) } - return e + return WithLowPriorityWhenUnchanged(e) } // OnlyControllerOwner if provided will only look at the first OwnerReference with Controller: true. diff --git a/pkg/handler/eventhandler.go b/pkg/handler/eventhandler.go index 57107f20e9..84a10ac077 100644 --- a/pkg/handler/eventhandler.go +++ b/pkg/handler/eventhandler.go @@ -18,6 +18,7 @@ package handler import ( "context" + "reflect" "time" "k8s.io/client-go/util/workqueue" @@ -108,10 +109,46 @@ type TypedFuncs[object any, request comparable] struct { GenericFunc func(context.Context, event.TypedGenericEvent[object], workqueue.TypedRateLimitingInterface[request]) } +var typeForClientObject = reflect.TypeFor[client.Object]() + +func implementsClientObject[object any]() bool { + return reflect.TypeFor[object]().Implements(typeForClientObject) +} + +func isPriorityQueue[request comparable](q workqueue.TypedRateLimitingInterface[request]) bool { + _, ok := q.(priorityqueue.PriorityQueue[request]) + return ok +} + // Create implements EventHandler. func (h TypedFuncs[object, request]) Create(ctx context.Context, e event.TypedCreateEvent[object], q workqueue.TypedRateLimitingInterface[request]) { if h.CreateFunc != nil { - h.CreateFunc(ctx, e, q) + if !implementsClientObject[object]() || !isPriorityQueue(q) || isNil(e.Object) { + h.CreateFunc(ctx, e, q) + return + } + wq := workqueueWithCustomAddFunc[request]{ + TypedRateLimitingInterface: q, + // We already know that we have a priority queue, that event.Object implements + // client.Object and that its not nil + addFunc: func(item request, q workqueue.TypedRateLimitingInterface[request]) { + // We construct a new event typed to client.Object because isObjectUnchanged + // is a generic and hence has to know at compile time the type of the event + // it gets. We only figure that out at runtime though, but we know for sure + // that it implements client.Object at this point so we can hardcode the event + // type to that. + evt := event.CreateEvent{Object: any(e.Object).(client.Object)} + var priority int + if isObjectUnchanged(evt) { + priority = LowPriority + } + q.(priorityqueue.PriorityQueue[request]).AddWithOpts( + priorityqueue.AddOpts{Priority: priority}, + item, + ) + }, + } + h.CreateFunc(ctx, e, wq) } } @@ -125,7 +162,27 @@ func (h TypedFuncs[object, request]) Delete(ctx context.Context, e event.TypedDe // Update implements EventHandler. func (h TypedFuncs[object, request]) Update(ctx context.Context, e event.TypedUpdateEvent[object], q workqueue.TypedRateLimitingInterface[request]) { if h.UpdateFunc != nil { - h.UpdateFunc(ctx, e, q) + if !implementsClientObject[object]() || !isPriorityQueue(q) || isNil(e.ObjectOld) || isNil(e.ObjectNew) { + h.UpdateFunc(ctx, e, q) + return + } + + wq := workqueueWithCustomAddFunc[request]{ + TypedRateLimitingInterface: q, + // We already know that we have a priority queue, that event.ObjectOld and ObjectNew implement + // client.Object and that they are not nil + addFunc: func(item request, q workqueue.TypedRateLimitingInterface[request]) { + var priority int + if any(e.ObjectOld).(client.Object).GetResourceVersion() == any(e.ObjectNew).(client.Object).GetResourceVersion() { + priority = LowPriority + } + q.(priorityqueue.PriorityQueue[request]).AddWithOpts( + priorityqueue.AddOpts{Priority: priority}, + item, + ) + }, + } + h.UpdateFunc(ctx, e, wq) } } @@ -142,43 +199,10 @@ const LowPriority = -100 // WithLowPriorityWhenUnchanged reduces the priority of events stemming from the initial listwatch or from a resync if // and only if a priorityqueue.PriorityQueue is used. If not, it does nothing. func WithLowPriorityWhenUnchanged[object client.Object, request comparable](u TypedEventHandler[object, request]) TypedEventHandler[object, request] { + // TypedFuncs already implements this so just wrap return TypedFuncs[object, request]{ - CreateFunc: func(ctx context.Context, tce event.TypedCreateEvent[object], trli workqueue.TypedRateLimitingInterface[request]) { - // Due to how the handlers are factored, we have to wrap the workqueue to be able - // to inject custom behavior. - u.Create(ctx, tce, workqueueWithCustomAddFunc[request]{ - TypedRateLimitingInterface: trli, - addFunc: func(item request, q workqueue.TypedRateLimitingInterface[request]) { - priorityQueue, isPriorityQueue := q.(priorityqueue.PriorityQueue[request]) - if !isPriorityQueue { - q.Add(item) - return - } - var priority int - if isObjectUnchanged(tce) { - priority = LowPriority - } - priorityQueue.AddWithOpts(priorityqueue.AddOpts{Priority: priority}, item) - }, - }) - }, - UpdateFunc: func(ctx context.Context, tue event.TypedUpdateEvent[object], trli workqueue.TypedRateLimitingInterface[request]) { - u.Update(ctx, tue, workqueueWithCustomAddFunc[request]{ - TypedRateLimitingInterface: trli, - addFunc: func(item request, q workqueue.TypedRateLimitingInterface[request]) { - priorityQueue, isPriorityQueue := q.(priorityqueue.PriorityQueue[request]) - if !isPriorityQueue { - q.Add(item) - return - } - var priority int - if tue.ObjectOld.GetResourceVersion() == tue.ObjectNew.GetResourceVersion() { - priority = LowPriority - } - priorityQueue.AddWithOpts(priorityqueue.AddOpts{Priority: priority}, item) - }, - }) - }, + CreateFunc: u.Create, + UpdateFunc: u.Update, DeleteFunc: u.Delete, GenericFunc: u.Generic, } @@ -199,3 +223,35 @@ func (w workqueueWithCustomAddFunc[request]) Add(item request) { func isObjectUnchanged[object client.Object](e event.TypedCreateEvent[object]) bool { return e.Object.GetCreationTimestamp().Time.Before(time.Now().Add(-time.Minute)) } + +// addToQueueCreate adds the reconcile.Request to the priorityqueue in the handler +// for Create requests if and only if the workqueue being used is of type priorityqueue.PriorityQueue[reconcile.Request] +func addToQueueCreate[T client.Object, request comparable](q workqueue.TypedRateLimitingInterface[request], evt event.TypedCreateEvent[T], item request) { + priorityQueue, isPriorityQueue := q.(priorityqueue.PriorityQueue[request]) + if !isPriorityQueue { + q.Add(item) + return + } + + var priority int + if isObjectUnchanged(evt) { + priority = LowPriority + } + priorityQueue.AddWithOpts(priorityqueue.AddOpts{Priority: priority}, item) +} + +// addToQueueUpdate adds the reconcile.Request to the priorityqueue in the handler +// for Update requests if and only if the workqueue being used is of type priorityqueue.PriorityQueue[reconcile.Request] +func addToQueueUpdate[T client.Object, request comparable](q workqueue.TypedRateLimitingInterface[request], evt event.TypedUpdateEvent[T], item request) { + priorityQueue, isPriorityQueue := q.(priorityqueue.PriorityQueue[request]) + if !isPriorityQueue { + q.Add(item) + return + } + + var priority int + if evt.ObjectOld.GetResourceVersion() == evt.ObjectNew.GetResourceVersion() { + priority = LowPriority + } + priorityQueue.AddWithOpts(priorityqueue.AddOpts{Priority: priority}, item) +} diff --git a/pkg/handler/eventhandler_test.go b/pkg/handler/eventhandler_test.go index 6e57c22c3b..748aa3fcc4 100644 --- a/pkg/handler/eventhandler_test.go +++ b/pkg/handler/eventhandler_test.go @@ -659,7 +659,7 @@ var _ = Describe("Eventhandler", func() { }) Describe("Funcs", func() { - failingFuncs := handler.Funcs{ + failingFuncs := handler.TypedFuncs[client.Object, reconcile.Request]{ CreateFunc: func(context.Context, event.CreateEvent, workqueue.TypedRateLimitingInterface[reconcile.Request]) { defer GinkgoRecover() Fail("Did not expect CreateEvent to be called.") @@ -776,128 +776,224 @@ var _ = Describe("Eventhandler", func() { }) Describe("WithLowPriorityWhenUnchanged", func() { - It("should lower the priority of a create request for an object that was created more than one minute in the past", func() { - actualOpts := priorityqueue.AddOpts{} - var actualRequests []reconcile.Request - wq := &fakePriorityQueue{ - addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { - actualOpts = o - actualRequests = items + handlerPriorityTests := []struct { + name string + handler func() handler.EventHandler + }{ + { + name: "WithLowPriorityWhenUnchanged wrapper", + handler: func() handler.EventHandler { return handler.WithLowPriorityWhenUnchanged(customHandler{}) }, + }, + { + name: "EnqueueRequestForObject", + handler: func() handler.EventHandler { return &handler.EnqueueRequestForObject{} }, + }, + { + name: "EnqueueRequestForOwner", + handler: func() handler.EventHandler { + return handler.EnqueueRequestForOwner( + scheme.Scheme, + mapper, + &corev1.Pod{}, + ) }, - } - - h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{}) - h.Create(ctx, event.CreateEvent{ - Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - }}, - }, wq) - - Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority})) - Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) - }) - - It("should not lower the priority of a create request for an object that was created less than one minute in the past", func() { - actualOpts := priorityqueue.AddOpts{} - var actualRequests []reconcile.Request - wq := &fakePriorityQueue{ - addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { - actualOpts = o - actualRequests = items + }, + { + name: "TypedEnqueueRequestForOwner", + handler: func() handler.EventHandler { + return handler.TypedEnqueueRequestForOwner[client.Object]( + scheme.Scheme, + mapper, + &corev1.Pod{}, + ) }, - } - - h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{}) - h.Create(ctx, event.CreateEvent{ - Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - CreationTimestamp: metav1.Now(), - }}, - }, wq) - - Expect(actualOpts).To(Equal(priorityqueue.AddOpts{})) - Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) - }) - - It("should lower the priority of an update request with unchanged RV", func() { - actualOpts := priorityqueue.AddOpts{} - var actualRequests []reconcile.Request - wq := &fakePriorityQueue{ - addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { - actualOpts = o - actualRequests = items + }, + { + name: "Funcs", + handler: func() handler.EventHandler { + return handler.TypedFuncs[client.Object, reconcile.Request]{ + CreateFunc: func(ctx context.Context, tce event.TypedCreateEvent[client.Object], wq workqueue.TypedRateLimitingInterface[reconcile.Request]) { + wq.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: tce.Object.GetNamespace(), + Name: tce.Object.GetName(), + }}) + }, + UpdateFunc: func(ctx context.Context, tue event.TypedUpdateEvent[client.Object], wq workqueue.TypedRateLimitingInterface[reconcile.Request]) { + wq.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: tue.ObjectNew.GetNamespace(), + Name: tue.ObjectNew.GetName(), + }}) + }, + } }, - } + }, + { + name: "EnqueueRequestsFromMapFunc", + handler: func() handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []reconcile.Request { + return []reconcile.Request{{NamespacedName: types.NamespacedName{ + Name: obj.GetName(), + Namespace: obj.GetNamespace(), + }}} + }) + }, + }, + } + for _, test := range handlerPriorityTests { + When("handler is "+test.name, func() { + It("should lower the priority of a create request for an object that was created more than one minute in the past", func() { + actualOpts := priorityqueue.AddOpts{} + var actualRequests []reconcile.Request + wq := &fakePriorityQueue{ + addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { + actualOpts = o + actualRequests = items + }, + } - h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{}) - h.Update(ctx, event.UpdateEvent{ - ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - }}, - ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - }}, - }, wq) + test.handler().Create(ctx, event.CreateEvent{ + Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + }, wq) + + Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority})) + Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) + }) - Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority})) - Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) - }) + It("should not lower the priority of a create request for an object that was created less than one minute in the past", func() { + actualOpts := priorityqueue.AddOpts{} + var actualRequests []reconcile.Request + wq := &fakePriorityQueue{ + addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { + actualOpts = o + actualRequests = items + }, + } - It("should not lower the priority of an update request with changed RV", func() { - actualOpts := priorityqueue.AddOpts{} - var actualRequests []reconcile.Request - wq := &fakePriorityQueue{ - addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { - actualOpts = o - actualRequests = items - }, - } + test.handler().Create(ctx, event.CreateEvent{ + Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + CreationTimestamp: metav1.Now(), + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + }, wq) + + Expect(actualOpts).To(Equal(priorityqueue.AddOpts{})) + Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) + }) - h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{}) - h.Update(ctx, event.UpdateEvent{ - ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - }}, - ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - ResourceVersion: "1", - }}, - }, wq) + It("should lower the priority of an update request with unchanged RV", func() { + actualOpts := priorityqueue.AddOpts{} + var actualRequests []reconcile.Request + wq := &fakePriorityQueue{ + addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { + actualOpts = o + actualRequests = items + }, + } - Expect(actualOpts).To(Equal(priorityqueue.AddOpts{})) - Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) - }) + test.handler().Update(ctx, event.UpdateEvent{ + ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + }, wq) + + Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority})) + Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) + }) - It("should have no effect on create if the workqueue is not a priorityqueue", func() { - h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{}) - h.Create(ctx, event.CreateEvent{ - Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - }}, - }, q) + It("should not lower the priority of an update request with changed RV", func() { + actualOpts := priorityqueue.AddOpts{} + var actualRequests []reconcile.Request + wq := &fakePriorityQueue{ + addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) { + actualOpts = o + actualRequests = items + }, + } - Expect(q.Len()).To(Equal(1)) - item, _ := q.Get() - Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}})) - }) + test.handler().Update(ctx, event.UpdateEvent{ + ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + ResourceVersion: "1", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + }, wq) + + Expect(actualOpts).To(Equal(priorityqueue.AddOpts{})) + Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}})) + }) - It("should have no effect on Update if the workqueue is not a priorityqueue", func() { - h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{}) - h.Update(ctx, event.UpdateEvent{ - ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - }}, - ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ - Name: "my-pod", - }}, - }, q) + It("should have no effect on create if the workqueue is not a priorityqueue", func() { + test.handler().Create(ctx, event.CreateEvent{ + Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + }, q) + + Expect(q.Len()).To(Equal(1)) + item, _ := q.Get() + Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}})) + }) - Expect(q.Len()).To(Equal(1)) - item, _ := q.Get() - Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}})) - }) + It("should have no effect on Update if the workqueue is not a priorityqueue", func() { + test.handler().Update(ctx, event.UpdateEvent{ + ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "Pod", + Name: "my-pod", + }}, + }}, + }, q) + + Expect(q.Len()).To(Equal(1)) + item, _ := q.Get() + Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}})) + }) + }) + } }) - }) type fakePriorityQueue struct { @@ -905,9 +1001,42 @@ type fakePriorityQueue struct { addWithOpts func(o priorityqueue.AddOpts, items ...reconcile.Request) } +func (f *fakePriorityQueue) Add(item reconcile.Request) { + f.AddWithOpts(priorityqueue.AddOpts{}, item) +} + func (f *fakePriorityQueue) AddWithOpts(o priorityqueue.AddOpts, items ...reconcile.Request) { f.addWithOpts(o, items...) } func (f *fakePriorityQueue) GetWithPriority() (item reconcile.Request, priority int, shutdown bool) { panic("GetWithPriority is not expected to be called") } + +// customHandler re-implements the basic enqueueRequestForObject logic +// to be able to test the WithLowPriorityWhenUnchanged wrapper +type customHandler struct{} + +func (ch customHandler) Create(ctx context.Context, evt event.CreateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: evt.Object.GetNamespace(), + Name: evt.Object.GetName(), + }}) +} +func (ch customHandler) Update(ctx context.Context, evt event.UpdateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: evt.ObjectNew.GetNamespace(), + Name: evt.ObjectNew.GetName(), + }}) +} +func (ch customHandler) Delete(ctx context.Context, evt event.DeleteEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: evt.Object.GetNamespace(), + Name: evt.Object.GetName(), + }}) +} +func (ch customHandler) Generic(ctx context.Context, evt event.GenericEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: evt.Object.GetNamespace(), + Name: evt.Object.GetName(), + }}) +}