fix: clustercache GetKubernetesClient reuses one client per cluster

lib/clustercache/cluster_cache.go

@@ -49,6 +49,8 @@ type clusterCache struct {
 	rwMux sync.RWMutex
 	// Keeps cache of rest.Config for existing clusters
 	configs map[corev1.ObjectReference]*rest.Config
+	// Keeps cache of clients built from configs
+	clients map[corev1.ObjectReference]client.Client
 
 	mappers               map[corev1.ObjectReference]*restmapper.DeferredDiscoveryRESTMapper
 	cachedDiscoveryClient map[corev1.ObjectReference]discovery.CachedDiscoveryInterface
@@ -69,6 +71,7 @@ func GetManager() *clusterCache {
 		if managerInstance == nil {
 			managerInstance = &clusterCache{
 				configs:               make(map[corev1.ObjectReference]*rest.Config),
+				clients:               make(map[corev1.ObjectReference]client.Client),
 				clusters:              make(map[corev1.ObjectReference]*corev1.ObjectReference),
 				mappers:               make(map[corev1.ObjectReference]*restmapper.DeferredDiscoveryRESTMapper),
 				cachedDiscoveryClient: make(map[corev1.ObjectReference]discovery.CachedDiscoveryInterface),
@@ -92,6 +95,7 @@ func (m *clusterCache) RemoveCluster(clusterNamespace, clusterName string,
 
 	// Remove from cache the restConfig for this cluster
 	delete(m.configs, *cluster)
+	delete(m.clients, *cluster)
 
 	if sec, ok := m.clusters[*cluster]; ok {
 		m.updateSecretMap(sec, cluster)
@@ -117,6 +121,7 @@ func (m *clusterCache) RemoveSecret(sec *corev1.ObjectReference) {
 	clusters := v.Items()
 	for i := range clusters {
 		delete(m.configs, clusters[i])
+		delete(m.clients, clusters[i])
 		delete(m.clusters, clusters[i])
 		delete(m.cachedDiscoveryClient, clusters[i])
 		delete(m.mappers, clusters[i])
@@ -397,8 +402,28 @@ func (m *clusterCache) GetKubernetesClient(ctx context.Context, mgmtClient clien
 		return nil, nil
 	}
 
-	logger.V(logs.LogVerbose).Info("return new client")
-	return client.New(config, client.Options{Scheme: mgmtClient.Scheme()})
+	cluster := getClusterObjectReference(clusterNamespace, clusterName, clusterType)
+
+	m.rwMux.Lock()
+	defer m.rwMux.Unlock()
+
+	if c, ok := m.clients[*cluster]; ok {
+		logger.V(logs.LogVerbose).Info("remote client cache hit")
+		return c, nil
+	}
+
+	logger.V(logs.LogDebug).Info("remote client cache miss")
+	c, err := client.New(config, client.Options{Scheme: mgmtClient.Scheme()})
+	if err != nil {
+		return nil, err
+	}
+
+	// config may have been evicted or replaced since GetKubernetesRestConfig returned it
+	if m.configs[*cluster] == config {
+		m.clients[*cluster] = c
+	}
+
+	return c, nil
 }
 
 func (m *clusterCache) StoreRestConfig(clusterNamespace, clusterName string,
@@ -410,6 +435,7 @@ func (m *clusterCache) StoreRestConfig(clusterNamespace, clusterName string,
 	defer m.rwMux.Unlock()
 
 	m.configs[*cluster] = config
+	delete(m.clients, *cluster)
 }
 
 func (m *clusterCache) updateSecretMap(sec, cluster *corev1.ObjectReference) {

lib/clustercache/cluster_cache_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -29,16 +30,40 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2/textlogger"
 
 	libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1"
 	"github.com/projectsveltos/libsveltos/lib/clustercache"
+	"github.com/projectsveltos/libsveltos/lib/k8s_utils"
 )
 
 const (
 	sveltosKubeconfigPostfix = "-sveltos-kubeconfig"
 	kubeconfigSecretDataKey  = "value"
+
+	widgetCRDTemplate = `apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: widgets.%s
+spec:
+  group: %s
+  scope: Cluster
+  names:
+    kind: Widget
+    listKind: WidgetList
+    plural: widgets
+    singular: widget
+  versions:
+  - name: v1
+    served: true
+    storage: true
+    schema:
+      openAPIV3Schema:
+        type: object
+        x-kubernetes-preserve-unknown-fields: true
+`
 )
 
 var _ = Describe("Clustercache", func() {
@@ -270,6 +295,51 @@ var _ = Describe("Clustercache", func() {
 		}
 		Expect(cacheMgr.GetConfigFromMap(clusterObj)).ToNot(BeNil())
 	})
+
+	It("GetKubernetesClient returns the cached client until the cluster is removed", func() {
+		createClusterResources(cluster)
+
+		cacheMgr := clustercache.GetManager()
+		first, err := cacheMgr.GetKubernetesClient(context.TODO(), testEnv.Client, cluster.Namespace,
+			cluster.Name, "", "", libsveltosv1beta1.ClusterTypeSveltos, logger)
+		Expect(err).To(BeNil())
+		Expect(first).ToNot(BeNil())
+
+		second, err := cacheMgr.GetKubernetesClient(context.TODO(), testEnv.Client, cluster.Namespace,
+			cluster.Name, "", "", libsveltosv1beta1.ClusterTypeSveltos, logger)
+		Expect(err).To(BeNil())
+		Expect(second).To(BeIdenticalTo(first))
+
+		cacheMgr.RemoveCluster(cluster.Namespace, cluster.Name, libsveltosv1beta1.ClusterTypeSveltos)
+
+		third, err := cacheMgr.GetKubernetesClient(context.TODO(), testEnv.Client, cluster.Namespace,
+			cluster.Name, "", "", libsveltosv1beta1.ClusterTypeSveltos, logger)
+		Expect(err).To(BeNil())
+		Expect(third).ToNot(BeIdenticalTo(first))
+	})
+
+	It("GetKubernetesClient cached client resolves a kind whose CRD was installed after its first use", func() {
+		createClusterResources(cluster)
+
+		cacheMgr := clustercache.GetManager()
+		remoteClient, err := cacheMgr.GetKubernetesClient(context.TODO(), testEnv.Client, cluster.Namespace,
+			cluster.Name, "", "", libsveltosv1beta1.ClusterTypeSveltos, logger)
+		Expect(err).To(BeNil())
+		Expect(remoteClient.List(context.TODO(), &corev1.NamespaceList{})).To(Succeed())
+
+		group := randomString() + ".example.com"
+		crd, err := k8s_utils.GetUnstructured([]byte(fmt.Sprintf(widgetCRDTemplate, group, group)))
+		Expect(err).To(BeNil())
+		Expect(testEnv.Create(context.TODO(), crd)).To(Succeed())
+
+		widget := &unstructured.Unstructured{}
+		widget.SetAPIVersion(group + "/v1")
+		widget.SetKind("Widget")
+		widget.SetName(randomString())
+		Eventually(func() error {
+			return remoteClient.Create(context.TODO(), widget)
+		}, 20*time.Second, time.Second).Should(Succeed())
+	})
 })
 
 func createClusterResources(cluster *libsveltosv1beta1.SveltosCluster) *corev1.Secret {