Use a cache when creating a new helm client to avoid initialization

cmd/manager/main.go

@@ -47,6 +47,7 @@ var (
 )
 
 func main() {
+
 	var metricsAddr string
 	var enableLeaderElection bool
 	var disableWebhooks bool
@@ -118,13 +119,15 @@ func main() {
 	eventBroadcaster.StartLogging(func(format string, args ...interface{}) { logg.Info(fmt.Sprintf(format, args...)) })
 	eventBroadcaster.StartRecordingToSink(&typedcorev1.EventSinkImpl{Interface: clientSet.CoreV1().Events("")})
 
+	factory := chart.NewHelmClientFactory()
+
 	if err = (&controllers.AppReconciler{
 		TemplateReader: storage,
 		Client:         mgr.GetClient(),
 		Log:            logg,
 		Scheme:         mgr.GetScheme(),
 		HelmFactoryFn: func(namespace string) (controllers.Helm, error) {
-			return chart.NewHelmClient(namespace, mgr.GetClient(), logg)
+			return factory.NewHelmClient(namespace, mgr.GetClient(), logg)
 		},
 		Now:   time.Now,
 		Group: group,
@@ -145,7 +148,7 @@ func main() {
 		Scheme:         mgr.GetScheme(),
 		TemplateReader: storage,
 		HelmFactoryFn: func(namespace string) (controllers.Helm, error) {
-			return chart.NewHelmClient(namespace, mgr.GetClient(), logg)
+			return factory.NewHelmClient(namespace, mgr.GetClient(), logg)
 		},
 		Recorder: eventBroadcaster.NewRecorder(clientgoscheme.Scheme, v1.EventSource{
 			Component: "ketch-controller",

internal/chart/helm_client.go

@@ -3,8 +3,6 @@ package chart
 import (
 	"errors"
 	"fmt"
-	"log"
-	"os"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -14,8 +12,6 @@ import (
 	"helm.sh/helm/v3/pkg/storage/driver"
 	helmTime "helm.sh/helm/v3/pkg/time"
 
-	"k8s.io/cli-runtime/pkg/genericclioptions"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -39,32 +35,6 @@ type TemplateValuer interface {
 	GetName() string
 }
 
-// NewHelmClient returns a HelmClient instance.
-func NewHelmClient(namespace string, c client.Client, log logr.Logger) (*HelmClient, error) {
-	cfg, err := getActionConfig(namespace)
-	if err != nil {
-		return nil, err
-	}
-	return &HelmClient{cfg: cfg, namespace: namespace, c: c, log: log.WithValues("helm-client", namespace)}, nil
-}
-
-func getActionConfig(namespace string) (*action.Configuration, error) {
-	actionConfig := new(action.Configuration)
-
-	config := ctrl.GetConfigOrDie()
-
-	// Create the ConfigFlags struct instance with initialized values from ServiceAccount
-	kubeConfig := genericclioptions.NewConfigFlags(false)
-	kubeConfig.APIServer = &config.Host
-	kubeConfig.BearerToken = &config.BearerToken
-	kubeConfig.CAFile = &config.CAFile
-	kubeConfig.Namespace = &namespace
-	if err := actionConfig.Init(kubeConfig, namespace, os.Getenv("HELM_DRIVER"), log.Printf); err != nil {
-		return nil, err
-	}
-	return actionConfig, nil
-}
-
 // InstallOption to perform additional configuration of action.Install before running a chart installation.
 type InstallOption func(install *action.Install)
 

internal/chart/helm_client_factory.go

@@ -0,0 +1,85 @@
+package chart
+
+import (
+	"log"
+	"os"
+	"sync"
+	"time"
+
+	"github.com/go-logr/logr"
+	"helm.sh/helm/v3/pkg/action"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// HelmClientFactory provides "NewHelmClient()" method to get a helm client.
+// HelmClientFactory internally maintains a cache for action.Configurations per k8s namespace
+// decreasing the cost of creating a new helm client.
+type HelmClientFactory struct {
+	sync.Mutex
+	configurations              map[string]*action.Configuration // map[namespaceName]*action.Configuration
+	configurationsLastUsedTimes map[string]time.Time
+
+	lastCleanupTime time.Time
+
+	getActionConfig func(namespace string) (*action.Configuration, error)
+}
+
+func NewHelmClientFactory() *HelmClientFactory {
+	return &HelmClientFactory{
+		configurations:              map[string]*action.Configuration{},
+		configurationsLastUsedTimes: map[string]time.Time{},
+		getActionConfig:             getActionConfig,
+	}
+}
+
+// NewHelmClient returns a HelmClient instance.
+func (f *HelmClientFactory) NewHelmClient(namespace string, c client.Client, log logr.Logger) (*HelmClient, error) {
+	f.Lock()
+	defer f.Unlock()
+
+	f.cleanup()
+
+	cfg, ok := f.configurations[namespace]
+	if !ok {
+		var err error
+		cfg, err = f.getActionConfig(namespace)
+		if err != nil {
+			return nil, err
+		}
+		f.configurations[namespace] = cfg
+	}
+	f.configurationsLastUsedTimes[namespace] = time.Now()
+	return &HelmClient{cfg: cfg, namespace: namespace, c: c, log: log.WithValues("helm-client", namespace)}, nil
+}
+
+func (f *HelmClientFactory) cleanup() {
+	if time.Since(f.lastCleanupTime) < 10*time.Minute {
+		return
+	}
+	for ns, timestamp := range f.configurationsLastUsedTimes {
+		if timestamp.Before(f.lastCleanupTime) {
+			delete(f.configurations, ns)
+			delete(f.configurationsLastUsedTimes, ns)
+		}
+	}
+	f.lastCleanupTime = time.Now()
+}
+
+func getActionConfig(namespace string) (*action.Configuration, error) {
+	actionConfig := new(action.Configuration)
+
+	config := ctrl.GetConfigOrDie()
+
+	// Create the ConfigFlags struct instance with initialized values from ServiceAccount
+	kubeConfig := genericclioptions.NewConfigFlags(false)
+	kubeConfig.APIServer = &config.Host
+	kubeConfig.BearerToken = &config.BearerToken
+	kubeConfig.CAFile = &config.CAFile
+	kubeConfig.Namespace = &namespace
+	if err := actionConfig.Init(kubeConfig, namespace, os.Getenv("HELM_DRIVER"), log.Printf); err != nil {
+		return nil, err
+	}
+	return actionConfig, nil
+}

internal/chart/helm_client_factory_test.go

@@ -0,0 +1,80 @@
+package chart
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"helm.sh/helm/v3/pkg/action"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+)
+
+func TestHelmClientFactory_NewHelmClient_VerifyCacheIsUsed(t *testing.T) {
+
+	getActionConfigIsCalled := map[string]int{}
+
+	factory := &HelmClientFactory{
+		configurations:              map[string]*action.Configuration{},
+		configurationsLastUsedTimes: map[string]time.Time{},
+		getActionConfig: func(namespace string) (*action.Configuration, error) {
+			getActionConfigIsCalled[namespace] += 1
+			return &action.Configuration{}, nil
+		},
+	}
+	now := time.Now()
+	cli, err := factory.NewHelmClient("my-namespace", nil, log.NullLogger{})
+	require.Nil(t, err)
+	require.NotNil(t, cli)
+	require.True(t, factory.configurationsLastUsedTimes["my-namespace"].After(now))
+	require.True(t, factory.configurationsLastUsedTimes["my-namespace"].Before(time.Now()))
+	require.Equal(t, map[string]int{"my-namespace": 1}, getActionConfigIsCalled)
+
+	now = time.Now()
+	cli, err = factory.NewHelmClient("my-namespace", nil, log.NullLogger{})
+	require.Nil(t, err)
+	require.NotNil(t, cli)
+	require.Equal(t, map[string]int{"my-namespace": 1}, getActionConfigIsCalled)
+	require.True(t, factory.configurationsLastUsedTimes["my-namespace"].After(now))
+	require.True(t, factory.configurationsLastUsedTimes["my-namespace"].Before(time.Now()))
+
+	cli, err = factory.NewHelmClient("another-namespace", nil, log.NullLogger{})
+	require.Nil(t, err)
+	require.NotNil(t, cli)
+	require.Equal(t, map[string]int{"my-namespace": 1, "another-namespace": 1}, getActionConfigIsCalled)
+}
+
+func TestHelmClientFactory_cleanup(t *testing.T) {
+	now := time.Now()
+	factory := &HelmClientFactory{
+		configurations: map[string]*action.Configuration{
+			"namespace-1": {},
+			"namespace-2": {},
+			"namespace-3": {},
+			"namespace-4": {},
+		},
+		configurationsLastUsedTimes: map[string]time.Time{
+			"namespace-1": now.Add(-20 * time.Minute),
+			"namespace-2": now.Add(-5 * time.Minute),
+			"namespace-3": now.Add(-20 * time.Minute),
+			"namespace-4": now.Add(-5 * time.Minute),
+		},
+	}
+
+	factory.lastCleanupTime = now.Add(-1 * time.Minute)
+	factory.cleanup()
+	require.Equal(t, 4, len(factory.configurations))
+	require.Equal(t, 4, len(factory.configurationsLastUsedTimes))
+	require.Equal(t, factory.lastCleanupTime, now.Add(-1*time.Minute))
+
+	factory.lastCleanupTime = now.Add(-16 * time.Minute)
+	factory.cleanup()
+	require.Equal(t, map[string]*action.Configuration{
+		"namespace-2": {},
+		"namespace-4": {},
+	}, factory.configurations)
+	require.Equal(t, map[string]time.Time{
+		"namespace-2": now.Add(-5 * time.Minute),
+		"namespace-4": now.Add(-5 * time.Minute),
+	}, factory.configurationsLastUsedTimes)
+	require.True(t, factory.lastCleanupTime.After(now))
+}