Skip to content

Commit

Permalink
Use findObjectsForSrc to watch the referenced topology
Browse files Browse the repository at this point in the history
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 <fpantano@redhat.com>
  • Loading branch information
fmount committed Jan 13, 2025
1 parent 77e640f commit 3bf5630
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 55 deletions.
2 changes: 1 addition & 1 deletion api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,4 @@ replace github.com/openshift/api => github.com/openshift/api v0.0.0-202408300231

replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/fmount/infra-operator/apis v0.0.0-20250109124018-4262fdefc70b //allow-merging

replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 //allow-merging
replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/fmount/lib-common/modules/common v0.0.0-20250113104104-c9ea0b3613b8 //allow-merging
4 changes: 2 additions & 2 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ github.com/evanphx/json-patch v5.7.0+incompatible h1:vgGkfT/9f8zE6tvSCe74nfpAVDQ
github.com/evanphx/json-patch v5.7.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 h1:FTnFgkzbg5agJorJB4wXfU4LtW8xfGAsozo8It1i6vU=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/fmount/lib-common/modules/common v0.0.0-20250113104104-c9ea0b3613b8 h1:CtAvSW23yErmLWMvIfH2ygs7vs+wP21mt50p+2toYLw=
github.com/fmount/lib-common/modules/common v0.0.0-20250113104104-c9ea0b3613b8/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
Expand Down
2 changes: 2 additions & 0 deletions controllers/glance_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -60,6 +61,7 @@ var (
caBundleSecretNameField,
tlsAPIInternalField,
tlsAPIPublicField,
topologyField,
}
)

Expand Down
71 changes: 23 additions & 48 deletions controllers/glanceapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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{}).
Expand All @@ -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)
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,4 @@ replace github.com/openshift/api => github.com/openshift/api v0.0.0-202408300231

replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/fmount/infra-operator/apis v0.0.0-20250109124018-4262fdefc70b //allow-merging

replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 //allow-merging
replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/fmount/lib-common/modules/common v0.0.0-20250113104104-c9ea0b3613b8 //allow-merging
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0
github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
github.com/fmount/infra-operator/apis v0.0.0-20250109124018-4262fdefc70b h1:wmRi/Aovg7m2PeT1OwVW42817jPlhXHdo1IjPgKli2o=
github.com/fmount/infra-operator/apis v0.0.0-20250109124018-4262fdefc70b/go.mod h1:nGQZr2aelatTWkW/34ijcq8E+oHDUFAcfgGY1UNIITM=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 h1:FTnFgkzbg5agJorJB4wXfU4LtW8xfGAsozo8It1i6vU=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/fmount/lib-common/modules/common v0.0.0-20250113104104-c9ea0b3613b8 h1:CtAvSW23yErmLWMvIfH2ygs7vs+wP21mt50p+2toYLw=
github.com/fmount/lib-common/modules/common v0.0.0-20250113104104-c9ea0b3613b8/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
Expand Down
2 changes: 1 addition & 1 deletion pkg/glanceapi/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func StatefulSet(
// If possible two pods of the same service should not
// run on the same worker node. If this is not possible
// the get still created on the same worker node.
statefulset.Spec.Template.Spec.Affinity, err = affinity.DistributePods(
statefulset.Spec.Template.Spec.Affinity, err = affinity.DistributePodsOverride(
common.AppSelector,
[]string{
glance.ServiceName,
Expand Down

0 comments on commit 3bf5630

Please sign in to comment.