From 3302c5b3f4fd6d5240b5c519d614800ab94e1556 Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Fri, 10 Jan 2025 17:33:53 +0100 Subject: [PATCH] Use findObjectsForSrc to watch the referenced topology With the goal of deduplicating the current code as much as possible and help spreading the pattern across the operators, this patch adds the topologyRef dynamic field to the existing glanceAPIWatchList. This allows to both index the field (and use fieldSelector when the GlanceAPI CR is retrieved), and remove the topologyFn/tpFn functions. The watcher is now based on GenerationChangedPredicate, which only triggers a reconcile when .Spec for the referenced topology changes. Signed-off-by: Francesco Pantano --- controllers/glance_common.go | 2 + controllers/glanceapi_controller.go | 71 ++++++++++------------------- 2 files changed, 25 insertions(+), 48 deletions(-) diff --git a/controllers/glance_common.go b/controllers/glance_common.go index 5af11ac4..f9a7d5d8 100644 --- a/controllers/glance_common.go +++ b/controllers/glance_common.go @@ -49,6 +49,7 @@ const ( caBundleSecretNameField = ".spec.tls.caBundleSecretName" tlsAPIInternalField = ".spec.tls.api.internal.secretName" tlsAPIPublicField = ".spec.tls.api.public.secretName" + topologyField = ".spec.topologyRef.Name" ) var ( @@ -60,6 +61,7 @@ var ( caBundleSecretNameField, tlsAPIInternalField, tlsAPIPublicField, + topologyField, } ) diff --git a/controllers/glanceapi_controller.go b/controllers/glanceapi_controller.go index 09ecd18e..864de87a 100644 --- a/controllers/glanceapi_controller.go +++ b/controllers/glanceapi_controller.go @@ -22,7 +22,6 @@ import ( "strings" batchv1 "k8s.io/api/batch/v1" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -34,7 +33,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -246,6 +244,18 @@ func (r *GlanceAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { return err } + // index topologyField + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &glancev1.GlanceAPI{}, topologyField, func(rawObj client.Object) []string { + // Extract the topology name from the spec, if one is provided + cr := rawObj.(*glancev1.GlanceAPI) + if cr.Spec.Topology == nil { + return nil + } + return []string{cr.Spec.Topology.Name} + }); err != nil { + return err + } + // Watch for changes to any CustomServiceConfigSecrets. Global secrets svcSecretFn := func(_ context.Context, o client.Object) []reconcile.Request { var namespace string = o.GetNamespace() @@ -336,44 +346,6 @@ func (r *GlanceAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { } return nil } - tpFn := predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - oldObj := e.ObjectOld.(*topologyv1.Topology) - newObj := e.ObjectNew.(*topologyv1.Topology) - // Compare spec - return !equality.Semantic.DeepEqual(oldObj.Spec, newObj.Spec) - }, - } - - topologyFn := func(_ context.Context, o client.Object) []reconcile.Request { - result := []reconcile.Request{} - // get all GlanceAPIs CRs - glanceAPIs := &glancev1.GlanceAPIList{} - listOpts := []client.ListOption{ - client.InNamespace(o.GetNamespace()), - } - if err := r.Client.List(context.Background(), glanceAPIs, listOpts...); err != nil { - r.Log.Error(err, "Unable to retrieve GlanceAPI CRs %w") - return nil - } - - for _, cr := range glanceAPIs.Items { - if cr.Spec.Topology != nil { - if o.GetName() == cr.Spec.Topology.Name { - name := client.ObjectKey{ - Namespace: o.GetNamespace(), - Name: cr.Name, - } - r.Log.Info(fmt.Sprintf("Topology %s is used by GlanceAPI CR %s", o.GetName(), cr.Name)) - result = append(result, reconcile.Request{NamespacedName: name}) - } - } - } - if len(result) > 0 { - return result - } - return nil - } return ctrl.NewControllerManagedBy(mgr). For(&glancev1.GlanceAPI{}). @@ -393,8 +365,8 @@ func (r *GlanceAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches(&memcachedv1.Memcached{}, handler.EnqueueRequestsFromMapFunc(memcachedFn)). Watches(&topologyv1.Topology{}, - handler.EnqueueRequestsFromMapFunc(topologyFn), - builder.WithPredicates(tpFn)). + handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc), + builder.WithPredicates(predicate.GenerationChangedPredicate{})). Complete(r) } @@ -857,7 +829,8 @@ func (r *GlanceAPIReconciler) reconcileNormal( // When the Topology CR reference is updated and the current GlanceAPI // switches to a new Topology, remove the finalizer from the previous // Topology - if instance.Status.LastAppliedTopology != "" { + if instance.Spec.Topology == nil || + (instance.Spec.Topology.Name != instance.Status.LastAppliedTopology) { _, err = r.ensureDeletedTopology(ctx, instance, helper) if err != nil { return ctrl.Result{}, err @@ -1564,19 +1537,21 @@ func (r *GlanceAPIReconciler) ensureDeletedTopology( ) (ctrl.Result, error) { ns := instance.Namespace - // no Topology is currently passed to the GlanceAPI, and it was not used - // before - if instance.Spec.Topology == nil && instance.Status.LastAppliedTopology == "" { + // no Topology is passed to the GlanceAPI, and it was not used before + if instance.Status.LastAppliedTopology == "" { return ctrl.Result{}, nil } + + // Topology is referenced in the .Spec, check the namespace if instance.Spec.Topology != nil { - // Check namespace and set name + // Check namespace if instance.Spec.Topology.Namespace != "" { ns = instance.Spec.Topology.Namespace } } name := instance.Status.LastAppliedTopology + // Remove the finalizer from the Topology CR topology, _, err := topologyv1.GetTopologyByName( ctx, @@ -1597,7 +1572,7 @@ func (r *GlanceAPIReconciler) ensureDeletedTopology( if err != nil && !k8s_errors.IsNotFound(err) { return ctrl.Result{}, err } - util.LogForObject(h, "Removed finalizer from Topology", instance) + util.LogForObject(h, "Removed finalizer from Topology", topology) } } return ctrl.Result{}, err