Merge pull request #3292 from tsatam/ARO-4632/operator-predicates

ARO Operator - Reduce unnecessary reconciles by limiting watched resources/changes

pkg/operator/controllers/autosizednodes/autosizednodes_controller.go

@@ -6,7 +6,6 @@ package autosizednodes
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/Azure/go-autorest/autorest/to"
 	mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
@@ -21,6 +20,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 )
 
 type Reconciler struct {
@@ -101,19 +101,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 
 // SetupWithManager prepares the controller with info who to watch
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	clusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		name := o.GetName()
-		return strings.EqualFold(arov1alpha1.SingletonClusterName, name)
-	})
-
-	b := ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(clusterPredicate))
-
-		// Controller adds ControllerManagedBy to KubeletConfit created by this controller.
-		// Any changes will trigger reconcile, but only for that config.
-	return b.
-		Named(ControllerName).
+	// Controller adds ControllerManagedBy to KubeletConfit created by this controller.
+	// Any changes will trigger reconcile, but only for that config.
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Owns(&mcv1.KubeletConfig{}).
+		Named(ControllerName).
 		Complete(r)
 }
 

pkg/operator/controllers/banner/banner_controller.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 )
 
 const (
@@ -59,16 +60,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 
 // SetupWithManager creates the controller
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	aroBannerPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
 		return o.GetName() == BannerName
 	})
 
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		// watching ConsoleNotifications in case a user edits it
 		Watches(&source.Kind{Type: &consolev1.ConsoleNotification{}}, &handler.EnqueueRequestForObject{}, builder.WithPredicates(aroBannerPredicate)).
 		Named(ControllerName).

pkg/operator/controllers/checkers/clusterdnschecker/controller.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/conditions"
 )
 
@@ -117,16 +118,12 @@ func (r *Reconciler) condition(checkErr error, result result) *operatorv1.Operat
 
 // SetupWithManager setup our manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	defaultClusterDNSPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
 		return o.GetName() == "default"
 	})
 
 	builder := ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Watches(
 			&source.Kind{Type: &operatorv1.DNS{}},
 			&handler.EnqueueRequestForObject{},

pkg/operator/controllers/checkers/ingresscertificatechecker/controller.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/conditions"
 )
 
@@ -118,20 +119,12 @@ func (r *Reconciler) condition(checkErr error) *operatorv1.OperatorCondition {
 
 // SetupWithManager setup our manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	defaultIngressControllerPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
 		return o.GetNamespace() == "openshift-ingress-operator" && o.GetName() == "default"
 	})
 
-	clusterVersionPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == "version"
-	})
-
 	builder := ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Watches(
 			&source.Kind{Type: &operatorv1.IngressController{}},
 			&handler.EnqueueRequestForObject{},
@@ -140,7 +133,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(
 			&source.Kind{Type: &configv1.ClusterVersion{}},
 			&handler.EnqueueRequestForObject{},
-			builder.WithPredicates(clusterVersionPredicate),
+			builder.WithPredicates(predicates.ClusterVersion),
 		)
 
 	return builder.Named(ControllerName).Complete(r)

pkg/operator/controllers/checkers/internetchecker/controller.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/conditions"
 )
 
@@ -122,12 +123,8 @@ func (r *Reconciler) conditionType() string {
 
 // SetupWithManager setup our manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
-	builder := ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate))
-
-	return builder.Named(ControllerName).Complete(r)
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
+		Named(ControllerName).
+		Complete(r)
 }

pkg/operator/controllers/checkers/serviceprincipalchecker/controller.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/clusterauthorizer"
 	"github.com/Azure/ARO-RP/pkg/util/conditions"
 )
@@ -114,16 +115,12 @@ func (r *Reconciler) condition(checkErr error) *operatorv1.OperatorCondition {
 
 // SetupWithManager setup our manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	clusterSPPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
 		return o.GetName() == clusterauthorizer.AzureCredentialSecretName && o.GetNamespace() == clusterauthorizer.AzureCredentialSecretNameSpace
 	})
 
 	builder := ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Watches(
 			&source.Kind{Type: &corev1.Secret{}},
 			&handler.EnqueueRequestForObject{},

pkg/operator/controllers/cloudproviderconfig/cloudproviderconfig_controller.go

@@ -23,6 +23,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 )
 
 const (
@@ -164,16 +165,12 @@ func (r *CloudProviderConfigReconciler) Reconcile(ctx context.Context, request c
 func (r *CloudProviderConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	r.Log.Info("starting cloud-provider-config controller")
 
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	cloudProviderConfigPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
 		return o.GetName() == cloudProviderConfigName.Name && o.GetNamespace() == cloudProviderConfigName.Namespace
 	})
 
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Watches(
 			&source.Kind{Type: &corev1.ConfigMap{}},
 			&handler.EnqueueRequestForObject{},

pkg/operator/controllers/clusteroperatoraro/clusteroperatoraro_controller.go

@@ -21,10 +21,10 @@ 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/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/version"
 )
 
@@ -163,12 +163,9 @@ func (r *Reconciler) defaultOperator() *configv1.ClusterOperator {
 
 // SetupWithManager setup our manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		// we want to reconcile on status changes on the ARO Cluster resource here, unlike most other reconcilers
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicates.AROCluster)).
 		Owns(&configv1.ClusterOperator{}).
 		Named(ControllerName).
 		Complete(r)

pkg/operator/controllers/dnsmasq/cluster_controller.go

@@ -18,6 +18,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
 )
 
@@ -81,12 +82,8 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 
 // SetupWithManager setup our mananger
 func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Named(ClusterControllerName).
 		Complete(r)
 }

pkg/operator/controllers/genevalogging/genevalogging_controller.go

@@ -20,6 +20,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
 )
 
@@ -111,12 +112,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 
 // SetupWithManager setup our manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Owns(&appsv1.DaemonSet{}).
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Namespace{}).

pkg/operator/controllers/guardrails/guardrails_controller.go

@@ -22,6 +22,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/guardrails/config"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/deployer"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
 )
@@ -167,12 +168,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 
 // SetupWithManager setup our manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
 	grBuilder := ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate))
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{})))
 
 	resources, err := r.deployer.Template(&config.GuardRailsDeploymentConfig{}, staticFiles)
 	if err != nil {

pkg/operator/controllers/ingress/ingress_controller.go

@@ -19,6 +19,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 )
 
 const (
@@ -80,12 +81,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 
 // SetupWithManager setup the mananger for openshift ingress controller resource
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == arov1alpha1.SingletonClusterName
-	})
-
-	builder := ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate))
-
-	return builder.Named(ControllerName).Complete(r)
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
+		Named(ControllerName).
+		Complete(r)
 }

pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller.go

@@ -6,7 +6,6 @@ package machinehealthcheck
 import (
 	"context"
 	_ "embed"
-	"strings"
 	"time"
 
 	configv1 "github.com/openshift/api/config/v1"
@@ -27,6 +26,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
 )
 
@@ -166,22 +166,15 @@ func (r *Reconciler) isClusterUpgrading(ctx context.Context) (bool, error) {
 
 // SetupWithManager will manage only our MHC resource with our specific controller name
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return strings.EqualFold(arov1alpha1.SingletonClusterName, o.GetName())
-	})
-	clusterVersionPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		return o.GetName() == "version"
-	})
-
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&arov1alpha1.Cluster{}, builder.WithPredicates(aroClusterPredicate)).
+		For(&arov1alpha1.Cluster{}, builder.WithPredicates(predicate.And(predicates.AROCluster, predicate.GenerationChangedPredicate{}))).
 		Named(ControllerName).
 		Owns(&machinev1beta1.MachineHealthCheck{}).
 		Owns(&monitoringv1.PrometheusRule{}).
 		Watches(
 			&source.Kind{Type: &configv1.ClusterVersion{}},
 			&handler.EnqueueRequestForObject{},
-			builder.WithPredicates(clusterVersionPredicate),
+			builder.WithPredicates(predicates.ClusterVersion),
 		).
 		Complete(r)
 }

pkg/operator/controllers/machineset/machineset_controller.go

@@ -15,11 +15,11 @@ import (
 	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/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/Azure/ARO-RP/pkg/operator"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
+	"github.com/Azure/ARO-RP/pkg/operator/predicates"
 )
 
 const (
@@ -108,13 +108,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 }
 
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	machineSetPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
-		role := o.GetLabels()["machine.openshift.io/cluster-api-machine-role"]
-		return strings.EqualFold("worker", role)
-	})
-
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&machinev1beta1.MachineSet{}, builder.WithPredicates(machineSetPredicate)).
+		For(&machinev1beta1.MachineSet{}, builder.WithPredicates(predicates.MachineRoleWorker)).
 		Named(ControllerName).
 		Complete(r)
 }