Skip to content

Commit 1de701a

Browse files
sbueringersbueringer
committed
logging: align to Kubernetes structured logging, add reconcileID
Signed-off-by: Stefan Büringer [email protected]
1 parent eb292e5 commit 1de701a

File tree

10 files changed

+133
-36
lines changed

10 files changed

+133
-36
lines changed

‎examples/builtins/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
appsv1 "k8s.io/api/apps/v1"
2323
corev1 "k8s.io/api/core/v1"
2424
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
25+
2526
"sigs.k8s.io/controller-runtime/pkg/client/config"
2627
"sigs.k8s.io/controller-runtime/pkg/controller"
2728
"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -50,7 +51,7 @@ func main() {
5051

5152
// Setup a new controller to reconcile ReplicaSets
5253
entryLog.Info("Setting up controller")
53-
c, err := controller.New("foo-controller", mgr, controller.Options{
54+
c, err := controller.New("foo-controller", nil, mgr, controller.Options{
5455
Reconciler: &reconcileReplicaSet{client: mgr.GetClient()},
5556
})
5657
if err != nil {

‎pkg/builder/controller.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,8 @@ func (blder *Builder) doController(r reconcile.Reconciler) error {
308308
if ctrlOptions.Log.GetSink() == nil {
309309
ctrlOptions.Log = blder.mgr.GetLogger()
310310
}
311-
ctrlOptions.Log = ctrlOptions.Log.WithValues("reconciler group", gvk.Group, "reconciler kind", gvk.Kind)
312311

313312
// Build the controller and return.
314-
blder.ctrl, err = newController(blder.getControllerName(gvk), blder.mgr, ctrlOptions)
313+
blder.ctrl, err = newController(blder.getControllerName(gvk), &gvk, blder.mgr, ctrlOptions)
315314
return err
316315
}

‎pkg/builder/controller_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ var _ = Describe("application", func() {
143143
})
144144

145145
It("should return an error if it cannot create the controller", func() {
146-
newController = func(name string, mgr manager.Manager, options controller.Options) (
146+
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (
147147
controller.Controller, error) {
148148
return nil, fmt.Errorf("expected error")
149149
}
@@ -163,10 +163,10 @@ var _ = Describe("application", func() {
163163

164164
It("should override max concurrent reconcilers during creation of controller", func() {
165165
const maxConcurrentReconciles = 5
166-
newController = func(name string, mgr manager.Manager, options controller.Options) (
166+
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (
167167
controller.Controller, error) {
168168
if options.MaxConcurrentReconciles == maxConcurrentReconciles {
169-
return controller.New(name, mgr, options)
169+
return controller.New(name, gvk, mgr, options)
170170
}
171171
return nil, fmt.Errorf("max concurrent reconcilers expected %d but found %d", maxConcurrentReconciles, options.MaxConcurrentReconciles)
172172
}
@@ -186,10 +186,10 @@ var _ = Describe("application", func() {
186186

187187
It("should override max concurrent reconcilers during creation of controller, when using", func() {
188188
const maxConcurrentReconciles = 10
189-
newController = func(name string, mgr manager.Manager, options controller.Options) (
189+
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (
190190
controller.Controller, error) {
191191
if options.MaxConcurrentReconciles == maxConcurrentReconciles {
192-
return controller.New(name, mgr, options)
192+
return controller.New(name, gvk, mgr, options)
193193
}
194194
return nil, fmt.Errorf("max concurrent reconcilers expected %d but found %d", maxConcurrentReconciles, options.MaxConcurrentReconciles)
195195
}
@@ -214,9 +214,9 @@ var _ = Describe("application", func() {
214214

215215
It("should override rate limiter during creation of controller", func() {
216216
rateLimiter := workqueue.DefaultItemBasedRateLimiter()
217-
newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
217+
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
218218
if options.RateLimiter == rateLimiter {
219-
return controller.New(name, mgr, options)
219+
return controller.New(name, gvk, mgr, options)
220220
}
221221
return nil, fmt.Errorf("rate limiter expected %T but found %T", rateLimiter, options.RateLimiter)
222222
}
@@ -237,9 +237,9 @@ var _ = Describe("application", func() {
237237
It("should override logger during creation of controller", func() {
238238

239239
logger := &testLogger{}
240-
newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
240+
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
241241
if options.Log.GetSink() == logger {
242-
return controller.New(name, mgr, options)
242+
return controller.New(name, gvk, mgr, options)
243243
}
244244
return nil, fmt.Errorf("logger expected %T but found %T", logger, options.Log)
245245
}
@@ -258,11 +258,11 @@ var _ = Describe("application", func() {
258258
})
259259

260260
It("should prefer reconciler from options during creation of controller", func() {
261-
newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
261+
newController = func(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
262262
if options.Reconciler != (typedNoop{}) {
263-
return nil, fmt.Errorf("Custom reconciler expected %T but found %T", typedNoop{}, options.Reconciler)
263+
return nil, fmt.Errorf("custom reconciler expected %T but found %T", typedNoop{}, options.Reconciler)
264264
}
265-
return controller.New(name, mgr, options)
265+
return controller.New(name, gvk, mgr, options)
266266
}
267267

268268
By("creating a controller manager")

‎pkg/controller/controller.go

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@ package controller
1818

1919
import (
2020
"context"
21+
crand "crypto/rand"
22+
"encoding/binary"
2123
"fmt"
24+
"math/rand"
2225
"time"
2326

2427
"github.com/go-logr/logr"
28+
"k8s.io/apimachinery/pkg/runtime/schema"
2529
"k8s.io/client-go/util/workqueue"
2630

2731
"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -84,8 +88,8 @@ type Controller interface {
8488

8589
// New returns a new Controller registered with the Manager. The Manager will ensure that shared Caches have
8690
// been synced before the Controller is Started.
87-
func New(name string, mgr manager.Manager, options Options) (Controller, error) {
88-
c, err := NewUnmanaged(name, mgr, options)
91+
func New(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options Options) (Controller, error) {
92+
c, err := NewUnmanaged(name, gvk, mgr, options)
8993
if err != nil {
9094
return nil, err
9195
}
@@ -96,7 +100,7 @@ func New(name string, mgr manager.Manager, options Options) (Controller, error)
96100

97101
// NewUnmanaged returns a new controller without adding it to the manager. The
98102
// caller is responsible for starting the returned controller.
99-
func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller, error) {
103+
func NewUnmanaged(name string, gvk *schema.GroupVersionKind, mgr manager.Manager, options Options) (Controller, error) {
100104
if options.Reconciler == nil {
101105
return nil, fmt.Errorf("must specify Reconciler")
102106
}
@@ -126,6 +130,19 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
126130
return nil, err
127131
}
128132

133+
// Add controller and reconciler group / kind to logger.
134+
log := options.Log.WithValues("controller", name)
135+
if gvk != nil {
136+
log = log.WithValues("reconciler group", gvk.Group, "reconciler kind", gvk.Kind)
137+
}
138+
139+
// Initialize random source, later used to generate reconcileIDs.
140+
var rngSeed int64
141+
if err := binary.Read(crand.Reader, binary.LittleEndian, &rngSeed); err != nil {
142+
return nil, fmt.Errorf("could not read random bytes to seed random source for reconcileID generation: %v", err)
143+
}
144+
randSource := rand.New(rand.NewSource(rngSeed)) //nolint:gosec // math/rand is enough, we don't need crypto/rand.
145+
129146
// Create controller with dependencies set
130147
return &controller.Controller{
131148
Do: options.Reconciler,
@@ -136,7 +153,9 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
136153
CacheSyncTimeout: options.CacheSyncTimeout,
137154
SetFields: mgr.SetFields,
138155
Name: name,
139-
Log: options.Log.WithName("controller").WithName(name).WithValues("controller", name),
156+
GroupVersionKind: gvk,
157+
Log: log,
158+
RandSource: randSource,
140159
RecoverPanic: options.RecoverPanic,
141160
}, nil
142161
}

‎pkg/controller/controller_integration_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
"k8s.io/apimachinery/pkg/runtime/schema"
2626
"k8s.io/apimachinery/pkg/types"
27+
2728
"sigs.k8s.io/controller-runtime/pkg/cache"
2829
"sigs.k8s.io/controller-runtime/pkg/controller"
2930
"sigs.k8s.io/controller-runtime/pkg/controller/controllertest"
@@ -33,6 +34,7 @@ import (
3334

3435
. "github.com/onsi/ginkgo"
3536
. "github.com/onsi/gomega"
37+
3638
"sigs.k8s.io/controller-runtime/pkg/manager"
3739
)
3840

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

5658
By("Creating the Controller")
57-
instance, err := controller.New("foo-controller", cm, controller.Options{
59+
instance, err := controller.New("foo-controller", nil, cm, controller.Options{
5860
Reconciler: reconcile.Func(
5961
func(_ context.Context, request reconcile.Request) (reconcile.Result, error) {
6062
reconciled <- request

‎pkg/controller/controller_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ var _ = Describe("controller.Controller", func() {
4545
It("should return an error if Name is not Specified", func() {
4646
m, err := manager.New(cfg, manager.Options{})
4747
Expect(err).NotTo(HaveOccurred())
48-
c, err := controller.New("", m, controller.Options{Reconciler: rec})
48+
c, err := controller.New("", nil, m, controller.Options{Reconciler: rec})
4949
Expect(c).To(BeNil())
5050
Expect(err.Error()).To(ContainSubstring("must specify Name for Controller"))
5151
})
@@ -54,7 +54,7 @@ var _ = Describe("controller.Controller", func() {
5454
m, err := manager.New(cfg, manager.Options{})
5555
Expect(err).NotTo(HaveOccurred())
5656

57-
c, err := controller.New("foo", m, controller.Options{})
57+
c, err := controller.New("foo", nil, m, controller.Options{})
5858
Expect(c).To(BeNil())
5959
Expect(err.Error()).To(ContainSubstring("must specify Reconciler"))
6060
})
@@ -63,7 +63,7 @@ var _ = Describe("controller.Controller", func() {
6363
m, err := manager.New(cfg, manager.Options{})
6464
Expect(err).NotTo(HaveOccurred())
6565

66-
c, err := controller.New("foo", m, controller.Options{Reconciler: &failRec{}})
66+
c, err := controller.New("foo", nil, m, controller.Options{Reconciler: &failRec{}})
6767
Expect(c).To(BeNil())
6868
Expect(err).To(HaveOccurred())
6969
Expect(err.Error()).To(ContainSubstring("expected error"))
@@ -73,11 +73,11 @@ var _ = Describe("controller.Controller", func() {
7373
m, err := manager.New(cfg, manager.Options{})
7474
Expect(err).NotTo(HaveOccurred())
7575

76-
c1, err := controller.New("c1", m, controller.Options{Reconciler: rec})
76+
c1, err := controller.New("c1", nil, m, controller.Options{Reconciler: rec})
7777
Expect(err).NotTo(HaveOccurred())
7878
Expect(c1).ToNot(BeNil())
7979

80-
c2, err := controller.New("c2", m, controller.Options{Reconciler: rec})
80+
c2, err := controller.New("c2", nil, m, controller.Options{Reconciler: rec})
8181
Expect(err).NotTo(HaveOccurred())
8282
Expect(c2).ToNot(BeNil())
8383
})
@@ -107,7 +107,7 @@ var _ = Describe("controller.Controller", func() {
107107
m, err := manager.New(cfg, manager.Options{})
108108
Expect(err).NotTo(HaveOccurred())
109109

110-
c, err := controller.New("new-controller", m, controller.Options{Reconciler: rec})
110+
c, err := controller.New("new-controller", nil, m, controller.Options{Reconciler: rec})
111111
Expect(c.Watch(watch, &handler.EnqueueRequestForObject{})).To(Succeed())
112112
Expect(err).NotTo(HaveOccurred())
113113

@@ -134,7 +134,7 @@ var _ = Describe("controller.Controller", func() {
134134
m, err := manager.New(cfg, manager.Options{})
135135
Expect(err).NotTo(HaveOccurred())
136136

137-
_, err = controller.New("new-controller", m, controller.Options{Reconciler: rec})
137+
_, err = controller.New("new-controller", nil, m, controller.Options{Reconciler: rec})
138138
Expect(err).NotTo(HaveOccurred())
139139

140140
// force-close keep-alive connections. These'll time anyway (after

‎pkg/controller/example_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
corev1 "k8s.io/api/core/v1"
2424
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2525
"k8s.io/apimachinery/pkg/runtime/schema"
26+
2627
"sigs.k8s.io/controller-runtime/pkg/controller"
2728
"sigs.k8s.io/controller-runtime/pkg/handler"
2829
logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -41,7 +42,7 @@ var (
4142
// This example creates a new Controller named "pod-controller" with a no-op reconcile function. The
4243
// manager.Manager will be used to Start the Controller, and will provide it a shared Cache and Client.
4344
func ExampleNew() {
44-
_, err := controller.New("pod-controller", mgr, controller.Options{
45+
_, err := controller.New("pod-controller", nil, mgr, controller.Options{
4546
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
4647
// Your business logic to implement the API by creating, updating, deleting objects goes here.
4748
return reconcile.Result{}, nil
@@ -59,7 +60,7 @@ func ExampleController() {
5960

6061
// Create a new Controller that will call the provided Reconciler function in response
6162
// to events.
62-
c, err := controller.New("pod-controller", mgr, controller.Options{
63+
c, err := controller.New("pod-controller", nil, mgr, controller.Options{
6364
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
6465
// Your business logic to implement the API by creating, updating, deleting objects goes here.
6566
return reconcile.Result{}, nil
@@ -90,7 +91,7 @@ func ExampleController_unstructured() {
9091

9192
// Create a new Controller that will call the provided Reconciler function in response
9293
// to events.
93-
c, err := controller.New("pod-controller", mgr, controller.Options{
94+
c, err := controller.New("pod-controller", nil, mgr, controller.Options{
9495
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
9596
// Your business logic to implement the API by creating, updating, deleting objects goes here.
9697
return reconcile.Result{}, nil
@@ -129,7 +130,7 @@ func ExampleNewUnmanaged() {
129130

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

0 commit comments

Comments
 (0)