From 466549a74f6e6787f25ae1787c2303bdc8f970e7 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Tue, 8 Mar 2022 08:53:51 +0100 Subject: [PATCH] update --- examples/builtins/main.go | 3 +- go.mod | 2 +- pkg/builder/controller.go | 29 ++++++-- pkg/builder/controller_test.go | 32 +++++---- pkg/controller/controller.go | 28 +++----- pkg/controller/controller_integration_test.go | 4 +- pkg/controller/controller_test.go | 14 ++-- pkg/controller/example_test.go | 9 ++- pkg/internal/controller/controller.go | 71 ++++--------------- pkg/internal/controller/controller_test.go | 8 ++- .../recorder/recorder_integration_test.go | 3 +- 11 files changed, 84 insertions(+), 119 deletions(-) diff --git a/examples/builtins/main.go b/examples/builtins/main.go index 78465f7f5a..ff1f0dfa3b 100644 --- a/examples/builtins/main.go +++ b/examples/builtins/main.go @@ -22,7 +22,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" - "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -51,7 +50,7 @@ func main() { // Setup a new controller to reconcile ReplicaSets entryLog.Info("Setting up controller") - c, err := controller.New("foo-controller", nil, mgr, controller.Options{ + c, err := controller.New("foo-controller", mgr, controller.Options{ Reconciler: &reconcileReplicaSet{client: mgr.GetClient()}, }) if err != nil { diff --git a/go.mod b/go.mod index e809f71295..7629bfbf0f 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( k8s.io/apimachinery v0.23.0 k8s.io/client-go v0.23.0 k8s.io/component-base v0.23.0 + k8s.io/klog/v2 v2.30.0 k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b sigs.k8s.io/yaml v1.3.0 ) @@ -59,7 +60,6 @@ require ( gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect - k8s.io/klog/v2 v2.30.0 // indirect k8s.io/kube-openapi v0.0.0-20211115234752-e816edb12b65 // indirect sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.0 // indirect diff --git a/pkg/builder/controller.go b/pkg/builder/controller.go index 5a1f7546aa..6ba801829d 100644 --- a/pkg/builder/controller.go +++ b/pkg/builder/controller.go @@ -23,6 +23,7 @@ import ( "github.com/go-logr/logr" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" @@ -148,9 +149,9 @@ func (blder *Builder) WithOptions(options controller.Options) *Builder { return blder } -// WithLogger overrides the controller options's logger used. -func (blder *Builder) WithLogger(log logr.Logger) *Builder { - blder.ctrlOptions.Log = log +// WithLogConstructor overrides the controller options's LogConstructor. +func (blder *Builder) WithLogConstructor(logConstructor func(*reconcile.Request) logr.Logger) *Builder { + blder.ctrlOptions.LogConstructor = logConstructor return blder } @@ -289,6 +290,9 @@ func (blder *Builder) doController(r reconcile.Reconciler) error { if err != nil { return err } + if gvk.Kind == "" { + return fmt.Errorf("kind should not be empty") + } // Setup concurrency. if ctrlOptions.MaxConcurrentReconciles == 0 { @@ -304,12 +308,25 @@ func (blder *Builder) doController(r reconcile.Reconciler) error { ctrlOptions.CacheSyncTimeout = *globalOpts.CacheSyncTimeout } + controllerName := blder.getControllerName(gvk) + // Setup the logger. - if ctrlOptions.Log.GetSink() == nil { - ctrlOptions.Log = blder.mgr.GetLogger() + if ctrlOptions.LogConstructor == nil { + log := blder.mgr.GetLogger() + log = log.WithValues("reconcilerGroup", gvk.Group, "reconcilerKind", gvk.Kind) + log = log.WithValues("controller", controllerName) + + lowerCamelCaseKind := strings.ToLower(gvk.Kind[:1]) + gvk.Kind[1:] + + ctrlOptions.LogConstructor = func(req *reconcile.Request) logr.Logger { + if req != nil { + log = log.WithValues(lowerCamelCaseKind, klog.KRef(req.Namespace, req.Name)) + } + return log + } } // Build the controller and return. - blder.ctrl, err = newController(blder.getControllerName(gvk), &gvk, blder.mgr, ctrlOptions) + blder.ctrl, err = newController(controllerName, blder.mgr, ctrlOptions) return err } diff --git a/pkg/builder/controller_test.go b/pkg/builder/controller_test.go index 9105f60a73..56c1a41458 100644 --- a/pkg/builder/controller_test.go +++ b/pkg/builder/controller_test.go @@ -143,7 +143,7 @@ var _ = Describe("application", func() { }) It("should return an error if it cannot create the controller", func() { - newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) ( + newController = func(name string, mgr manager.Manager, options controller.Options) ( controller.Controller, error) { return nil, fmt.Errorf("expected error") } @@ -163,10 +163,10 @@ var _ = Describe("application", func() { It("should override max concurrent reconcilers during creation of controller", func() { const maxConcurrentReconciles = 5 - newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) ( + newController = func(name string, mgr manager.Manager, options controller.Options) ( controller.Controller, error) { if options.MaxConcurrentReconciles == maxConcurrentReconciles { - return controller.New(name, gvk, mgr, options) + return controller.New(name, mgr, options) } return nil, fmt.Errorf("max concurrent reconcilers expected %d but found %d", maxConcurrentReconciles, options.MaxConcurrentReconciles) } @@ -186,10 +186,10 @@ var _ = Describe("application", func() { It("should override max concurrent reconcilers during creation of controller, when using", func() { const maxConcurrentReconciles = 10 - newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) ( + newController = func(name string, mgr manager.Manager, options controller.Options) ( controller.Controller, error) { if options.MaxConcurrentReconciles == maxConcurrentReconciles { - return controller.New(name, gvk, mgr, options) + return controller.New(name, mgr, options) } return nil, fmt.Errorf("max concurrent reconcilers expected %d but found %d", maxConcurrentReconciles, options.MaxConcurrentReconciles) } @@ -214,9 +214,9 @@ var _ = Describe("application", func() { It("should override rate limiter during creation of controller", func() { rateLimiter := workqueue.DefaultItemBasedRateLimiter() - newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (controller.Controller, error) { + newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) { if options.RateLimiter == rateLimiter { - return controller.New(name, gvk, mgr, options) + return controller.New(name, mgr, options) } return nil, fmt.Errorf("rate limiter expected %T but found %T", rateLimiter, options.RateLimiter) } @@ -237,11 +237,11 @@ var _ = Describe("application", func() { It("should override logger during creation of controller", func() { logger := &testLogger{} - newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (controller.Controller, error) { - if options.Log.GetSink() == logger { - return controller.New(name, gvk, mgr, options) + newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) { + if options.LogConstructor(nil).GetSink() == logger { + return controller.New(name, mgr, options) } - return nil, fmt.Errorf("logger expected %T but found %T", logger, options.Log) + return nil, fmt.Errorf("logger expected %T but found %T", logger, options.LogConstructor) } By("creating a controller manager") @@ -251,18 +251,20 @@ var _ = Describe("application", func() { instance, err := ControllerManagedBy(m). For(&appsv1.ReplicaSet{}). Owns(&appsv1.ReplicaSet{}). - WithLogger(logr.New(logger)). + WithLogConstructor(func(request *reconcile.Request) logr.Logger { + return logr.New(logger) + }). Build(noop) Expect(err).NotTo(HaveOccurred()) Expect(instance).NotTo(BeNil()) }) It("should prefer reconciler from options during creation of controller", func() { - newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (controller.Controller, error) { + newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) { if options.Reconciler != (typedNoop{}) { - return nil, fmt.Errorf("custom reconciler expected %T but found %T", typedNoop{}, options.Reconciler) + return nil, fmt.Errorf("Custom reconciler expected %T but found %T", typedNoop{}, options.Reconciler) } - return controller.New(name, gvk, mgr, options) + return controller.New(name, mgr, options) } By("creating a controller manager") diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 7badda68e5..3b6b80617d 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -25,7 +25,6 @@ import ( "time" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -50,9 +49,9 @@ type Options struct { // The overall is a token bucket and the per-item is exponential. RateLimiter ratelimiter.RateLimiter - // Log is the logger used for this controller and passed to each reconciliation - // request via the context field. - Log logr.Logger + // LogConstructor is used to construct a logger used for this controller and passed + // to each reconciliation via the context field. + LogConstructor func(request *reconcile.Request) logr.Logger // CacheSyncTimeout refers to the time limit set to wait for syncing caches. // Defaults to 2 minutes if not set. @@ -88,8 +87,8 @@ type Controller interface { // New returns a new Controller registered with the Manager. The Manager will ensure that shared Caches have // been synced before the Controller is Started. -func New(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options Options) (Controller, error) { - c, err := NewUnmanaged(name, gvk, mgr, options) +func New(name string, mgr manager.Manager, options Options) (Controller, error) { + c, err := NewUnmanaged(name, mgr, options) if err != nil { return nil, err } @@ -100,7 +99,7 @@ func New(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options // NewUnmanaged returns a new controller without adding it to the manager. The // caller is responsible for starting the returned controller. -func NewUnmanaged(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options Options) (Controller, error) { +func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller, error) { if options.Reconciler == nil { return nil, fmt.Errorf("must specify Reconciler") } @@ -109,8 +108,10 @@ func NewUnmanaged(name string, gvk *schema.GroupVersionKind, mgr manager.Manager return nil, fmt.Errorf("must specify Name for Controller") } - if options.Log.GetSink() == nil { - options.Log = mgr.GetLogger() + if options.LogConstructor == nil { + options.LogConstructor = func(request *reconcile.Request) logr.Logger { + return mgr.GetLogger() + } } if options.MaxConcurrentReconciles <= 0 { @@ -130,12 +131,6 @@ func NewUnmanaged(name string, gvk *schema.GroupVersionKind, mgr manager.Manager return nil, err } - // Add controller and reconciler group / kind to logger. - log := options.Log.WithValues("controller", name) - if gvk != nil { - log = log.WithValues("reconciler group", gvk.Group, "reconciler kind", gvk.Kind) - } - // Initialize random source, later used to generate reconcileIDs. var rngSeed int64 if err := binary.Read(crand.Reader, binary.LittleEndian, &rngSeed); err != nil { @@ -153,8 +148,7 @@ func NewUnmanaged(name string, gvk *schema.GroupVersionKind, mgr manager.Manager CacheSyncTimeout: options.CacheSyncTimeout, SetFields: mgr.SetFields, Name: name, - GroupVersionKind: gvk, - Log: log, + LogConstructor: options.LogConstructor, RandSource: randSource, RecoverPanic: options.RecoverPanic, }, nil diff --git a/pkg/controller/controller_integration_test.go b/pkg/controller/controller_integration_test.go index 886137328c..9f347b0032 100644 --- a/pkg/controller/controller_integration_test.go +++ b/pkg/controller/controller_integration_test.go @@ -24,7 +24,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllertest" @@ -34,7 +33,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/manager" ) @@ -56,7 +54,7 @@ var _ = Describe("controller", func() { Expect(err).NotTo(HaveOccurred()) By("Creating the Controller") - instance, err := controller.New("foo-controller", nil, cm, controller.Options{ + instance, err := controller.New("foo-controller", cm, controller.Options{ Reconciler: reconcile.Func( func(_ context.Context, request reconcile.Request) (reconcile.Result, error) { reconciled <- request diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index ce70c282bf..d3e8419a16 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -45,7 +45,7 @@ var _ = Describe("controller.Controller", func() { It("should return an error if Name is not Specified", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - c, err := controller.New("", nil, m, controller.Options{Reconciler: rec}) + c, err := controller.New("", m, controller.Options{Reconciler: rec}) Expect(c).To(BeNil()) Expect(err.Error()).To(ContainSubstring("must specify Name for Controller")) }) @@ -54,7 +54,7 @@ var _ = Describe("controller.Controller", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - c, err := controller.New("foo", nil, m, controller.Options{}) + c, err := controller.New("foo", m, controller.Options{}) Expect(c).To(BeNil()) Expect(err.Error()).To(ContainSubstring("must specify Reconciler")) }) @@ -63,7 +63,7 @@ var _ = Describe("controller.Controller", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - c, err := controller.New("foo", nil, m, controller.Options{Reconciler: &failRec{}}) + c, err := controller.New("foo", m, controller.Options{Reconciler: &failRec{}}) Expect(c).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) @@ -73,11 +73,11 @@ var _ = Describe("controller.Controller", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - c1, err := controller.New("c1", nil, m, controller.Options{Reconciler: rec}) + c1, err := controller.New("c1", m, controller.Options{Reconciler: rec}) Expect(err).NotTo(HaveOccurred()) Expect(c1).ToNot(BeNil()) - c2, err := controller.New("c2", nil, m, controller.Options{Reconciler: rec}) + c2, err := controller.New("c2", m, controller.Options{Reconciler: rec}) Expect(err).NotTo(HaveOccurred()) Expect(c2).ToNot(BeNil()) }) @@ -107,7 +107,7 @@ var _ = Describe("controller.Controller", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - c, err := controller.New("new-controller", nil, m, controller.Options{Reconciler: rec}) + c, err := controller.New("new-controller", m, controller.Options{Reconciler: rec}) Expect(c.Watch(watch, &handler.EnqueueRequestForObject{})).To(Succeed()) Expect(err).NotTo(HaveOccurred()) @@ -134,7 +134,7 @@ var _ = Describe("controller.Controller", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - _, err = controller.New("new-controller", nil, m, controller.Options{Reconciler: rec}) + _, err = controller.New("new-controller", m, controller.Options{Reconciler: rec}) Expect(err).NotTo(HaveOccurred()) // force-close keep-alive connections. These'll time anyway (after diff --git a/pkg/controller/example_test.go b/pkg/controller/example_test.go index cf8a540fef..3d8e399703 100644 --- a/pkg/controller/example_test.go +++ b/pkg/controller/example_test.go @@ -23,7 +23,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -42,7 +41,7 @@ var ( // This example creates a new Controller named "pod-controller" with a no-op reconcile function. The // manager.Manager will be used to Start the Controller, and will provide it a shared Cache and Client. func ExampleNew() { - _, err := controller.New("pod-controller", nil, mgr, controller.Options{ + _, err := controller.New("pod-controller", mgr, controller.Options{ Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) { // Your business logic to implement the API by creating, updating, deleting objects goes here. return reconcile.Result{}, nil @@ -60,7 +59,7 @@ func ExampleController() { // Create a new Controller that will call the provided Reconciler function in response // to events. - c, err := controller.New("pod-controller", nil, mgr, controller.Options{ + c, err := controller.New("pod-controller", mgr, controller.Options{ Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) { // Your business logic to implement the API by creating, updating, deleting objects goes here. return reconcile.Result{}, nil @@ -91,7 +90,7 @@ func ExampleController_unstructured() { // Create a new Controller that will call the provided Reconciler function in response // to events. - c, err := controller.New("pod-controller", nil, mgr, controller.Options{ + c, err := controller.New("pod-controller", mgr, controller.Options{ Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) { // Your business logic to implement the API by creating, updating, deleting objects goes here. return reconcile.Result{}, nil @@ -130,7 +129,7 @@ func ExampleNewUnmanaged() { // Configure creates a new controller but does not add it to the supplied // manager. - c, err := controller.NewUnmanaged("pod-controller", nil, mgr, controller.Options{ + c, err := controller.NewUnmanaged("pod-controller", mgr, controller.Options{ Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) { return reconcile.Result{}, nil }), diff --git a/pkg/internal/controller/controller.go b/pkg/internal/controller/controller.go index fb7a20672b..b727e68367 100644 --- a/pkg/internal/controller/controller.go +++ b/pkg/internal/controller/controller.go @@ -22,15 +22,12 @@ import ( "errors" "fmt" "math/rand" - "strings" "sync" "time" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/util/workqueue" - "sigs.k8s.io/controller-runtime/pkg/handler" ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -88,16 +85,13 @@ type Controller struct { // startWatches maintains a list of sources, handlers, and predicates to start when the controller is started. startWatches []watchDescription - // Log is used to log messages to users during reconciliation, or for example when a watch is started. - Log logr.Logger + // LogConstructor is used to construct a logger to then log messages to users during reconciliation, + // or for example when a watch is started. + LogConstructor func(request *reconcile.Request) logr.Logger // RandSource is used to generate reconcileIDs for logging. RandSource *rand.Rand - // GVK is used to create the log key for the object. - // If not set, "obj" is used instead. - GroupVersionKind *schema.GroupVersionKind - // RecoverPanic indicates whether the panic caused by reconcile should be recovered. RecoverPanic bool } @@ -155,7 +149,7 @@ func (c *Controller) Watch(src source.Source, evthdler handler.EventHandler, prc return nil } - c.Log.Info("Starting EventSource", "source", src) + c.LogConstructor(nil).Info("Starting EventSource", "source", src) return src.Start(c.ctx, evthdler, c.Queue, prct...) } @@ -190,7 +184,7 @@ func (c *Controller) Start(ctx context.Context) error { // caches to sync so that they have a chance to register their intendeded // caches. for _, watch := range c.startWatches { - c.Log.Info("Starting EventSource", "source", fmt.Sprintf("%s", watch.src)) + c.LogConstructor(nil).Info("Starting EventSource", "source", fmt.Sprintf("%s", watch.src)) if err := watch.src.Start(ctx, watch.handler, c.Queue, watch.predicates...); err != nil { return err @@ -198,7 +192,7 @@ func (c *Controller) Start(ctx context.Context) error { } // Start the SharedIndexInformer factories to begin populating the SharedIndexInformer caches - c.Log.Info("Starting Controller") + c.LogConstructor(nil).Info("Starting Controller") for _, watch := range c.startWatches { syncingSource, ok := watch.src.(source.SyncingSource) @@ -215,7 +209,7 @@ func (c *Controller) Start(ctx context.Context) error { // is an error or a timeout if err := syncingSource.WaitForSync(sourceStartCtx); err != nil { err := fmt.Errorf("failed to wait for %s caches to sync: %w", c.Name, err) - c.Log.Error(err, "Could not wait for Cache to sync") + c.LogConstructor(nil).Error(err, "Could not wait for Cache to sync") return err } @@ -232,7 +226,7 @@ func (c *Controller) Start(ctx context.Context) error { c.startWatches = nil // Launch workers to process resources - c.Log.Info("Starting workers", "worker count", c.MaxConcurrentReconciles) + c.LogConstructor(nil).Info("Starting workers", "worker count", c.MaxConcurrentReconciles) wg.Add(c.MaxConcurrentReconciles) for i := 0; i < c.MaxConcurrentReconciles; i++ { go func() { @@ -252,9 +246,9 @@ func (c *Controller) Start(ctx context.Context) error { } <-ctx.Done() - c.Log.Info("Shutdown signal received, waiting for all workers to finish") + c.LogConstructor(nil).Info("Shutdown signal received, waiting for all workers to finish") wg.Wait() - c.Log.Info("All workers finished") + c.LogConstructor(nil).Info("All workers finished") return nil } @@ -313,17 +307,12 @@ func (c *Controller) reconcileHandler(ctx context.Context, obj interface{}) { // Forget here else we'd go into a loop of attempting to // process a work item that is invalid. c.Queue.Forget(obj) - c.Log.Error(nil, "Queue item was not a Request", "type", fmt.Sprintf("%T", obj), "value", obj) + c.LogConstructor(nil).Error(nil, "Queue item was not a Request", "type", fmt.Sprintf("%T", obj), "value", obj) // Return true, don't take a break return } - // Add object to the logger. - var objectLogKey = "obj" - if c.GroupVersionKind != nil { - objectLogKey = strings.ToLower(c.GroupVersionKind.Kind) - } - log := c.Log.WithValues(objectLogKey, KRef(req.Namespace, req.Name)) + log := c.LogConstructor(&req) // Add reconcileID to the logger. reconcileID, err := c.generateReconcileID() @@ -368,7 +357,7 @@ func (c *Controller) reconcileHandler(ctx context.Context, obj interface{}) { // GetLogger returns this controller's logger. func (c *Controller) GetLogger() logr.Logger { - return c.Log + return c.LogConstructor(nil) } // InjectFunc implement SetFields.Injector. @@ -382,40 +371,6 @@ func (c *Controller) updateMetrics(reconcileTime time.Duration) { ctrlmetrics.ReconcileTime.WithLabelValues(c.Name).Observe(reconcileTime.Seconds()) } -// KRef returns ObjectRef from name and namespace -// Note: This is a copy of the func from klog. It has been copied to avoid -// introducing a dependency to klog, while still implement logging according -// to the Kubernetes structured logging KEP. -func KRef(namespace, name string) ObjectRef { - return ObjectRef{ - Name: name, - Namespace: namespace, - } -} - -// ObjectRef references a kubernetes object -// Note: This is a copy of the struct from klog. It has been copied to avoid -// introducing a dependency to klog, while still implement logging according -// to the Kubernetes structured logging KEP. -type ObjectRef struct { - Name string `json:"name"` - Namespace string `json:"namespace,omitempty"` -} - -// MarshalLog ensures that loggers with support for structured output will log -// as a struct by removing the String method via a custom type. -func (ref ObjectRef) MarshalLog() interface{} { - type or ObjectRef - return or(ref) -} - -func (ref ObjectRef) String() string { - if ref.Namespace != "" { - return fmt.Sprintf("%s/%s", ref.Namespace, ref.Name) - } - return ref.Name -} - // generateReconcileID generates a reconcileID for logging. func (c *Controller) generateReconcileID() (string, error) { id := [16]byte{} diff --git a/pkg/internal/controller/controller_test.go b/pkg/internal/controller/controller_test.go index 0d15d0f1ed..e15a9d3b1c 100644 --- a/pkg/internal/controller/controller_test.go +++ b/pkg/internal/controller/controller_test.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "github.com/go-logr/logr" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/prometheus/client_golang/prometheus" @@ -33,7 +34,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" - "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/cache/informertest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -72,8 +72,10 @@ var _ = Describe("controller", func() { MaxConcurrentReconciles: 1, Do: fakeReconcile, MakeQueue: func() workqueue.RateLimitingInterface { return queue }, - Log: log.RuntimeLog.WithName("controller").WithName("test"), - RandSource: rand.New(rand.NewSource(0)), //nolint:gosec // math/rand is enough, we don't need crypto/rand. + LogConstructor: func(_ *reconcile.Request) logr.Logger { + return log.RuntimeLog.WithName("controller").WithName("test") + }, + RandSource: rand.New(rand.NewSource(0)), //nolint:gosec // math/rand is enough, we don't need crypto/rand. } Expect(ctrl.InjectFunc(func(interface{}) error { return nil })).To(Succeed()) }) diff --git a/pkg/internal/recorder/recorder_integration_test.go b/pkg/internal/recorder/recorder_integration_test.go index a1e7bd6570..5bafaabf5a 100644 --- a/pkg/internal/recorder/recorder_integration_test.go +++ b/pkg/internal/recorder/recorder_integration_test.go @@ -25,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/scheme" ref "k8s.io/client-go/tools/reference" - "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -45,7 +44,7 @@ var _ = Describe("recorder", func() { By("Creating the Controller") recorder := cm.GetEventRecorderFor("test-recorder") - instance, err := controller.New("foo-controller", nil, cm, controller.Options{ + instance, err := controller.New("foo-controller", cm, controller.Options{ Reconciler: reconcile.Func( func(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { dp, err := clientset.AppsV1().Deployments(request.Namespace).Get(ctx, request.Name, metav1.GetOptions{})