Skip to content

Commit 29c322f

Browse files
committed
Refactor DesignateWorker controller to use generic helper functions
- Extract findWorkerListObjects to return ObjectMeta instead of full objects - Create generic createRequestsFromObjectUpdates helper function - Add requestsForObjectUpdates method for cleaner watch setup - Refactor existing findObjectsForSrc to use new helper - Add comprehensive godoc comments to all new functions - Add TODO/NOTE comments for future validation and enhancement work This reduces code duplication and improves memory efficiency by only keeping ObjectMeta for reconcile requests. The pattern can be extended to other controllers in the operator.
1 parent b669ba7 commit 29c322f

File tree

3 files changed

+78
-14
lines changed

3 files changed

+78
-14
lines changed

api/v1beta1/designate_webhook.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ func (r *Designate) ValidateCreate() (admission.Warnings, error) {
128128
allErrs = append(allErrs, err...)
129129
}
130130

131+
// NOTE(beagles): validator here and for updates about
132+
// - nsRecords ending with period
133+
// - the same not mentioned multiple times for different priorities.
134+
// - check expected names in service overrides (maybe just check camel case)
135+
//
131136
if len(allErrs) != 0 {
132137
return nil, apierrors.NewInvalid(
133138
schema.GroupKind{Group: "designate.openstack.org", Kind: "Designate"},

controllers/designateunbound_controller.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,7 @@ func stubZoneDefaults(values map[string]string) map[string]string {
553553
// return the expected defaults defined in the build.
554554
// TODO(beagles): it might be a good idea to have this settable through an environment variable to bridge
555555
// unexpected compatibility issues.
556+
// NOTE(beagles): we should also consider adding the cidr for the cluster network.
556557
func (r *UnboundReconciler) getClusterJoinSubnets(ctx context.Context) []string {
557558
Log := r.GetLogger(ctx)
558559

controllers/designateworker_controller.go

Lines changed: 72 additions & 14 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 -
@@ -338,7 +339,7 @@ func (r *DesignateWorkerReconciler) SetupWithManager(ctx context.Context, mgr ct
338339
handler.EnqueueRequestsFromMapFunc(svcSecretFn)).
339340
Watches(
340341
&corev1.Secret{},
341-
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
342+
handler.EnqueueRequestsFromMapFunc(r.requestsForObjectUpdates),
342343
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
343344
).
344345
// watch the config CMs we don't own
@@ -350,6 +351,71 @@ func (r *DesignateWorkerReconciler) SetupWithManager(ctx context.Context, mgr ct
350351
Complete(r)
351352
}
352353

354+
// findObjectFunc is a function type for listing objects that match a given field selector.
355+
// It returns a slice of ObjectMeta for the matching objects, allowing callers to create
356+
// reconcile requests without needing full object details.
357+
type findObjectFunc func(ctx context.Context, cl client.Client, field string, src client.Object, log *logr.Logger) ([]metav1.ObjectMeta, error)
358+
359+
// findWorkerListObjects finds all DesignateWorker objects that reference the given source object
360+
// through a specific field. It returns only the ObjectMeta for matched objects to minimize memory usage.
361+
func findWorkerListObjects(ctx context.Context, cl client.Client, field string, src client.Object, log *logr.Logger) ([]metav1.ObjectMeta, error) {
362+
crList := &designatev1beta1.DesignateWorkerList{}
363+
err := cl.List(ctx, crList, &client.ListOptions{
364+
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
365+
Namespace: src.GetNamespace(),
366+
})
367+
if err != nil {
368+
log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
369+
return []metav1.ObjectMeta{}, err
370+
}
371+
items := make([]metav1.ObjectMeta, len(crList.Items))
372+
for i, item := range crList.Items {
373+
items[i] = item.ObjectMeta
374+
}
375+
return items, nil
376+
}
377+
378+
// createRequestsFromObjectUpdates is a generic helper function that creates reconcile requests
379+
// for all objects that reference a given source object. It iterates through configured watch fields
380+
// and uses the provided findObjectFunc to locate dependent objects.
381+
// This function encapsulates the boilerplate pattern used across multiple controllers to handle
382+
// changes in watched resources (secrets, configmaps, topology, etc.).
383+
func createRequestsFromObjectUpdates(ctx context.Context, cl client.Client, src client.Object, fn findObjectFunc, log *logr.Logger) []reconcile.Request {
384+
allWatchFields := []string{
385+
passwordSecretField,
386+
caBundleSecretNameField,
387+
topologyField,
388+
}
389+
requests := []reconcile.Request{}
390+
for _, field := range allWatchFields {
391+
objs, err := fn(ctx, cl, field, src, log)
392+
if err != nil {
393+
return requests
394+
}
395+
for _, obj := range objs {
396+
log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), obj.GetName(), obj.GetNamespace()))
397+
requests = append(requests,
398+
reconcile.Request{
399+
NamespacedName: types.NamespacedName{
400+
Name: obj.GetName(),
401+
Namespace: obj.GetNamespace(),
402+
},
403+
},
404+
)
405+
}
406+
}
407+
408+
return requests
409+
}
410+
411+
// requestsForObjectUpdates creates reconcile requests for DesignateWorker objects that depend on
412+
// the given source object (e.g., a Secret or Topology). This method is used as a handler function
413+
// in the controller's watch setup.
414+
func (r *DesignateWorkerReconciler) requestsForObjectUpdates(ctx context.Context, src client.Object) []reconcile.Request {
415+
Log := r.GetLogger(ctx)
416+
return createRequestsFromObjectUpdates(ctx, r.Client, src, findWorkerListObjects, &Log)
417+
}
418+
353419
func (r *DesignateWorkerReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
354420
requests := []reconcile.Request{}
355421

@@ -362,25 +428,17 @@ func (r *DesignateWorkerReconciler) findObjectsForSrc(ctx context.Context, src c
362428
}
363429

364430
for _, field := range allWatchFields {
365-
crList := &designatev1beta1.DesignateWorkerList{}
366-
listOps := &client.ListOptions{
367-
FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
368-
Namespace: src.GetNamespace(),
369-
}
370-
err := r.List(context.TODO(), crList, listOps)
431+
objs, err := findWorkerListObjects(ctx, r.Client, field, src, &Log)
371432
if err != nil {
372-
Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
373433
return requests
374434
}
375-
376-
for _, item := range crList.Items {
377-
Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace()))
378-
435+
for _, obj := range objs {
436+
Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), obj.GetName(), obj.GetNamespace()))
379437
requests = append(requests,
380438
reconcile.Request{
381439
NamespacedName: types.NamespacedName{
382-
Name: item.GetName(),
383-
Namespace: item.GetNamespace(),
440+
Name: obj.GetName(),
441+
Namespace: obj.GetNamespace(),
384442
},
385443
},
386444
)

0 commit comments

Comments
 (0)