Skip to content

Commit ecd5c19

Browse files
authored
Fix select annotation changed predicate (#4967)
It was mistakenly classifying creates as containing an annotation change. This was previously harmless because we always reconciled the resource (say, ResourceGroup) if the obj was created anyway, but now that we also use this predicate for watching namespaces, we do not want events on namespace create, only on namespace updates that contain an annotation change.
1 parent e43ce71 commit ecd5c19

File tree

3 files changed

+93
-1
lines changed

3 files changed

+93
-1
lines changed

v2/internal/reconcilers/generic/register.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,9 @@ func registerNamespaceWatcher(kubeclient kubeclient.Client, gvk schema.GroupVers
204204
}
205205
// list the objects for the current kind
206206

207-
log.V(Verbose).Info("Detected namespace reconcile-policy annotation", "namespace", obj.GetName())
207+
log.V(Verbose).Info("Detected namespace reconcile-policy annotation change",
208+
"namespace", obj.GetName(),
209+
"reconcile-policy", obj.GetAnnotations()["serviceoperator.azure.com/reconcile-policy"])
208210
for _, el := range aList.Items {
209211
if _, ok := el.GetAnnotations()["serviceoperator.azure.com/reconcile-policy"]; ok {
210212
// If the annotation is defined for the object, there's no need to reconcile it and we skip the object

v2/internal/util/predicates/select_annotations_predicate.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,21 @@ type selectAnnotationChangedPredicate struct {
3636

3737
var _ predicate.Predicate = selectAnnotationChangedPredicate{}
3838

39+
// Create implements CreateEvent filter - we don't care about create events for annotation changes.
40+
func (p selectAnnotationChangedPredicate) Create(e event.CreateEvent) bool {
41+
return false
42+
}
43+
44+
// Delete implements DeleteEvent filter - we don't care about delete events for annotation changes.
45+
func (p selectAnnotationChangedPredicate) Delete(e event.DeleteEvent) bool {
46+
return false
47+
}
48+
49+
// Generic implements GenericEvent filter - we don't care about generic events for annotation changes.
50+
func (p selectAnnotationChangedPredicate) Generic(e event.GenericEvent) bool {
51+
return false
52+
}
53+
3954
// Update implements UpdateEvent filter for annotation changes.
4055
func (p selectAnnotationChangedPredicate) Update(e event.UpdateEvent) bool {
4156
if e.ObjectOld == nil {

v2/internal/util/predicates/select_annotations_predicate_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,81 @@ func Test_SelectAnnotationsChangedPredicate_DetectsChanges(t *testing.T) {
144144
}
145145
}
146146

147+
func Test_SelectAnnotationsChangedPredicate_CreateEvent(t *testing.T) {
148+
t.Parallel()
149+
g := NewGomegaWithT(t)
150+
151+
annotationKey := "foobar"
152+
predicate := predicates.MakeSelectAnnotationChangedPredicate(
153+
map[string]predicates.HasAnnotationChanged{
154+
annotationKey: predicates.HasBasicAnnotationChanged,
155+
})
156+
157+
watchedObj := &SampleObj{
158+
ObjectMeta: metav1.ObjectMeta{
159+
Annotations: map[string]string{
160+
annotationKey: "1234",
161+
},
162+
},
163+
}
164+
165+
result := predicate.Create(
166+
event.CreateEvent{
167+
Object: watchedObj,
168+
})
169+
g.Expect(result).To(BeFalse())
170+
}
171+
172+
func Test_SelectAnnotationsChangedPredicate_DeleteEvent(t *testing.T) {
173+
t.Parallel()
174+
g := NewGomegaWithT(t)
175+
176+
annotationKey := "foobar"
177+
predicate := predicates.MakeSelectAnnotationChangedPredicate(
178+
map[string]predicates.HasAnnotationChanged{
179+
annotationKey: predicates.HasBasicAnnotationChanged,
180+
})
181+
182+
watchedObj := &SampleObj{
183+
ObjectMeta: metav1.ObjectMeta{
184+
Annotations: map[string]string{
185+
annotationKey: "1234",
186+
},
187+
},
188+
}
189+
190+
result := predicate.Delete(
191+
event.DeleteEvent{
192+
Object: watchedObj,
193+
})
194+
g.Expect(result).To(BeFalse())
195+
}
196+
197+
func Test_SelectAnnotationsChangedPredicate_GenericEvent(t *testing.T) {
198+
t.Parallel()
199+
g := NewGomegaWithT(t)
200+
201+
annotationKey := "foobar"
202+
predicate := predicates.MakeSelectAnnotationChangedPredicate(
203+
map[string]predicates.HasAnnotationChanged{
204+
annotationKey: predicates.HasBasicAnnotationChanged,
205+
})
206+
207+
watchedObj := &SampleObj{
208+
ObjectMeta: metav1.ObjectMeta{
209+
Annotations: map[string]string{
210+
annotationKey: "1234",
211+
},
212+
},
213+
}
214+
215+
result := predicate.Generic(
216+
event.GenericEvent{
217+
Object: watchedObj,
218+
})
219+
g.Expect(result).To(BeFalse())
220+
}
221+
147222
func Test_SelectAnnotationsChangedPredicate_MissingAnnotationPassesNilToHandler(t *testing.T) {
148223
t.Parallel()
149224
g := NewGomegaWithT(t)

0 commit comments

Comments
 (0)