From 01197892a4d9fc20ca525940feaafe42ab476191 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 30 Apr 2022 17:11:53 +0200 Subject: [PATCH] refactor: optimizing watchers predicates --- controllers/config/manager.go | 27 ++----------------- controllers/rbac/manager.go | 39 ++++----------------------- controllers/secret/ca.go | 4 +-- controllers/servicelabels/abstract.go | 24 +++++------------ controllers/utils/name_matching.go | 19 +++++++++++++ 5 files changed, 34 insertions(+), 79 deletions(-) create mode 100644 controllers/utils/name_matching.go diff --git a/controllers/config/manager.go b/controllers/config/manager.go index b396f148..5c3e64be 100644 --- a/controllers/config/manager.go +++ b/controllers/config/manager.go @@ -9,13 +9,11 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" + "github.com/clastix/capsule/controllers/utils" "github.com/clastix/capsule/pkg/configuration" ) @@ -31,30 +29,9 @@ func (c *Manager) InjectClient(client client.Client) error { return nil } -func filterByName(objName, desired string) bool { - return objName == desired -} - -func forOptionPerInstanceName(instanceName string) builder.ForOption { - return builder.WithPredicates(predicate.Funcs{ - CreateFunc: func(event event.CreateEvent) bool { - return filterByName(event.Object.GetName(), instanceName) - }, - DeleteFunc: func(deleteEvent event.DeleteEvent) bool { - return filterByName(deleteEvent.Object.GetName(), instanceName) - }, - UpdateFunc: func(updateEvent event.UpdateEvent) bool { - return filterByName(updateEvent.ObjectNew.GetName(), instanceName) - }, - GenericFunc: func(genericEvent event.GenericEvent) bool { - return filterByName(genericEvent.Object.GetName(), instanceName) - }, - }) -} - func (c *Manager) SetupWithManager(mgr ctrl.Manager, configurationName string) error { return ctrl.NewControllerManagedBy(mgr). - For(&capsulev1alpha1.CapsuleConfiguration{}, forOptionPerInstanceName(configurationName)). + For(&capsulev1alpha1.CapsuleConfiguration{}, utils.NamesMatchingPredicate(configurationName)). Complete(c) } diff --git a/controllers/rbac/manager.go b/controllers/rbac/manager.go index c4a1aa03..92699c8d 100644 --- a/controllers/rbac/manager.go +++ b/controllers/rbac/manager.go @@ -14,16 +14,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" - "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/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" + "github.com/clastix/capsule/controllers/utils" "github.com/clastix/capsule/pkg/configuration" ) @@ -39,46 +38,18 @@ func (r *Manager) InjectClient(c client.Client) error { return nil } -func (r *Manager) filterByNames(name string) bool { - return name == ProvisionerRoleName || name == DeleterRoleName -} - //nolint:dupl func (r *Manager) SetupWithManager(ctx context.Context, mgr ctrl.Manager, configurationName string) (err error) { + namesPredicate := utils.NamesMatchingPredicate(ProvisionerRoleName, DeleterRoleName) + crErr := ctrl.NewControllerManagedBy(mgr). - For(&rbacv1.ClusterRole{}, builder.WithPredicates(predicate.Funcs{ - CreateFunc: func(event event.CreateEvent) bool { - return r.filterByNames(event.Object.GetName()) - }, - DeleteFunc: func(deleteEvent event.DeleteEvent) bool { - return r.filterByNames(deleteEvent.Object.GetName()) - }, - UpdateFunc: func(updateEvent event.UpdateEvent) bool { - return r.filterByNames(updateEvent.ObjectNew.GetName()) - }, - GenericFunc: func(genericEvent event.GenericEvent) bool { - return r.filterByNames(genericEvent.Object.GetName()) - }, - })). + For(&rbacv1.ClusterRole{}, namesPredicate). Complete(r) if crErr != nil { err = multierror.Append(err, crErr) } crbErr := ctrl.NewControllerManagedBy(mgr). - For(&rbacv1.ClusterRoleBinding{}, builder.WithPredicates(predicate.Funcs{ - CreateFunc: func(event event.CreateEvent) bool { - return r.filterByNames(event.Object.GetName()) - }, - DeleteFunc: func(deleteEvent event.DeleteEvent) bool { - return r.filterByNames(deleteEvent.Object.GetName()) - }, - UpdateFunc: func(updateEvent event.UpdateEvent) bool { - return r.filterByNames(updateEvent.ObjectNew.GetName()) - }, - GenericFunc: func(genericEvent event.GenericEvent) bool { - return r.filterByNames(genericEvent.Object.GetName()) - }, - })). + For(&rbacv1.ClusterRoleBinding{}, namesPredicate). Watches(source.NewKindWithCache(&capsulev1alpha1.CapsuleConfiguration{}, mgr.GetCache()), handler.Funcs{ UpdateFunc: func(updateEvent event.UpdateEvent, limitingInterface workqueue.RateLimitingInterface) { if updateEvent.ObjectNew.GetName() == configurationName { diff --git a/controllers/secret/ca.go b/controllers/secret/ca.go index 343183c8..4336d060 100644 --- a/controllers/secret/ca.go +++ b/controllers/secret/ca.go @@ -54,10 +54,10 @@ func (r *CAReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&corev1.Secret{}). Watches(source.NewKindWithCache(&admissionregistrationv1.ValidatingWebhookConfiguration{}, mgr.GetCache()), enqueueFn, builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { - return r.Configuration.ValidatingWebhookConfigurationName() == object.GetName() + return object.GetName() == r.Configuration.ValidatingWebhookConfigurationName() }))). Watches(source.NewKindWithCache(&admissionregistrationv1.MutatingWebhookConfiguration{}, mgr.GetCache()), enqueueFn, builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { - return r.Configuration.MutatingWebhookConfigurationName() == object.GetName() + return object.GetName() == r.Configuration.MutatingWebhookConfigurationName() }))). Complete(r) } diff --git a/controllers/servicelabels/abstract.go b/controllers/servicelabels/abstract.go index 1f6cbfb3..b88b228f 100644 --- a/controllers/servicelabels/abstract.go +++ b/controllers/servicelabels/abstract.go @@ -17,7 +17,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/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -104,26 +103,15 @@ func (r *abstractServiceLabelsReconciler) sync(available map[string]string, tena return available } -func (r *abstractServiceLabelsReconciler) forOptionPerInstanceName() builder.ForOption { - return builder.WithPredicates(predicate.Funcs{ - CreateFunc: func(event event.CreateEvent) bool { - return r.IsNamespaceInTenant(event.Object.GetNamespace()) - }, - DeleteFunc: func(deleteEvent event.DeleteEvent) bool { - return r.IsNamespaceInTenant(deleteEvent.Object.GetNamespace()) - }, - UpdateFunc: func(updateEvent event.UpdateEvent) bool { - return r.IsNamespaceInTenant(updateEvent.ObjectNew.GetNamespace()) - }, - GenericFunc: func(genericEvent event.GenericEvent) bool { - return r.IsNamespaceInTenant(genericEvent.Object.GetNamespace()) - }, - }) +func (r *abstractServiceLabelsReconciler) forOptionPerInstanceName(ctx context.Context) builder.ForOption { + return builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { + return r.IsNamespaceInTenant(ctx, object.GetNamespace()) + })) } -func (r *abstractServiceLabelsReconciler) IsNamespaceInTenant(namespace string) bool { +func (r *abstractServiceLabelsReconciler) IsNamespaceInTenant(ctx context.Context, namespace string) bool { tl := &capsulev1beta1.TenantList{} - if err := r.client.List(context.Background(), tl, client.MatchingFieldsSelector{ + if err := r.client.List(ctx, tl, client.MatchingFieldsSelector{ Selector: fields.OneTermEqualSelector(".status.namespaces", namespace), }); err != nil { return false diff --git a/controllers/utils/name_matching.go b/controllers/utils/name_matching.go new file mode 100644 index 00000000..7b1c540c --- /dev/null +++ b/controllers/utils/name_matching.go @@ -0,0 +1,19 @@ +package utils + +import ( + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +func NamesMatchingPredicate(names ...string) builder.Predicates { + return builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { + for _, name := range names { + if object.GetName() == name { + return true + } + } + + return false + })) +}