Skip to content

Commit 7d762d6

Browse files
beaglesCursor
andcommitted
Refactor all controllers to use shared watch helper functions
This change introduces a new shared helper pattern for all controllers to reduce code duplication and improve maintainability. A new watch_helpers.go file provides a generic CreateRequestsFromObjectUpdates function that encapsulates the common pattern used across all seven designate controllers when handling watched resource updates. Each controller now implements a lightweight findXXXListObjects function specific to its type and a requestsForObjectUpdates method that leverages the shared helper. This approach returns only ObjectMeta instead of full objects, improving memory efficiency. The old findObjectsForSrc methods have been completely removed across all controllers, eliminating 222 lines of duplicated code. All watch handlers now consistently use the new requestsForObjectUpdates method, providing a uniform pattern that's easier to understand, maintain, and extend. The refactoring covers DesignateAPI (with TLS field support), DesignateCentral, DesignateProducer, DesignateMdns, DesignateBackendbind9, DesignateUnbound, and DesignateWorker controllers. This is purely a refactoring change with no functional differences. All controllers pass fmt, vet, and linter checks. The net result is 38 fewer lines of code with significantly improved consistency and maintainability that can serve as a template for other operators in the openstack-k8s-operators ecosystem. Co-authored-by: Cursor <[email protected]>
1 parent 29c322f commit 7d762d6

8 files changed

+224
-262
lines changed

controllers/designateapi_controller.go

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import (
5959
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
6060
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
6161
mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
62+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
6263
)
6364

6465
// GetClient -
@@ -382,60 +383,52 @@ func (r *DesignateAPIReconciler) SetupWithManager(ctx context.Context, mgr ctrl.
382383
handler.EnqueueRequestsFromMapFunc(svcSecretFn)).
383384
Watches(
384385
&corev1.Secret{},
385-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
386+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
386387
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
387388
).
388389
Watches(&networkv1.NetworkAttachmentDefinition{},
389390
handler.EnqueueRequestsFromMapFunc(nadFn)).
390391
Watches(&topologyv1.Topology{},
391-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
392+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
392393
builder.WithPredicates(predicate.GenerationChangedPredicate{})).
393394
Watches(&keystonev1.KeystoneAPI{},
394395
handler.EnqueueRequestsFromMapFunc(r.findObjectForSrc),
395396
builder.WithPredicates(keystonev1.KeystoneAPIStatusChangedPredicate)).
396397
Complete(r)
397398
}
398399

399-
func (r *DesignateAPIReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
400-
requests := []reconcile.Request{}
400+
// findAPIListObjects finds all DesignateAPI objects that reference the given source object
401+
// through a specific field. It returns only the ObjectMeta for matched objects to minimize memory usage.
402+
func findAPIListObjects(ctx context.Context, cl client.Client, field string, src client.Object, log *logr.Logger) ([]metav1.ObjectMeta, error) {
403+
crList := &designatev1beta1.DesignateAPIList{}
404+
err := cl.List(ctx, crList, &client.ListOptions{
405+
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
406+
Namespace: src.GetNamespace(),
407+
})
408+
if err != nil {
409+
log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
410+
return []metav1.ObjectMeta{}, err
411+
}
412+
items := make([]metav1.ObjectMeta, len(crList.Items))
413+
for i, item := range crList.Items {
414+
items[i] = item.ObjectMeta
415+
}
416+
return items, nil
417+
}
401418

419+
// requestsForObjectUpdates creates reconcile requests for DesignateAPI objects that depend on
420+
// the given source object (e.g., a Secret or Topology). This method is used as a handler function
421+
// in the controller's watch setup.
422+
func (r *DesignateAPIReconciler) requestsForObjectUpdates(ctx context.Context, src client.Object) []reconcile.Request {
402423
Log := r.GetLogger(ctx)
403-
404424
allWatchFields := []string{
405425
passwordSecretField,
406426
caBundleSecretNameField,
407427
tlsAPIInternalField,
408428
tlsAPIPublicField,
409429
topologyField,
410430
}
411-
412-
for _, field := range allWatchFields {
413-
crList := &designatev1beta1.DesignateAPIList{}
414-
listOps := &client.ListOptions{
415-
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
416-
Namespace: src.GetNamespace(),
417-
}
418-
err := r.List(context.TODO(), crList, listOps)
419-
if err != nil {
420-
Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
421-
return requests
422-
}
423-
424-
for _, item := range crList.Items {
425-
Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace()))
426-
427-
requests = append(requests,
428-
reconcile.Request{
429-
NamespacedName: types.NamespacedName{
430-
Name: item.GetName(),
431-
Namespace: item.GetNamespace(),
432-
},
433-
},
434-
)
435-
}
436-
}
437-
438-
return requests
431+
return CreateRequestsFromObjectUpdates(ctx, r.Client, src, allWatchFields, findAPIListObjects, &Log)
439432
}
440433

441434
func (r *DesignateAPIReconciler) findObjectForSrc(ctx context.Context, src client.Object) []reconcile.Request {

controllers/designatebackendbind9_controller.go

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import (
5252
"github.com/openstack-k8s-operators/lib-common/modules/common/secret"
5353
"github.com/openstack-k8s-operators/lib-common/modules/common/statefulset"
5454
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
55+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5556
)
5657

5758
// GetClient -
@@ -293,46 +294,39 @@ func (r *DesignateBackendbind9Reconciler) SetupWithManager(ctx context.Context,
293294
Watches(&corev1.ConfigMap{},
294295
handler.EnqueueRequestsFromMapFunc(configMapFn)).
295296
Watches(&topologyv1.Topology{},
296-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
297+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
297298
builder.WithPredicates(predicate.GenerationChangedPredicate{})).
298299
Complete(r)
299300
}
300301

301-
func (r *DesignateBackendbind9Reconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
302-
requests := []reconcile.Request{}
302+
// findBackendbind9ListObjects finds all DesignateBackendbind9 objects that reference the given source object
303+
// through a specific field. It returns only the ObjectMeta for matched objects to minimize memory usage.
304+
func findBackendbind9ListObjects(ctx context.Context, cl client.Client, field string, src client.Object, log *logr.Logger) ([]metav1.ObjectMeta, error) {
305+
crList := &designatev1beta1.DesignateBackendbind9List{}
306+
err := cl.List(ctx, crList, &client.ListOptions{
307+
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
308+
Namespace: src.GetNamespace(),
309+
})
310+
if err != nil {
311+
log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
312+
return []metav1.ObjectMeta{}, err
313+
}
314+
items := make([]metav1.ObjectMeta, len(crList.Items))
315+
for i, item := range crList.Items {
316+
items[i] = item.ObjectMeta
317+
}
318+
return items, nil
319+
}
303320

321+
// requestsForObjectUpdates creates reconcile requests for DesignateBackendbind9 objects that depend on
322+
// the given source object (e.g., a Topology). This method is used as a handler function
323+
// in the controller's watch setup.
324+
func (r *DesignateBackendbind9Reconciler) requestsForObjectUpdates(ctx context.Context, src client.Object) []reconcile.Request {
304325
Log := r.GetLogger(ctx)
305-
306326
allWatchFields := []string{
307327
topologyField,
308328
}
309-
310-
for _, field := range allWatchFields {
311-
crList := &designatev1beta1.DesignateBackendbind9List{}
312-
listOps := &client.ListOptions{
313-
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
314-
Namespace: src.GetNamespace(),
315-
}
316-
err := r.List(context.TODO(), crList, listOps)
317-
if err != nil {
318-
Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
319-
return requests
320-
}
321-
322-
for _, item := range crList.Items {
323-
Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace()))
324-
325-
requests = append(requests,
326-
reconcile.Request{
327-
NamespacedName: types.NamespacedName{
328-
Name: item.GetName(),
329-
Namespace: item.GetNamespace(),
330-
},
331-
},
332-
)
333-
}
334-
}
335-
return requests
329+
return CreateRequestsFromObjectUpdates(ctx, r.Client, src, allWatchFields, findBackendbind9ListObjects, &Log)
336330
}
337331

338332
func (r *DesignateBackendbind9Reconciler) reconcileDelete(ctx context.Context, instance *designatev1beta1.DesignateBackendbind9, helper *helper.Helper) (ctrl.Result, error) {

controllers/designatecentral_controller.go

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import (
5252
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
5353
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
5454
mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
55+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5556
)
5657

5758
// GetClient -
@@ -332,54 +333,46 @@ func (r *DesignateCentralReconciler) SetupWithManager(ctx context.Context, mgr c
332333
handler.EnqueueRequestsFromMapFunc(svcSecretFn)).
333334
Watches(
334335
&corev1.Secret{},
335-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
336+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
336337
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
337338
).
338339
// watch the config CMs we don't own
339340
Watches(&topologyv1.Topology{},
340-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
341+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
341342
builder.WithPredicates(predicate.GenerationChangedPredicate{})).
342343
Complete(r)
343344
}
344345

345-
func (r *DesignateCentralReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
346-
requests := []reconcile.Request{}
346+
// findCentralListObjects finds all DesignateCentral objects that reference the given source object
347+
// through a specific field. It returns only the ObjectMeta for matched objects to minimize memory usage.
348+
func findCentralListObjects(ctx context.Context, cl client.Client, field string, src client.Object, log *logr.Logger) ([]metav1.ObjectMeta, error) {
349+
crList := &designatev1beta1.DesignateCentralList{}
350+
err := cl.List(ctx, crList, &client.ListOptions{
351+
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
352+
Namespace: src.GetNamespace(),
353+
})
354+
if err != nil {
355+
log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
356+
return []metav1.ObjectMeta{}, err
357+
}
358+
items := make([]metav1.ObjectMeta, len(crList.Items))
359+
for i, item := range crList.Items {
360+
items[i] = item.ObjectMeta
361+
}
362+
return items, nil
363+
}
347364

365+
// requestsForObjectUpdates creates reconcile requests for DesignateCentral objects that depend on
366+
// the given source object (e.g., a Secret or Topology). This method is used as a handler function
367+
// in the controller's watch setup.
368+
func (r *DesignateCentralReconciler) requestsForObjectUpdates(ctx context.Context, src client.Object) []reconcile.Request {
348369
Log := r.GetLogger(ctx)
349-
350370
allWatchFields := []string{
351371
passwordSecretField,
352372
caBundleSecretNameField,
353373
topologyField,
354374
}
355-
356-
for _, field := range allWatchFields {
357-
crList := &designatev1beta1.DesignateCentralList{}
358-
listOps := &client.ListOptions{
359-
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
360-
Namespace: src.GetNamespace(),
361-
}
362-
err := r.List(context.TODO(), crList, listOps)
363-
if err != nil {
364-
Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
365-
return requests
366-
}
367-
368-
for _, item := range crList.Items {
369-
Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace()))
370-
371-
requests = append(requests,
372-
reconcile.Request{
373-
NamespacedName: types.NamespacedName{
374-
Name: item.GetName(),
375-
Namespace: item.GetNamespace(),
376-
},
377-
},
378-
)
379-
}
380-
}
381-
382-
return requests
375+
return CreateRequestsFromObjectUpdates(ctx, r.Client, src, allWatchFields, findCentralListObjects, &Log)
383376
}
384377

385378
func (r *DesignateCentralReconciler) reconcileDelete(ctx context.Context, instance *designatev1beta1.DesignateCentral, helper *helper.Helper) (ctrl.Result, error) {

controllers/designatemdns_controller.go

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import (
5454
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
5555
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
5656
mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
57+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5758
)
5859

5960
// GetClient -
@@ -337,56 +338,48 @@ func (r *DesignateMdnsReconciler) SetupWithManager(ctx context.Context, mgr ctrl
337338
handler.EnqueueRequestsFromMapFunc(svcSecretFn)).
338339
Watches(
339340
&corev1.Secret{},
340-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
341+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
341342
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
342343
).
343344
// watch the config CMs we don't own
344345
Watches(&corev1.ConfigMap{},
345346
handler.EnqueueRequestsFromMapFunc(configMapFn)).
346347
Watches(&topologyv1.Topology{},
347-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
348+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
348349
builder.WithPredicates(predicate.GenerationChangedPredicate{})).
349350
Complete(r)
350351
}
351352

352-
func (r *DesignateMdnsReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
353-
requests := []reconcile.Request{}
353+
// findMdnsListObjects finds all DesignateMdns objects that reference the given source object
354+
// through a specific field. It returns only the ObjectMeta for matched objects to minimize memory usage.
355+
func findMdnsListObjects(ctx context.Context, cl client.Client, field string, src client.Object, log *logr.Logger) ([]metav1.ObjectMeta, error) {
356+
crList := &designatev1beta1.DesignateMdnsList{}
357+
err := cl.List(ctx, crList, &client.ListOptions{
358+
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
359+
Namespace: src.GetNamespace(),
360+
})
361+
if err != nil {
362+
log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
363+
return []metav1.ObjectMeta{}, err
364+
}
365+
items := make([]metav1.ObjectMeta, len(crList.Items))
366+
for i, item := range crList.Items {
367+
items[i] = item.ObjectMeta
368+
}
369+
return items, nil
370+
}
354371

372+
// requestsForObjectUpdates creates reconcile requests for DesignateMdns objects that depend on
373+
// the given source object (e.g., a Secret or Topology). This method is used as a handler function
374+
// in the controller's watch setup.
375+
func (r *DesignateMdnsReconciler) requestsForObjectUpdates(ctx context.Context, src client.Object) []reconcile.Request {
355376
Log := r.GetLogger(ctx)
356-
357377
allWatchFields := []string{
358378
passwordSecretField,
359379
caBundleSecretNameField,
360380
topologyField,
361381
}
362-
363-
for _, field := range allWatchFields {
364-
crList := &designatev1beta1.DesignateMdnsList{}
365-
listOps := &client.ListOptions{
366-
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
367-
Namespace: src.GetNamespace(),
368-
}
369-
err := r.List(context.TODO(), crList, listOps)
370-
if err != nil {
371-
Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
372-
return requests
373-
}
374-
375-
for _, item := range crList.Items {
376-
Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace()))
377-
378-
requests = append(requests,
379-
reconcile.Request{
380-
NamespacedName: types.NamespacedName{
381-
Name: item.GetName(),
382-
Namespace: item.GetNamespace(),
383-
},
384-
},
385-
)
386-
}
387-
}
388-
389-
return requests
382+
return CreateRequestsFromObjectUpdates(ctx, r.Client, src, allWatchFields, findMdnsListObjects, &Log)
390383
}
391384

392385
func (r *DesignateMdnsReconciler) reconcileDelete(ctx context.Context, instance *designatev1beta1.DesignateMdns, helper *helper.Helper) (ctrl.Result, error) {

0 commit comments

Comments
 (0)