Merge pull request #1095 from vincepri/logger-manager-controller

✨ Add options to configure a logger for manager and controllers

pkg/builder/controller.go

@@ -20,7 +20,9 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -45,6 +47,7 @@ type Builder struct {
 	config           *rest.Config
 	ctrl             controller.Controller
 	ctrlOptions      controller.Options
+	log              logr.Logger
 	name             string
 }
 
@@ -155,6 +158,12 @@ func (blder *Builder) Named(name string) *Builder {
 	return blder
 }
 
+// WithLogger overrides the controller options's logger used.
+func (blder *Builder) WithLogger(log logr.Logger) *Builder {
+	blder.log = log
+	return blder
+}
+
 // Complete builds the Application ControllerManagedBy.
 func (blder *Builder) Complete(r reconcile.Reconciler) error {
 	_, err := blder.Build(r)
@@ -228,26 +237,33 @@ func (blder *Builder) loadRestConfig() {
 	}
 }
 
-func (blder *Builder) getControllerName() (string, error) {
+func (blder *Builder) getControllerName(gvk schema.GroupVersionKind) string {
 	if blder.name != "" {
-		return blder.name, nil
+		return blder.name
 	}
-	gvk, err := getGvk(blder.forInput.object, blder.mgr.GetScheme())
-	if err != nil {
-		return "", err
-	}
-	return strings.ToLower(gvk.Kind), nil
+	return strings.ToLower(gvk.Kind)
 }
 
 func (blder *Builder) doController(r reconcile.Reconciler) error {
-	name, err := blder.getControllerName()
-	if err != nil {
-		return err
-	}
 	ctrlOptions := blder.ctrlOptions
 	if ctrlOptions.Reconciler == nil {
 		ctrlOptions.Reconciler = r
 	}
-	blder.ctrl, err = newController(name, blder.mgr, ctrlOptions)
+
+	// Retrieve the GVK from the object we're reconciling
+	// to prepopulate logger information, and to optionally generate a default name.
+	gvk, err := getGvk(blder.forInput.object, blder.mgr.GetScheme())
+	if err != nil {
+		return err
+	}
+
+	// Setup the logger.
+	if ctrlOptions.Log == nil {
+		ctrlOptions.Log = blder.mgr.GetLogger()
+	}
+	ctrlOptions.Log = ctrlOptions.Log.WithValues("reconcilerGroup", gvk.Group, "reconcilerKind", gvk.Kind)
+
+	// Build the controller and return.
+	blder.ctrl, err = newController(blder.getControllerName(gvk), blder.mgr, ctrlOptions)
 	return err
 }

pkg/controller/controller.go

@@ -19,10 +19,10 @@ package controller
 import (
 	"fmt"
 
+	"github.com/go-logr/logr"
 	"k8s.io/client-go/util/workqueue"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/internal/controller"
-	"sigs.k8s.io/controller-runtime/pkg/internal/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/ratelimiter"
@@ -42,6 +42,9 @@ type Options struct {
 	// Defaults to MaxOfRateLimiter which has both overall and per-item rate limiting.
 	// The overall is a token bucket and the per-item is exponential.
 	RateLimiter ratelimiter.RateLimiter
+
+	// Log is the logger used for this controller.
+	Log logr.Logger
 }
 
 // Controller implements a Kubernetes API.  A Controller manages a work queue fed reconcile.Requests
@@ -96,22 +99,24 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
 		options.RateLimiter = workqueue.DefaultControllerRateLimiter()
 	}
 
+	if options.Log == nil {
+		options.Log = mgr.GetLogger()
+	}
+
 	// Inject dependencies into Reconciler
 	if err := mgr.SetFields(options.Reconciler); err != nil {
 		return nil, err
 	}
 
 	// Create controller with dependencies set
-	c := &controller.Controller{
+	return &controller.Controller{
 		Do: options.Reconciler,
 		MakeQueue: func() workqueue.RateLimitingInterface {
 			return workqueue.NewNamedRateLimitingQueue(options.RateLimiter, name)
 		},
 		MaxConcurrentReconciles: options.MaxConcurrentReconciles,
 		SetFields:               mgr.SetFields,
 		Name:                    name,
-		Log:                     log.RuntimeLog.WithName("controller").WithValues("controller", name),
-	}
-
-	return c, nil
+		Log:                     options.Log.WithName("controller").WithValues("controller", name),
+	}, nil
 }

pkg/internal/controller/controller.go

@@ -228,11 +228,13 @@ func (c *Controller) reconcileHandler(obj interface{}) bool {
 		return true
 	}
 
+	log := c.Log.WithValues("name", req.Name, "namespace", req.Namespace)
+
 	// RunInformersAndControllers the syncHandler, passing it the namespace/Name string of the
 	// resource to be synced.
 	if result, err := c.Do.Reconcile(req); err != nil {
 		c.Queue.AddRateLimited(req)
-		c.Log.Error(err, "Reconciler error", "name", req.Name, "namespace", req.Namespace)
+		log.Error(err, "Reconciler error")
 		ctrlmetrics.ReconcileErrors.WithLabelValues(c.Name).Inc()
 		ctrlmetrics.ReconcileTotal.WithLabelValues(c.Name, "error").Inc()
 		return false
@@ -256,7 +258,7 @@ func (c *Controller) reconcileHandler(obj interface{}) bool {
 	c.Queue.Forget(obj)
 
 	// TODO(directxman12): What does 1 mean?  Do we want level constants?  Do we want levels at all?
-	c.Log.V(1).Info("Successfully Reconciled", "name", req.Name, "namespace", req.Namespace)
+	log.V(1).Info("Successfully Reconciled")
 
 	ctrlmetrics.ReconcileTotal.WithLabelValues(c.Name, "success").Inc()
 	// Return true, don't take a break

pkg/manager/internal.go

@@ -25,6 +25,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -133,6 +134,10 @@ type controllerManager struct {
 	// It and `internalStop` should point to the same channel.
 	internalStopper chan<- struct{}
 
+	// Logger is the logger that should be used by this manager.
+	// If none is set, it defaults to log.Log global logger.
+	logger logr.Logger
+
 	// leaderElectionCancel is used to cancel the leader election. It is distinct from internalStopper,
 	// because for safety reasons we need to os.Exit() when we lose the leader election, meaning that
 	// it must be deferred until after gracefulShutdown is done.
@@ -357,6 +362,10 @@ func (cm *controllerManager) GetWebhookServer() *webhook.Server {
 	return cm.webhookServer
 }
 
+func (cm *controllerManager) GetLogger() logr.Logger {
+	return cm.logger
+}
+
 func (cm *controllerManager) serveMetrics(stop <-chan struct{}) {
 	handler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{
 		ErrorHandling: promhttp.HTTPErrorOnError,

pkg/manager/manager.go

@@ -36,6 +36,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	internalrecorder "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
 	"sigs.k8s.io/controller-runtime/pkg/leaderelection"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 	"sigs.k8s.io/controller-runtime/pkg/recorder"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -111,6 +112,9 @@ type Manager interface {
 
 	// GetWebhookServer returns a webhook.Server
 	GetWebhookServer() *webhook.Server
+
+	// GetLogger returns this manager's logger.
+	GetLogger() logr.Logger
 }
 
 // Options are the arguments for creating a new Manager
@@ -131,6 +135,10 @@ type Options struct {
 	// so that all controllers will not send list requests simultaneously.
 	SyncPeriod *time.Duration
 
+	// Logger is the logger that should be used by this manager.
+	// If none is set, it defaults to log.Log global logger.
+	Logger logr.Logger
+
 	// LeaderElection determines whether or not to use leader election when
 	// starting the manager.
 	LeaderElection bool
@@ -345,6 +353,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		mapper:                  mapper,
 		metricsListener:         metricsListener,
 		metricsExtraHandlers:    metricsExtraHandlers,
+		logger:                  options.Logger,
 		internalStop:            stop,
 		internalStopper:         stop,
 		elected:                 make(chan struct{}),
@@ -462,5 +471,9 @@ func setOptionsDefaults(options Options) Options {
 		options.GracefulShutdownTimeout = &gracefulShutdownTimeout
 	}
 
+	if options.Logger == nil {
+		options.Logger = logf.Log
+	}
+
 	return options
 }