fix: reduce unnecessary caching and remove predicate from dynamic watches (#963)

Our cache configuration resulted in watching all cluster-scoped
resources for each cluster-scoped resource that shows up in a
ClusterExtension. Now, we only watch:
- All ClusterExtensions
- All ClusterCatalogs
- All objects in the systemNamespace
- All other objects that specifically match the dependentPredicate

This commit also removes the dependent predicate so that we reconcile
for any change to managed resources. This is important to ensure that
future features (like health checking) can account for any change to
managed operator objects.

Signed-off-by: Joe Lanford <[email protected]>

Author: joelanford

cmd/manager/main.go

@@ -37,6 +37,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
+	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
 	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
 	registryv1handler "github.com/operator-framework/rukpak/pkg/handler"
 	"github.com/operator-framework/rukpak/pkg/provisioner/registry"
@@ -127,12 +128,13 @@ func main() {
 		LeaderElectionID:       "9c4404e7.operatorframework.io",
 		Cache: crcache.Options{
 			ByObject: map[client.Object]crcache.ByObject{
-				&ocv1alpha1.ClusterExtension{}: {},
+				&ocv1alpha1.ClusterExtension{}: {Label: k8slabels.Everything()},
+				&catalogd.ClusterCatalog{}:     {Label: k8slabels.Everything()},
 			},
 			DefaultNamespaces: map[string]crcache.Config{
-				systemNamespace:       {},
-				crcache.AllNamespaces: {LabelSelector: dependentSelector},
+				systemNamespace: {LabelSelector: k8slabels.Everything()},
 			},
+			DefaultLabelSelector: dependentSelector,
 		},
 		// LeaderElectionReleaseOnCancel defines if the leader should step down voluntarily
 		// when the Manager ends. This requires the binary to immediately end when the

internal/controllers/clusterextension_controller.go

@@ -45,6 +45,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crcontroller "sigs.k8s.io/controller-runtime/pkg/controller"
@@ -63,7 +64,6 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/property"
 	rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
 	registryv1handler "github.com/operator-framework/rukpak/pkg/handler"
-	helmpredicate "github.com/operator-framework/rukpak/pkg/helm-operator-plugins/predicate"
 	rukpaksource "github.com/operator-framework/rukpak/pkg/source"
 	"github.com/operator-framework/rukpak/pkg/storage"
 	"github.com/operator-framework/rukpak/pkg/util"
@@ -357,7 +357,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 					source.Kind(r.cache,
 						obj,
 						crhandler.EnqueueRequestForOwner(r.Scheme(), r.RESTMapper(), ext, crhandler.OnlyControllerOwner()),
-						helmpredicate.DependentPredicateFuncs[client.Object](),
 					),
 				); err != nil {
 					return err
@@ -572,24 +571,24 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	controller, err := ctrl.NewControllerManagedBy(mgr).
 		For(&ocv1alpha1.ClusterExtension{}).
 		Watches(&catalogd.ClusterCatalog{},
-			crhandler.EnqueueRequestsFromMapFunc(clusterExtensionRequestsForCatalog(mgr.GetClient(), mgr.GetLogger()))).
-		WithEventFilter(predicate.Funcs{
-			UpdateFunc: func(ue event.UpdateEvent) bool {
-				oldObject, isOldCatalog := ue.ObjectOld.(*catalogd.ClusterCatalog)
-				newObject, isNewCatalog := ue.ObjectNew.(*catalogd.ClusterCatalog)
-
-				if !isOldCatalog || !isNewCatalog {
-					return true
-				}
+			crhandler.EnqueueRequestsFromMapFunc(clusterExtensionRequestsForCatalog(mgr.GetClient(), mgr.GetLogger())),
+			builder.WithPredicates(predicate.Funcs{
+				UpdateFunc: func(ue event.UpdateEvent) bool {
+					oldObject, isOldCatalog := ue.ObjectOld.(*catalogd.ClusterCatalog)
+					newObject, isNewCatalog := ue.ObjectNew.(*catalogd.ClusterCatalog)
+
+					if !isOldCatalog || !isNewCatalog {
+						return true
+					}
 
-				if oldObject.Status.ResolvedSource != nil && newObject.Status.ResolvedSource != nil {
-					if oldObject.Status.ResolvedSource.Image != nil && newObject.Status.ResolvedSource.Image != nil {
-						return oldObject.Status.ResolvedSource.Image.ResolvedRef != newObject.Status.ResolvedSource.Image.ResolvedRef
+					if oldObject.Status.ResolvedSource != nil && newObject.Status.ResolvedSource != nil {
+						if oldObject.Status.ResolvedSource.Image != nil && newObject.Status.ResolvedSource.Image != nil {
+							return oldObject.Status.ResolvedSource.Image.ResolvedRef != newObject.Status.ResolvedSource.Image.ResolvedRef
+						}
 					}
-				}
-				return true
-			},
-		}).
+					return true
+				},
+			})).
 		Build(r)
 
 	if err != nil {