Skip to content

Commit 2af3164

Browse files
alvaroalemanalvaroaleman
committed
🌱 Followups to default low priority in mappers
Addresses Stefans comments
1 parent 29debb1 commit 2af3164

File tree

3 files changed

+16
-253
lines changed

3 files changed

+16
-253
lines changed

‎pkg/handler/enqueue_mapped.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package handler
1818

1919
import (
2020
"context"
21-
"reflect"
2221

2322
"k8s.io/client-go/util/workqueue"
2423
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -65,15 +64,17 @@ func EnqueueRequestsFromMapFunc(fn MapFunc) EventHandler {
6564
// TypedEnqueueRequestsFromMapFunc is experimental and subject to future change.
6665
func TypedEnqueueRequestsFromMapFunc[object any, request comparable](fn TypedMapFunc[object, request]) TypedEventHandler[object, request] {
6766
return &enqueueRequestsFromMapFunc[object, request]{
68-
toRequests: fn,
67+
toRequests: fn,
68+
objectImplementsClientObject: implementsClientObject[object](),
6969
}
7070
}
7171

7272
var _ EventHandler = &enqueueRequestsFromMapFunc[client.Object, reconcile.Request]{}
7373

7474
type enqueueRequestsFromMapFunc[object any, request comparable] struct {
7575
// Mapper transforms the argument into a slice of keys to be reconciled
76-
toRequests TypedMapFunc[object, request]
76+
toRequests TypedMapFunc[object, request]
77+
objectImplementsClientObject bool
7778
}
7879

7980
// Create implements EventHandler.
@@ -85,7 +86,7 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Create(
8586
reqs := map[request]empty{}
8687

8788
var lowPriority bool
88-
if reflect.TypeFor[object]().Implements(reflect.TypeFor[client.Object]()) && isPriorityQueue(q) && !isNil(evt.Object) {
89+
if e.objectImplementsClientObject && isPriorityQueue(q) && !isNil(evt.Object) {
8990
clientObjectEvent := event.CreateEvent{Object: any(evt.Object).(client.Object)}
9091
if isObjectUnchanged(clientObjectEvent) {
9192
lowPriority = true
@@ -101,7 +102,7 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Update(
101102
q workqueue.TypedRateLimitingInterface[request],
102103
) {
103104
var lowPriority bool
104-
if reflect.TypeFor[object]().Implements(reflect.TypeFor[client.Object]()) && isPriorityQueue(q) && !isNil(evt.ObjectOld) && !isNil(evt.ObjectNew) {
105+
if e.objectImplementsClientObject && isPriorityQueue(q) && !isNil(evt.ObjectOld) && !isNil(evt.ObjectNew) {
105106
lowPriority = any(evt.ObjectOld).(client.Object).GetResourceVersion() == any(evt.ObjectNew).(client.Object).GetResourceVersion()
106107
}
107108
reqs := map[request]empty{}
@@ -134,12 +135,12 @@ func (e *enqueueRequestsFromMapFunc[object, request]) mapAndEnqueue(
134135
q workqueue.TypedRateLimitingInterface[request],
135136
o object,
136137
reqs map[request]empty,
137-
unchanged bool,
138+
lowPriority bool,
138139
) {
139140
for _, req := range e.toRequests(ctx, o) {
140141
_, ok := reqs[req]
141142
if !ok {
142-
if unchanged {
143+
if lowPriority {
143144
q.(priorityqueue.PriorityQueue[request]).AddWithOpts(priorityqueue.AddOpts{
144145
Priority: LowPriority,
145146
}, req)

‎pkg/handler/eventhandler.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,12 @@ type TypedFuncs[object any, request comparable] struct {
109109
GenericFunc func(context.Context, event.TypedGenericEvent[object], workqueue.TypedRateLimitingInterface[request])
110110
}
111111

112+
var typeForClientObject = reflect.TypeFor[client.Object]()
113+
114+
func implementsClientObject[object any]() bool {
115+
return reflect.TypeFor[object]().Implements(typeForClientObject)
116+
}
117+
112118
func isPriorityQueue[request comparable](q workqueue.TypedRateLimitingInterface[request]) bool {
113119
_, ok := q.(priorityqueue.PriorityQueue[request])
114120
return ok
@@ -117,7 +123,7 @@ func isPriorityQueue[request comparable](q workqueue.TypedRateLimitingInterface[
117123
// Create implements EventHandler.
118124
func (h TypedFuncs[object, request]) Create(ctx context.Context, e event.TypedCreateEvent[object], q workqueue.TypedRateLimitingInterface[request]) {
119125
if h.CreateFunc != nil {
120-
if !reflect.TypeFor[object]().Implements(reflect.TypeFor[client.Object]()) || !isPriorityQueue(q) || isNil(e.Object) {
126+
if !implementsClientObject[object]() || !isPriorityQueue(q) || isNil(e.Object) {
121127
h.CreateFunc(ctx, e, q)
122128
return
123129
}
@@ -156,7 +162,7 @@ func (h TypedFuncs[object, request]) Delete(ctx context.Context, e event.TypedDe
156162
// Update implements EventHandler.
157163
func (h TypedFuncs[object, request]) Update(ctx context.Context, e event.TypedUpdateEvent[object], q workqueue.TypedRateLimitingInterface[request]) {
158164
if h.UpdateFunc != nil {
159-
if !reflect.TypeFor[object]().Implements(reflect.TypeFor[client.Object]()) || !isPriorityQueue(q) || isNil(e.ObjectOld) || isNil(e.ObjectNew) {
165+
if !implementsClientObject[object]() || !isPriorityQueue(q) || isNil(e.ObjectOld) || isNil(e.ObjectNew) {
160166
h.UpdateFunc(ctx, e, q)
161167
return
162168
}

‎pkg/handler/eventhandler_test.go

-244
Original file line numberDiff line numberDiff line change
@@ -984,250 +984,6 @@ var _ = Describe("Eventhandler", func() {
984984
})
985985
}
986986
})
987-
988-
Describe("WithLowPriorityWhenUnchanged", func() {
989-
It("should lower the priority of a create request for an object that was created more than one minute in the past", func() {
990-
actualOpts := priorityqueue.AddOpts{}
991-
var actualRequests []reconcile.Request
992-
wq := &fakePriorityQueue{
993-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
994-
actualOpts = o
995-
actualRequests = items
996-
},
997-
}
998-
999-
h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{})
1000-
h.Create(ctx, event.CreateEvent{
1001-
Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1002-
Name: "my-pod",
1003-
}},
1004-
}, wq)
1005-
1006-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority}))
1007-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1008-
})
1009-
1010-
It("should lower the priority of a create request for an object that was created more than one minute in the past without the WithLowPriorityWrapper", func() {
1011-
actualOpts := priorityqueue.AddOpts{}
1012-
var actualRequests []reconcile.Request
1013-
wq := &fakePriorityQueue{
1014-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
1015-
actualOpts = o
1016-
actualRequests = items
1017-
},
1018-
}
1019-
1020-
h := &handler.EnqueueRequestForObject{}
1021-
h.Create(ctx, event.CreateEvent{
1022-
Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1023-
Name: "my-pod",
1024-
}},
1025-
}, wq)
1026-
1027-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority}))
1028-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1029-
})
1030-
1031-
It("should not lower the priority of a create request for an object that was created less than one minute in the past", func() {
1032-
actualOpts := priorityqueue.AddOpts{}
1033-
var actualRequests []reconcile.Request
1034-
wq := &fakePriorityQueue{
1035-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
1036-
actualOpts = o
1037-
actualRequests = items
1038-
},
1039-
}
1040-
1041-
h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{})
1042-
h.Create(ctx, event.CreateEvent{
1043-
Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1044-
Name: "my-pod",
1045-
CreationTimestamp: metav1.Now(),
1046-
}},
1047-
}, wq)
1048-
1049-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{}))
1050-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1051-
})
1052-
1053-
It("should not lower the priority of a create request for an object that was created less than one minute in the past without the WithLowPriority wrapperr", func() {
1054-
actualOpts := priorityqueue.AddOpts{}
1055-
var actualRequests []reconcile.Request
1056-
wq := &fakePriorityQueue{
1057-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
1058-
actualOpts = o
1059-
actualRequests = items
1060-
},
1061-
}
1062-
1063-
h := &handler.EnqueueRequestForObject{}
1064-
h.Create(ctx, event.CreateEvent{
1065-
Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1066-
Name: "my-pod",
1067-
CreationTimestamp: metav1.Now(),
1068-
}},
1069-
}, wq)
1070-
1071-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{}))
1072-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1073-
})
1074-
1075-
It("should lower the priority of an update request with unchanged RV", func() {
1076-
actualOpts := priorityqueue.AddOpts{}
1077-
var actualRequests []reconcile.Request
1078-
wq := &fakePriorityQueue{
1079-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
1080-
actualOpts = o
1081-
actualRequests = items
1082-
},
1083-
}
1084-
1085-
h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{})
1086-
h.Update(ctx, event.UpdateEvent{
1087-
ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1088-
Name: "my-pod",
1089-
}},
1090-
ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1091-
Name: "my-pod",
1092-
}},
1093-
}, wq)
1094-
1095-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority}))
1096-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1097-
})
1098-
1099-
It("should lower the priority of an update request with unchanged RV without the WithLowPriority wrapper", func() {
1100-
actualOpts := priorityqueue.AddOpts{}
1101-
var actualRequests []reconcile.Request
1102-
wq := &fakePriorityQueue{
1103-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
1104-
actualOpts = o
1105-
actualRequests = items
1106-
},
1107-
}
1108-
1109-
h := &handler.EnqueueRequestForObject{}
1110-
h.Update(ctx, event.UpdateEvent{
1111-
ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1112-
Name: "my-pod",
1113-
}},
1114-
ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1115-
Name: "my-pod",
1116-
}},
1117-
}, wq)
1118-
1119-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{Priority: handler.LowPriority}))
1120-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1121-
})
1122-
1123-
It("should not lower the priority of an update request with changed RV", func() {
1124-
actualOpts := priorityqueue.AddOpts{}
1125-
var actualRequests []reconcile.Request
1126-
wq := &fakePriorityQueue{
1127-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
1128-
actualOpts = o
1129-
actualRequests = items
1130-
},
1131-
}
1132-
1133-
h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{})
1134-
h.Update(ctx, event.UpdateEvent{
1135-
ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1136-
Name: "my-pod",
1137-
}},
1138-
ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1139-
Name: "my-pod",
1140-
ResourceVersion: "1",
1141-
}},
1142-
}, wq)
1143-
1144-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{}))
1145-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1146-
})
1147-
1148-
It("should not lower the priority of an update request with changed RV without the WithLowPriority wrapper", func() {
1149-
actualOpts := priorityqueue.AddOpts{}
1150-
var actualRequests []reconcile.Request
1151-
wq := &fakePriorityQueue{
1152-
addWithOpts: func(o priorityqueue.AddOpts, items ...reconcile.Request) {
1153-
actualOpts = o
1154-
actualRequests = items
1155-
},
1156-
}
1157-
1158-
h := &handler.EnqueueRequestForObject{}
1159-
h.Update(ctx, event.UpdateEvent{
1160-
ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1161-
Name: "my-pod",
1162-
}},
1163-
ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1164-
Name: "my-pod",
1165-
ResourceVersion: "1",
1166-
}},
1167-
}, wq)
1168-
1169-
Expect(actualOpts).To(Equal(priorityqueue.AddOpts{}))
1170-
Expect(actualRequests).To(Equal([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-pod"}}}))
1171-
})
1172-
1173-
It("should have no effect on create if the workqueue is not a priorityqueue", func() {
1174-
h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{})
1175-
h.Create(ctx, event.CreateEvent{
1176-
Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1177-
Name: "my-pod",
1178-
}},
1179-
}, q)
1180-
1181-
Expect(q.Len()).To(Equal(1))
1182-
item, _ := q.Get()
1183-
Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}}))
1184-
})
1185-
1186-
It("should have no effect on create if the workqueue is not a priorityqueue without the WithLowPriority wrapper", func() {
1187-
h := &handler.EnqueueRequestForObject{}
1188-
h.Create(ctx, event.CreateEvent{
1189-
Object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1190-
Name: "my-pod",
1191-
}},
1192-
}, q)
1193-
1194-
Expect(q.Len()).To(Equal(1))
1195-
item, _ := q.Get()
1196-
Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}}))
1197-
})
1198-
1199-
It("should have no effect on Update if the workqueue is not a priorityqueue", func() {
1200-
h := handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{})
1201-
h.Update(ctx, event.UpdateEvent{
1202-
ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1203-
Name: "my-pod",
1204-
}},
1205-
ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1206-
Name: "my-pod",
1207-
}},
1208-
}, q)
1209-
1210-
Expect(q.Len()).To(Equal(1))
1211-
item, _ := q.Get()
1212-
Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}}))
1213-
})
1214-
1215-
It("should have no effect on Update if the workqueue is not a priorityqueue without the WithLowPriority wrapper", func() {
1216-
h := &handler.EnqueueRequestForObject{}
1217-
h.Update(ctx, event.UpdateEvent{
1218-
ObjectOld: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1219-
Name: "my-pod",
1220-
}},
1221-
ObjectNew: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
1222-
Name: "my-pod",
1223-
}},
1224-
}, q)
1225-
1226-
Expect(q.Len()).To(Equal(1))
1227-
item, _ := q.Get()
1228-
Expect(item).To(Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-pod"}}))
1229-
})
1230-
})
1231987
})
1232988

1233989
type fakePriorityQueue struct {

0 commit comments

Comments
 (0)