Skip to content

✨ Add options to configure a logger for manager and controllers #1095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 28 additions & 12 deletions pkg/builder/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -45,6 +47,7 @@ type Builder struct {
config *rest.Config
ctrl controller.Controller
ctrlOptions controller.Options
log logr.Logger
name string
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
17 changes: 11 additions & 6 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
}
6 changes: 4 additions & 2 deletions pkg/internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions pkg/manager/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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{}),
Expand Down Expand Up @@ -462,5 +471,9 @@ func setOptionsDefaults(options Options) Options {
options.GracefulShutdownTimeout = &gracefulShutdownTimeout
}

if options.Logger == nil {
options.Logger = logf.Log
}

return options
}