Skip to content

Commit

Permalink
logging: align to Kubernetes structured logging, add reconcileID
Browse files Browse the repository at this point in the history 
Signed-off-by: Stefan Büringer buringerst@vmware.com
  • Loading branch information
@sbueringer
sbueringer committed Mar 4, 2022
1 parent eb292e5 commit 21fcf2e
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 36 deletions.
3 changes: 2 additions & 1 deletion examples/builtins/main.go
View file
Expand Up @@ -22,6 +22,7 @@ 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"
Expand Down Expand Up @@ -50,7 +51,7 @@ func main() {

// Setup a new controller to reconcile ReplicaSets
entryLog.Info("Setting up controller")
c, err := controller.New("foo-controller", mgr, controller.Options{
c, err := controller.New("foo-controller", nil, mgr, controller.Options{
Reconciler: &reconcileReplicaSet{client: mgr.GetClient()},
})
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions pkg/builder/controller.go
View file
Expand Up @@ -308,9 +308,8 @@ func (blder *Builder) doController(r reconcile.Reconciler) error {
if ctrlOptions.Log.GetSink() == nil {
ctrlOptions.Log = blder.mgr.GetLogger()
}
ctrlOptions.Log = ctrlOptions.Log.WithValues("reconciler group", gvk.Group, "reconciler kind", gvk.Kind)

// Build the controller and return.
blder.ctrl, err = newController(blder.getControllerName(gvk), blder.mgr, ctrlOptions)
blder.ctrl, err = newController(blder.getControllerName(gvk), &gvk, blder.mgr, ctrlOptions)
return err
}
24 changes: 12 additions & 12 deletions pkg/builder/controller_test.go
View file
Expand Up @@ -143,7 +143,7 @@ var _ = Describe("application", func() {
})

It("should return an error if it cannot create the controller", func() {
newController = func(name string, mgr manager.Manager, options controller.Options) (
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (
controller.Controller, error) {
return nil, fmt.Errorf("expected error")
}
Expand All @@ -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, mgr manager.Manager, options controller.Options) (
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (
controller.Controller, error) {
if options.MaxConcurrentReconciles == maxConcurrentReconciles {
return controller.New(name, mgr, options)
return controller.New(name, gvk, mgr, options)
}
return nil, fmt.Errorf("max concurrent reconcilers expected %d but found %d", maxConcurrentReconciles, options.MaxConcurrentReconciles)
}
Expand All @@ -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, mgr manager.Manager, options controller.Options) (
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (
controller.Controller, error) {
if options.MaxConcurrentReconciles == maxConcurrentReconciles {
return controller.New(name, mgr, options)
return controller.New(name, gvk, mgr, options)
}
return nil, fmt.Errorf("max concurrent reconcilers expected %d but found %d", maxConcurrentReconciles, options.MaxConcurrentReconciles)
}
Expand All @@ -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, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
if options.RateLimiter == rateLimiter {
return controller.New(name, mgr, options)
return controller.New(name, gvk, mgr, options)
}
return nil, fmt.Errorf("rate limiter expected %T but found %T", rateLimiter, options.RateLimiter)
}
Expand All @@ -237,9 +237,9 @@ var _ = Describe("application", func() {
It("should override logger during creation of controller", func() {

logger := &testLogger{}
newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
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, mgr, options)
return controller.New(name, gvk, mgr, options)
}
return nil, fmt.Errorf("logger expected %T but found %T", logger, options.Log)
}
Expand All @@ -258,11 +258,11 @@ var _ = Describe("application", func() {
})

It("should prefer reconciler from options during creation of controller", func() {
newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
newController = func(name string, gvk *schema.GroupVersionKind, 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, mgr, options)
return controller.New(name, gvk, mgr, options)
}

By("creating a controller manager")
Expand Down
27 changes: 23 additions & 4 deletions pkg/controller/controller.go
View file
Expand Up @@ -18,10 +18,14 @@ package controller

import (
"context"
crand "crypto/rand"
"encoding/binary"
"fmt"
"math/rand"
"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"
Expand Down Expand Up @@ -84,8 +88,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, mgr manager.Manager, options Options) (Controller, error) {
c, err := NewUnmanaged(name, mgr, options)
func New(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options Options) (Controller, error) {
c, err := NewUnmanaged(name, gvk, mgr, options)
if err != nil {
return nil, err
}
Expand All @@ -96,7 +100,7 @@ func New(name string, mgr manager.Manager, options Options) (Controller, error)

// NewUnmanaged returns a new controller without adding it to the manager. The
// caller is responsible for starting the returned controller.
func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller, error) {
func NewUnmanaged(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options Options) (Controller, error) {
if options.Reconciler == nil {
return nil, fmt.Errorf("must specify Reconciler")
}
Expand Down Expand Up @@ -126,6 +130,19 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
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 {
return nil, fmt.Errorf("could not read random bytes to seed random source for reconcileID generation: %v", err)
}
randSource := rand.New(rand.NewSource(rngSeed)) //nolint:gosec // math/rand is enough, we don't need crypto/rand.

// Create controller with dependencies set
return &controller.Controller{
Do: options.Reconciler,
Expand All @@ -136,7 +153,9 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
CacheSyncTimeout: options.CacheSyncTimeout,
SetFields: mgr.SetFields,
Name: name,
Log: options.Log.WithName("controller").WithName(name).WithValues("controller", name),
GVK: gvk,
Log: log,
RandSource: randSource,
RecoverPanic: options.RecoverPanic,
}, nil
}
4 changes: 3 additions & 1 deletion pkg/controller/controller_integration_test.go
View file
Expand Up @@ -24,6 +24,7 @@ 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"
Expand All @@ -33,6 +34,7 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"sigs.k8s.io/controller-runtime/pkg/manager"
)

Expand All @@ -54,7 +56,7 @@ var _ = Describe("controller", func() {
Expect(err).NotTo(HaveOccurred())

By("Creating the Controller")
instance, err := controller.New("foo-controller", cm, controller.Options{
instance, err := controller.New("foo-controller", nil, cm, controller.Options{
Reconciler: reconcile.Func(
func(_ context.Context, request reconcile.Request) (reconcile.Result, error) {
reconciled <- request
Expand Down
14 changes: 7 additions & 7 deletions pkg/controller/controller_test.go
View file
Expand Up @@ -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("", m, controller.Options{Reconciler: rec})
c, err := controller.New("", nil, m, controller.Options{Reconciler: rec})
Expect(c).To(BeNil())
Expect(err.Error()).To(ContainSubstring("must specify Name for Controller"))
})
Expand All @@ -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", m, controller.Options{})
c, err := controller.New("foo", nil, m, controller.Options{})
Expect(c).To(BeNil())
Expect(err.Error()).To(ContainSubstring("must specify Reconciler"))
})
Expand All @@ -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", m, controller.Options{Reconciler: &failRec{}})
c, err := controller.New("foo", nil, m, controller.Options{Reconciler: &failRec{}})
Expect(c).To(BeNil())
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("expected error"))
Expand All @@ -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", m, controller.Options{Reconciler: rec})
c1, err := controller.New("c1", nil, m, controller.Options{Reconciler: rec})
Expect(err).NotTo(HaveOccurred())
Expect(c1).ToNot(BeNil())

c2, err := controller.New("c2", m, controller.Options{Reconciler: rec})
c2, err := controller.New("c2", nil, m, controller.Options{Reconciler: rec})
Expect(err).NotTo(HaveOccurred())
Expect(c2).ToNot(BeNil())
})
Expand Down Expand Up @@ -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", m, controller.Options{Reconciler: rec})
c, err := controller.New("new-controller", nil, m, controller.Options{Reconciler: rec})
Expect(c.Watch(watch, &handler.EnqueueRequestForObject{})).To(Succeed())
Expect(err).NotTo(HaveOccurred())

Expand All @@ -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", m, controller.Options{Reconciler: rec})
_, err = controller.New("new-controller", nil, m, controller.Options{Reconciler: rec})
Expect(err).NotTo(HaveOccurred())

// force-close keep-alive connections. These'll time anyway (after
Expand Down
9 changes: 5 additions & 4 deletions pkg/controller/example_test.go
View file
Expand Up @@ -23,6 +23,7 @@ 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"
Expand All @@ -41,7 +42,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", mgr, controller.Options{
_, err := controller.New("pod-controller", nil, 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
Expand All @@ -59,7 +60,7 @@ func ExampleController() {

// Create a new Controller that will call the provided Reconciler function in response
// to events.
c, err := controller.New("pod-controller", mgr, controller.Options{
c, err := controller.New("pod-controller", nil, 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
Expand Down Expand Up @@ -90,7 +91,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", mgr, controller.Options{
c, err := controller.New("pod-controller", nil, 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
Expand Down Expand Up @@ -129,7 +130,7 @@ func ExampleNewUnmanaged() {

// Configure creates a new controller but does not add it to the supplied
// manager.
c, err := controller.NewUnmanaged("pod-controller", mgr, controller.Options{
c, err := controller.NewUnmanaged("pod-controller", nil, mgr, controller.Options{
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
return reconcile.Result{}, nil
}),
Expand Down

0 comments on commit 21fcf2e

Please sign in to comment.