diff --git a/reconcilers/child.go b/reconcilers/child.go index 2296041..3cab787 100644 --- a/reconcilers/child.go +++ b/reconcilers/child.go @@ -121,13 +121,19 @@ type ChildReconciler[Type, ChildType client.Object, ChildListType client.ObjectL MergeBeforeUpdate func(current, desired ChildType) // ListOptions allows custom options to be use when listing potential child resources. Each - // resource retrieved as part of the listing is confirmed via OurChild. + // resource retrieved as part of the listing is confirmed via OurChild. There is a performance + // benefit to limiting the number of resource return for each List operation, however, + // excluding an actual child will orphan that resource. // // Defaults to filtering by the reconciled resource's namespace: // []client.ListOption{ // client.InNamespace(resource.GetNamespace()), // } // + // ListOptions is required when a Finalizer is defined or SkipOwnerReference is true. An empty + // list is often sufficient although it may incur a performance penalty, especially when + // querying the API sever instead of an informer cache. + // // +optional ListOptions func(ctx context.Context, resource Type) []client.ListOption @@ -229,6 +235,11 @@ func (r *ChildReconciler[T, CT, CLT]) validate(ctx context.Context) error { return fmt.Errorf("ChildReconciler %q must implement OurChild since owner references are not used", r.Name) } + if r.ListOptions == nil && r.SkipOwnerReference { + // ListOptions is required when SkipOwnerReference is true + return fmt.Errorf("ChildReconciler %q must implement ListOptions since owner references are not used", r.Name) + } + // require MergeBeforeUpdate if r.MergeBeforeUpdate == nil { return fmt.Errorf("ChildReconciler %q must implement MergeBeforeUpdate", r.Name) diff --git a/reconcilers/childset.go b/reconcilers/childset.go index 2020572..b659bf3 100644 --- a/reconcilers/childset.go +++ b/reconcilers/childset.go @@ -116,13 +116,19 @@ type ChildSetReconciler[Type, ChildType client.Object, ChildListType client.Obje MergeBeforeUpdate func(current, desired ChildType) // ListOptions allows custom options to be use when listing potential child resources. Each - // resource retrieved as part of the listing is confirmed via OurChild. + // resource retrieved as part of the listing is confirmed via OurChild. There is a performance + // benefit to limiting the number of resource return for each List operation, however, + // excluding an actual child will orphan that resource. // // Defaults to filtering by the reconciled resource's namespace: // []client.ListOption{ // client.InNamespace(resource.GetNamespace()), // } // + // ListOptions is required when a Finalizer is defined or SkipOwnerReference is true. An empty + // list is often sufficient although it may incur a performance penalty, especially when + // querying the API sever instead of an informer cache. + // // +optional ListOptions func(ctx context.Context, resource Type) []client.ListOption @@ -257,6 +263,11 @@ func (r *ChildSetReconciler[T, CT, CLT]) validate(ctx context.Context) error { return fmt.Errorf("ChildSetReconciler %q must implement OurChild since owner references are not used", r.Name) } + if r.ListOptions == nil && r.SkipOwnerReference { + // ListOptions is required when SkipOwnerReference is true + return fmt.Errorf("ChildSetReconciler %q must implement ListOptions since owner references are not used", r.Name) + } + // require IdentifyChild if r.IdentifyChild == nil { return fmt.Errorf("ChildSetReconciler %q must implement IdentifyChild", r.Name) diff --git a/reconcilers/validate_test.go b/reconcilers/validate_test.go index ac8b4ef..f3ebd5c 100644 --- a/reconcilers/validate_test.go +++ b/reconcilers/validate_test.go @@ -463,6 +463,22 @@ func TestChildReconciler_validate(t *testing.T) { ListOptions: func(ctx context.Context, parent *corev1.ConfigMap) []client.ListOption { return []client.ListOption{} }, }, }, + { + name: "ListOptions missing", + parent: &corev1.ConfigMap{}, + reconciler: &ChildReconciler[*corev1.ConfigMap, *corev1.Pod, *corev1.PodList]{ + Name: "ListOptions missing", + ChildType: &corev1.Pod{}, + ChildListType: &corev1.PodList{}, + DesiredChild: func(ctx context.Context, parent *corev1.ConfigMap) (*corev1.Pod, error) { return nil, nil }, + ReflectChildStatusOnParent: func(ctx context.Context, parent *corev1.ConfigMap, child *corev1.Pod, err error) {}, + MergeBeforeUpdate: func(current, desired *corev1.Pod) {}, + SkipOwnerReference: true, + // ListOptions: func(ctx context.Context, parent *corev1.ConfigMap) []client.ListOption { return []client.ListOption{} }, + OurChild: func(resource *corev1.ConfigMap, child *corev1.Pod) bool { return true }, + }, + shouldErr: `ChildReconciler "ListOptions missing" must implement ListOptions since owner references are not used`, + }, { name: "Finalizer without OurChild", parent: &corev1.ConfigMap{}, @@ -622,6 +638,23 @@ func TestChildSetReconciler_validate(t *testing.T) { IdentifyChild: func(child *corev1.Pod) string { return "" }, }, }, + { + name: "ListOptions missing", + parent: &corev1.ConfigMap{}, + reconciler: &ChildSetReconciler[*corev1.ConfigMap, *corev1.Pod, *corev1.PodList]{ + Name: "ListOptions missing", + ChildType: &corev1.Pod{}, + ChildListType: &corev1.PodList{}, + DesiredChildren: func(ctx context.Context, parent *corev1.ConfigMap) ([]*corev1.Pod, error) { return nil, nil }, + ReflectChildrenStatusOnParent: func(ctx context.Context, parent *corev1.ConfigMap, result ChildSetResult[*corev1.Pod]) {}, + MergeBeforeUpdate: func(current, desired *corev1.Pod) {}, + SkipOwnerReference: true, + // ListOptions: func(ctx context.Context, parent *corev1.ConfigMap) []client.ListOption { return []client.ListOption{} }, + OurChild: func(resource *corev1.ConfigMap, child *corev1.Pod) bool { return true }, + IdentifyChild: func(child *corev1.Pod) string { return "" }, + }, + shouldErr: `ChildSetReconciler "ListOptions missing" must implement ListOptions since owner references are not used`, + }, { name: "Finalizer without OurChild", parent: &corev1.ConfigMap{},