Reduce the number of instantiated/used ackrtcache.Caches objects. (#32)

Issue: N/A

Currently, each ACK reconciler uses its own `ackrtcache.Caches`
instance. Long term this might cause some issues since each Cache
instance require its own KubeClient/shared informer etc...

This patch reduces the number of instantiated and used
`ackrtcache.Caches` to one. All the reconcilers will now use a copy of
the same cache instance since it's thread-safe.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: a-hilaly

pkg/runtime/adoption_reconciler.go

@@ -21,8 +21,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	kubernetes "k8s.io/client-go/kubernetes"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	ctrlrt "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	k8sctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
 	ackcfg "github.com/aws-controllers-k8s/runtime/pkg/config"
@@ -32,9 +34,6 @@ import (
 	ackrtcache "github.com/aws-controllers-k8s/runtime/pkg/runtime/cache"
 	ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
 	acktypes "github.com/aws-controllers-k8s/runtime/pkg/types"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	k8sctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 const (
@@ -52,14 +51,7 @@ type adoptionReconciler struct {
 // BindControllerManager sets up the AWSResourceReconciler with an instance
 // of an upstream controller-runtime.Manager
 func (r *adoptionReconciler) BindControllerManager(mgr ctrlrt.Manager) error {
-	clusterConfig := mgr.GetConfig()
-	clientset, err := kubernetes.NewForConfig(clusterConfig)
-	if err != nil {
-		return err
-	}
 	r.kc = mgr.GetClient()
-	r.cache = ackrtcache.New(clientset, r.log, r.cfg.WatchNamespace)
-	r.cache.Run()
 	return ctrlrt.NewControllerManagedBy(
 		mgr,
 	).For(
@@ -468,13 +460,15 @@ func NewAdoptionReconciler(
 	log logr.Logger,
 	cfg ackcfg.Config,
 	metrics *ackmetrics.Metrics,
+	cache ackrtcache.Caches,
 ) acktypes.Reconciler {
 	return &adoptionReconciler{
 		reconciler: reconciler{
 			sc:      sc,
 			log:     log.WithName("adopted-reconciler"),
 			cfg:     cfg,
 			metrics: metrics,
+			cache:   cache,
 		},
 	}
 }

pkg/runtime/cache/cache.go

@@ -58,7 +58,7 @@ type Caches struct {
 
 // New creates a new Caches object from a kubernetes.Interface and
 // a logr.Logger
-func New(clientset kubernetes.Interface, log logr.Logger, watchNamespace  string) Caches {
+func New(clientset kubernetes.Interface, log logr.Logger, watchNamespace string) Caches {
 	return Caches{
 		Accounts:   NewAccountCache(clientset, log),
 		Namespaces: NewNamespaceCache(clientset, log, watchNamespace),

pkg/runtime/reconciler.go

@@ -22,7 +22,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	kubernetes "k8s.io/client-go/kubernetes"
 	ctrlrt "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -75,14 +74,7 @@ func (r *resourceReconciler) BindControllerManager(mgr ctrlrt.Manager) error {
 	if r.rmf == nil {
 		return ackerr.NilResourceManagerFactory
 	}
-	clusterConfig := mgr.GetConfig()
-	clientset, err := kubernetes.NewForConfig(clusterConfig)
-	if err != nil {
-		return err
-	}
 	r.kc = mgr.GetClient()
-	r.cache = ackrtcache.New(clientset, r.log, r.cfg.WatchNamespace)
-	r.cache.Run()
 	rd := r.rmf.ResourceDescriptor()
 	return ctrlrt.NewControllerManagedBy(
 		mgr,
@@ -619,8 +611,9 @@ func NewReconciler(
 	log logr.Logger,
 	cfg ackcfg.Config,
 	metrics *ackmetrics.Metrics,
+	cache ackrtcache.Caches,
 ) acktypes.AWSResourceReconciler {
-	return NewReconcilerWithClient(sc, nil, rmf, log, cfg, metrics)
+	return NewReconcilerWithClient(sc, nil, rmf, log, cfg, metrics, cache)
 }
 
 // NewReconcilerWithClient returns a new reconciler object
@@ -632,6 +625,7 @@ func NewReconcilerWithClient(
 	log logr.Logger,
 	cfg ackcfg.Config,
 	metrics *ackmetrics.Metrics,
+	cache ackrtcache.Caches,
 ) acktypes.AWSResourceReconciler {
 	return &resourceReconciler{
 		reconciler: reconciler{
@@ -640,6 +634,7 @@ func NewReconcilerWithClient(
 			log:     log.WithName("ackrt"),
 			cfg:     cfg,
 			metrics: metrics,
+			cache:   cache,
 		},
 		rmf: rmf,
 		rd:  rmf.ResourceDescriptor(),

pkg/runtime/reconciler_test.go

@@ -30,6 +30,7 @@ import (
 	ackcfg "github.com/aws-controllers-k8s/runtime/pkg/config"
 	ackmetrics "github.com/aws-controllers-k8s/runtime/pkg/metrics"
 	ackrt "github.com/aws-controllers-k8s/runtime/pkg/runtime"
+	ackrtcache "github.com/aws-controllers-k8s/runtime/pkg/runtime/cache"
 
 	k8srtmocks "github.com/aws-controllers-k8s/runtime/mocks/apimachinery/pkg/runtime"
 	k8srtschemamocks "github.com/aws-controllers-k8s/runtime/mocks/apimachinery/pkg/runtime/schema"
@@ -142,7 +143,7 @@ func TestReconcilerUpdate(t *testing.T) {
 	// `AWSResourceDescriptor.Delta()` returned a non-empty Delta, that we end
 	// up calling the AWSResourceManager.Update() call in the Reconciler.Sync()
 	// method,
-	r := ackrt.NewReconcilerWithClient(sc, kc, rmf, fakeLogger, cfg, metrics)
+	r := ackrt.NewReconcilerWithClient(sc, kc, rmf, fakeLogger, cfg, metrics, ackrtcache.Caches{})
 
 	err := r.Sync(ctx, rm, desired)
 	require.Nil(err)
@@ -259,7 +260,7 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInMetadata(t *testing.T) {
 	// `AWSResourceDescriptor.Delta()` returned a non-empty Delta, that we end
 	// up calling the AWSResourceManager.Update() call in the Reconciler.Sync()
 	// method,
-	r := ackrt.NewReconcilerWithClient(sc, kc, rmf, fakeLogger, cfg, metrics)
+	r := ackrt.NewReconcilerWithClient(sc, kc, rmf, fakeLogger, cfg, metrics, ackrtcache.Caches{})
 
 	err := r.Sync(ctx, rm, desired)
 	require.Nil(err)
@@ -375,7 +376,7 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInSpec(t *testing.T) {
 	// `AWSResourceDescriptor.Delta()` returned a non-empty Delta, that we end
 	// up calling the AWSResourceManager.Update() call in the Reconciler.Sync()
 	// method,
-	r := ackrt.NewReconcilerWithClient(sc, kc, rmf, fakeLogger, cfg, metrics)
+	r := ackrt.NewReconcilerWithClient(sc, kc, rmf, fakeLogger, cfg, metrics, ackrtcache.Caches{})
 
 	err := r.Sync(ctx, rm, desired)
 	require.Nil(err)

pkg/runtime/service_controller.go

@@ -18,10 +18,12 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/prometheus/client_golang/prometheus"
+	kubernetes "k8s.io/client-go/kubernetes"
 	ctrlrt "sigs.k8s.io/controller-runtime"
 
 	ackcfg "github.com/aws-controllers-k8s/runtime/pkg/config"
 	ackmetrics "github.com/aws-controllers-k8s/runtime/pkg/metrics"
+	ackrtcache "github.com/aws-controllers-k8s/runtime/pkg/runtime/cache"
 	acktypes "github.com/aws-controllers-k8s/runtime/pkg/types"
 )
 
@@ -130,15 +132,24 @@ func (c *serviceController) WithResourceManagerFactories(
 func (c *serviceController) BindControllerManager(mgr ctrlrt.Manager, cfg ackcfg.Config) error {
 	c.metaLock.Lock()
 	defer c.metaLock.Unlock()
+
+	clusterConfig := mgr.GetConfig()
+	clientset, err := kubernetes.NewForConfig(clusterConfig)
+	if err != nil {
+		return err
+	}
+	cache := ackrtcache.New(clientset, c.log, cfg.WatchNamespace)
+	cache.Run()
+
 	for _, rmf := range c.rmFactories {
-		rec := NewReconciler(c, rmf, c.log, cfg, c.metrics)
+		rec := NewReconciler(c, rmf, c.log, cfg, c.metrics, cache)
 		if err := rec.BindControllerManager(mgr); err != nil {
 			return err
 		}
 		c.reconcilers = append(c.reconcilers, rec)
 	}
 
-	rec := NewAdoptionReconciler(c, c.log, cfg, c.metrics)
+	rec := NewAdoptionReconciler(c, c.log, cfg, c.metrics, cache)
 	if err := rec.BindControllerManager(mgr); err != nil {
 		return err
 	}