Skip to content

Commit 779eb71

Browse files
josvazgjosvazg
committed
CLOUDP-340286: Reconciler on skip removal
Signed-off-by: jose.vazquez <[email protected]>
1 parent 302422c commit 779eb71

File tree

3 files changed

+284
-7
lines changed

3 files changed

+284
-7
lines changed

‎internal/controller/customresource/customresource.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/Masterminds/semver"
2222
"go.uber.org/zap"
2323
apierrors "k8s.io/apimachinery/pkg/api/errors"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2425
"sigs.k8s.io/controller-runtime/pkg/client"
2526
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2627

@@ -136,7 +137,7 @@ func ResourceVersionIsValid(resource akov2.AtlasCustomResource) (bool, error) {
136137
}
137138

138139
// ReconciliationShouldBeSkipped returns 'true' if reconciliation should be skipped for this resource.
139-
func ReconciliationShouldBeSkipped(resource akov2.AtlasCustomResource) bool {
140+
func ReconciliationShouldBeSkipped(resource metav1.Object) bool {
140141
if v, ok := resource.GetAnnotations()[ReconciliationPolicyAnnotation]; ok {
141142
return v == ReconciliationPolicySkip
142143
}

‎internal/controller/watch/predicates.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"sigs.k8s.io/controller-runtime/pkg/client"
2222
"sigs.k8s.io/controller-runtime/pkg/event"
2323
"sigs.k8s.io/controller-runtime/pkg/predicate"
24+
25+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/customresource"
2426
)
2527

2628
// DeprecatedCommonPredicates returns the predicate which filter out the changes done to any field except for spec (e.g. status)
@@ -34,6 +36,11 @@ func DeprecatedCommonPredicates() predicate.Funcs {
3436
return true
3537
}
3638

39+
if customresource.ReconciliationShouldBeSkipped(e.ObjectOld) &&
40+
!customresource.ReconciliationShouldBeSkipped(e.ObjectNew) {
41+
return true
42+
}
43+
3744
if e.ObjectOld.GetGeneration() == e.ObjectNew.GetGeneration() && reflect.DeepEqual(e.ObjectNew.GetFinalizers(), e.ObjectOld.GetFinalizers()) {
3845
return false
3946
}
@@ -58,11 +65,25 @@ func SelectNamespacesPredicate(namespaces []string) predicate.Funcs {
5865
})
5966
}
6067

68+
// ReconcileAgainPredicate reconciles on updates when the skip reconcile
69+
// annotation is removed or changed
70+
func ReconcileAgainPredicate[T metav1.Object]() predicate.TypedPredicate[T] {
71+
return predicate.TypedFuncs[T]{
72+
UpdateFunc: func(e event.TypedUpdateEvent[T]) bool {
73+
if customresource.ReconciliationShouldBeSkipped(e.ObjectOld) &&
74+
!customresource.ReconciliationShouldBeSkipped(e.ObjectNew) {
75+
return true
76+
}
77+
return false
78+
},
79+
}
80+
}
81+
6182
// GlobalResyncAwareGenerationChangePredicate reconcile on unfrequent global
6283
// resyncs or on spec generation changes, but ignore finalizer changes
6384
func GlobalResyncAwareGenerationChangePredicate[T metav1.Object]() predicate.TypedPredicate[T] {
64-
return predicate.Or[T](
65-
predicate.Not[T](predicate.TypedResourceVersionChangedPredicate[T]{}), // for the global resync
85+
return predicate.Or(
86+
predicate.Not(predicate.TypedResourceVersionChangedPredicate[T]{}), // for the global resync
6687
predicate.TypedGenerationChangedPredicate[T]{},
6788
)
6889
}
@@ -79,8 +100,11 @@ func IgnoreDeletedPredicate[T metav1.Object]() predicate.TypedPredicate[T] {
79100

80101
// DefaultPredicates avoid spurious after deletion or finalizer changes handling
81102
func DefaultPredicates[T metav1.Object]() predicate.TypedPredicate[T] {
82-
return predicate.And[T](
83-
GlobalResyncAwareGenerationChangePredicate[T](),
103+
return predicate.And( // or the generation changed or timed out (except for deletions)
104+
predicate.Or(
105+
GlobalResyncAwareGenerationChangePredicate[T](),
106+
ReconcileAgainPredicate[T](),
107+
),
84108
IgnoreDeletedPredicate[T](),
85109
)
86110
}

‎internal/controller/watch/predicates_test.go‎

Lines changed: 254 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,20 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package watch
15+
package watch_test
1616

1717
import (
1818
"testing"
1919

2020
"github.com/stretchr/testify/assert"
21+
corev1 "k8s.io/api/core/v1"
2122
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2223
"sigs.k8s.io/controller-runtime/pkg/event"
2324

25+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api"
2426
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
27+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/customresource"
28+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/watch"
2529
)
2630

2731
func TestSelectNamespacesPredicate(t *testing.T) {
@@ -61,11 +65,259 @@ func TestSelectNamespacesPredicate(t *testing.T) {
6165

6266
for name, tt := range tests {
6367
t.Run(name, func(t *testing.T) {
64-
f := SelectNamespacesPredicate(tt.namespaces)
68+
f := watch.SelectNamespacesPredicate(tt.namespaces)
6569
assert.Equal(t, tt.expect, f.CreateFunc(tt.createEvent))
6670
assert.Equal(t, tt.expect, f.UpdateFunc(tt.updateEvent))
6771
assert.Equal(t, tt.expect, f.DeleteFunc(tt.deleteEvent))
6872
assert.Equal(t, tt.expect, f.GenericFunc(tt.genericEvent))
6973
})
7074
}
7175
}
76+
77+
func TestDeprecatedCommonPredicates(t *testing.T) {
78+
for _, tc := range []struct {
79+
title string
80+
old *akov2.AtlasProject
81+
new *akov2.AtlasProject
82+
want bool
83+
}{
84+
{
85+
title: "no changes - resync",
86+
old: sampleObj(),
87+
new: sampleObj(),
88+
want: true,
89+
},
90+
{
91+
title: "no gen change",
92+
old: sampleObj(resourceVersion("0")),
93+
new: sampleObj(resourceVersion("1")),
94+
want: false,
95+
},
96+
{
97+
title: "finalizers changed",
98+
old: sampleObj(resourceVersion("0")),
99+
new: sampleObj(resourceVersion("1"), finalizers([]string{"finalize"})),
100+
want: true,
101+
},
102+
{
103+
title: "skipped",
104+
old: sampleObj(resourceVersion("0")),
105+
new: sampleObj(resourceVersion("1"), skipAnnotation()),
106+
want: false,
107+
},
108+
{
109+
title: "no longer skipped",
110+
old: sampleObj(resourceVersion("0"), skipAnnotation()),
111+
new: sampleObj(
112+
resourceVersion("1"),
113+
),
114+
want: true,
115+
},
116+
} {
117+
t.Run(tc.title, func(t *testing.T) {
118+
f := watch.DeprecatedCommonPredicates()
119+
assert.Equal(t, tc.want, f.UpdateFunc(
120+
event.UpdateEvent{ObjectOld: tc.old, ObjectNew: tc.new}))
121+
})
122+
}
123+
}
124+
125+
func TestDefaultPredicates(t *testing.T) {
126+
for _, tc := range []struct {
127+
title string
128+
old *akov2.AtlasProject
129+
new *akov2.AtlasProject
130+
wantCreate bool
131+
wantUpdate bool
132+
wantDelete bool
133+
wantGeneric bool
134+
}{
135+
{
136+
title: "no changes",
137+
old: sampleObj(),
138+
new: sampleObj(),
139+
wantCreate: true,
140+
wantUpdate: true,
141+
wantDelete: false,
142+
wantGeneric: true,
143+
},
144+
{
145+
title: "no gen change",
146+
old: sampleObj(resourceVersion("0")),
147+
new: sampleObj(resourceVersion("1")),
148+
wantCreate: true,
149+
wantUpdate: false,
150+
wantDelete: false,
151+
wantGeneric: true,
152+
},
153+
{
154+
title: "finalizers set",
155+
old: sampleObj(resourceVersion("0")),
156+
new: sampleObj(resourceVersion("1"), finalizers([]string{"finalize"})),
157+
wantCreate: true,
158+
wantUpdate: false, // finalizer changes do not trigger updates unlike with deprecated
159+
wantDelete: false,
160+
wantGeneric: true,
161+
},
162+
{
163+
title: "skipped",
164+
old: sampleObj(
165+
resourceVersion("0"),
166+
),
167+
new: sampleObj(
168+
resourceVersion("1"),
169+
skipAnnotation(),
170+
),
171+
wantCreate: true,
172+
wantUpdate: false,
173+
wantDelete: false,
174+
wantGeneric: true,
175+
},
176+
{
177+
title: "no longer skipped",
178+
old: sampleObj(resourceVersion("0"), skipAnnotation()),
179+
new: sampleObj(resourceVersion("1")),
180+
wantCreate: true,
181+
wantUpdate: true,
182+
wantDelete: false,
183+
wantGeneric: true,
184+
},
185+
{
186+
title: "finalizers removed",
187+
old: sampleObj(resourceVersion("0"), finalizers([]string{"finalize"})),
188+
new: sampleObj(resourceVersion("1")),
189+
wantCreate: true,
190+
wantUpdate: false,
191+
wantDelete: false,
192+
wantGeneric: true,
193+
},
194+
} {
195+
t.Run(tc.title, func(t *testing.T) {
196+
f := watch.DefaultPredicates[*akov2.AtlasProject]()
197+
assert.Equal(t, tc.wantCreate,
198+
f.Create(event.TypedCreateEvent[*akov2.AtlasProject]{Object: tc.new}), "on create")
199+
assert.Equal(t, tc.wantUpdate,
200+
f.Update(event.TypedUpdateEvent[*akov2.AtlasProject]{
201+
ObjectOld: tc.old, ObjectNew: tc.new,
202+
}), "on update")
203+
assert.Equal(t, tc.wantDelete,
204+
f.Delete(event.TypedDeleteEvent[*akov2.AtlasProject]{Object: tc.new}), "on delete")
205+
assert.Equal(t, tc.wantGeneric,
206+
f.Generic(event.TypedGenericEvent[*akov2.AtlasProject]{Object: tc.new}), "on generic event")
207+
})
208+
}
209+
}
210+
211+
func TestReadyTransitionPredicate(t *testing.T) {
212+
for _, tc := range []struct {
213+
title string
214+
old *akov2.AtlasProject
215+
new *akov2.AtlasProject
216+
wantCreate bool
217+
wantUpdate bool
218+
wantDelete bool
219+
wantGeneric bool
220+
}{
221+
{
222+
title: "no changes",
223+
old: sampleObj(),
224+
new: sampleObj(),
225+
wantCreate: false,
226+
wantUpdate: false,
227+
wantDelete: true,
228+
wantGeneric: false,
229+
},
230+
{
231+
title: "ready now",
232+
old: sampleObj(ready(corev1.ConditionFalse)),
233+
new: sampleObj(ready(corev1.ConditionTrue)),
234+
wantCreate: false,
235+
wantUpdate: true,
236+
wantDelete: true,
237+
wantGeneric: false,
238+
},
239+
{
240+
title: "no longer ready",
241+
old: sampleObj(ready(corev1.ConditionTrue)),
242+
new: sampleObj(ready(corev1.ConditionFalse)),
243+
wantCreate: false,
244+
wantUpdate: false,
245+
wantDelete: true,
246+
wantGeneric: false,
247+
},
248+
} {
249+
t.Run(tc.title, func(t *testing.T) {
250+
f := watch.ReadyTransitionPredicate(projectReady)
251+
assert.Equal(t, tc.wantCreate,
252+
f.Create(event.CreateEvent{Object: tc.new}), "on create")
253+
assert.Equal(t, tc.wantUpdate,
254+
f.Update(event.UpdateEvent{ObjectOld: tc.old, ObjectNew: tc.new}), "on update")
255+
assert.Equal(t, tc.wantDelete,
256+
f.Delete(event.DeleteEvent{Object: tc.new}), "on delete")
257+
assert.Equal(t, tc.wantGeneric,
258+
f.Generic(event.GenericEvent{Object: tc.new}), "on generic event")
259+
})
260+
}
261+
}
262+
263+
func projectReady(p *akov2.AtlasProject) bool {
264+
for _, c := range p.Status.Conditions {
265+
if c.Type == api.ReadyType && c.Status == corev1.ConditionTrue {
266+
return true
267+
}
268+
}
269+
return false
270+
}
271+
272+
type optionFunc func(*akov2.AtlasProject) *akov2.AtlasProject
273+
274+
func sampleObj(opts ...optionFunc) *akov2.AtlasProject {
275+
p := &akov2.AtlasProject{
276+
ObjectMeta: metav1.ObjectMeta{
277+
Name: "project",
278+
Namespace: "ns",
279+
Annotations: map[string]string{},
280+
},
281+
Spec: akov2.AtlasProjectSpec{
282+
Name: "atlas-project",
283+
},
284+
}
285+
for _, opt := range opts {
286+
p = opt(p)
287+
}
288+
return p
289+
}
290+
291+
func resourceVersion(rs string) optionFunc {
292+
return func(p *akov2.AtlasProject) *akov2.AtlasProject {
293+
p.ResourceVersion = rs
294+
return p
295+
}
296+
}
297+
298+
func skipAnnotation() optionFunc {
299+
return func(p *akov2.AtlasProject) *akov2.AtlasProject {
300+
p.Annotations[customresource.ReconciliationPolicyAnnotation] =
301+
customresource.ReconciliationPolicySkip
302+
return p
303+
}
304+
}
305+
306+
func finalizers(f []string) optionFunc {
307+
return func(p *akov2.AtlasProject) *akov2.AtlasProject {
308+
p.Finalizers = f
309+
return p
310+
}
311+
}
312+
313+
func ready(status corev1.ConditionStatus) optionFunc {
314+
return func(p *akov2.AtlasProject) *akov2.AtlasProject {
315+
p.Status.Conditions = append(p.Status.Conditions,
316+
api.Condition{
317+
Type: api.ReadyType,
318+
Status: status,
319+
},
320+
)
321+
return p
322+
}
323+
}

0 commit comments

Comments
 (0)