Merge pull request #371 from justinsb/reuse_http_client

Optimization: Reuse HTTPClient / DynamicClient in reconcile

commonclient/factory_cr11.go

@@ -4,10 +4,13 @@ package commonclient
 
 import (
 	"context"
+	"net/http"
 
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/util/workqueue"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/cluster"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/source"
@@ -23,6 +26,12 @@ func WrapEventHandler(h EventHandler) handler.EventHandler {
 	return &eventHandlerWithoutContext{h: h}
 }
 
+// GetHTTPClient returns the http.Client associated with the Cluster
+func GetHTTPClient(c cluster.Cluster) (*http.Client, error) {
+	// This is a "polyfill", later versions of controller-runtime do it better.
+	return rest.HTTPClientFor(c.GetConfig())
+}
+
 type eventHandlerWithoutContext struct {
 	h EventHandler
 }

commonclient/factory_cr15.go

@@ -3,8 +3,11 @@
 package commonclient
 
 import (
+	"net/http"
+
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/cluster"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
@@ -19,4 +22,9 @@ func WrapEventHandler(h handler.EventHandler) handler.EventHandler {
 	return h
 }
 
+// GetHTTPClient returns the http.Client associated with the Cluster
+func GetHTTPClient(c cluster.Cluster) (*http.Client, error) {
+	return c.GetHTTPClient(), nil
+}
+
 type EventHandler = handler.EventHandler

pkg/patterns/declarative/pkg/applier/direct.go

@@ -88,10 +88,13 @@ func (d *DirectApplier) Apply(ctx context.Context, opt ApplierOptions) error {
 	b := d.inner.NewBuilder(opt)
 	f := d.inner.NewFactory(opt)
 
-	// TODO can we just reuse this
-	dynamicClient, err := f.DynamicClient()
-	if err != nil {
-		return err
+	dynamicClient := opt.DynamicClient
+	if dynamicClient == nil {
+		dc, err := f.DynamicClient()
+		if err != nil {
+			return err
+		}
+		dynamicClient = dc
 	}
 
 	if opt.Validate {

pkg/patterns/declarative/pkg/applier/type.go

@@ -43,5 +43,6 @@ type ApplierOptions struct {
 
 	// DynamicClient, if set, will be used for applying additional objects.
 	// If not set, a dynamic client will be built from RESTConfig.
+	// If the caller can provide a cached DynamicClient, that is more efficient.
 	DynamicClient dynamic.Interface
 }

pkg/patterns/declarative/reconciler.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"path/filepath"
 	"reflect"
 	"strings"
@@ -41,6 +42,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 
+	"sigs.k8s.io/kubebuilder-declarative-pattern/commonclient"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/utils"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/kustomize"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/applier"
@@ -51,18 +53,20 @@ var _ reconcile.Reconciler = &Reconciler{}
 
 type Reconciler struct {
 	prototype DeclarativeObject
-	client    client.Client
-	config    *rest.Config
+
+	client        client.Client
+	restConfig    *rest.Config
+	httpClient    *http.Client
+	dynamicClient dynamic.Interface
+	restMapper    meta.RESTMapper
 
 	metrics reconcileMetrics
 	mgr     manager.Manager
 
 	// recorder is the EventRecorder for creating k8s events
-	recorder      recorder.EventRecorder
-	dynamicClient dynamic.Interface
+	recorder recorder.EventRecorder
 
-	restMapper meta.RESTMapper
-	options    reconcilerParams
+	options reconcilerParams
 }
 
 type DeclarativeObject interface {
@@ -105,11 +109,17 @@ func (r *Reconciler) Init(mgr manager.Manager, prototype DeclarativeObject, opts
 	r.recorder = mgr.GetEventRecorderFor(controllerName)
 
 	r.client = mgr.GetClient()
-	r.config = mgr.GetConfig()
+	r.restConfig = mgr.GetConfig()
+	httpClient, err := commonclient.GetHTTPClient(mgr)
+	if err != nil {
+		return fmt.Errorf("getting HTTP client: %w", err)
+	}
+	r.httpClient = httpClient
+
 	r.mgr = mgr
 	globalObjectTracker.mgr = mgr
 
-	d, err := dynamic.NewForConfig(r.config)
+	d, err := dynamic.NewForConfigAndClient(r.restConfig, r.httpClient)
 	if err != nil {
 		return err
 	}
@@ -379,7 +389,7 @@ func (r *Reconciler) reconcileExists(ctx context.Context, name types.NamespacedN
 		return statusInfo, fmt.Errorf("building applyset parent: %w", err)
 	}
 	applierOpt := applier.ApplierOptions{
-		RESTConfig:        r.config,
+		RESTConfig:        r.restConfig,
 		RESTMapper:        r.restMapper,
 		Namespace:         ns,
 		ParentRef:         parentRef,
@@ -389,6 +399,7 @@ func (r *Reconciler) reconcileExists(ctx context.Context, name types.NamespacedN
 		Force:             true,
 		CascadingStrategy: r.options.cascadingStrategy,
 		Client:            r.client,
+		DynamicClient:     r.dynamicClient,
 	}
 
 	applyOperation := &ApplyOperation{