Skip to content

Commit 5f112fb

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1054 from vincepri/reconciler-context
⚠ Add a context w/ logger to Reconciler interface
2 parents fb1f0d6 + e230cb4 commit 5f112fb

19 files changed

+76
-61
lines changed

‎example_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,17 @@ type ReplicaSetReconciler struct {
116116
// * Read the ReplicaSet
117117
// * Read the Pods
118118
// * Set a Label on the ReplicaSet with the Pod count
119-
func (a *ReplicaSetReconciler) Reconcile(req controllers.Request) (controllers.Result, error) {
119+
func (a *ReplicaSetReconciler) Reconcile(ctx context.Context, req controllers.Request) (controllers.Result, error) {
120120
// Read the ReplicaSet
121121
rs := &appsv1.ReplicaSet{}
122-
err := a.Get(context.TODO(), req.NamespacedName, rs)
122+
err := a.Get(ctx, req.NamespacedName, rs)
123123
if err != nil {
124124
return controllers.Result{}, err
125125
}
126126

127127
// List the Pods matching the PodTemplate Labels
128128
pods := &corev1.PodList{}
129-
err = a.List(context.TODO(), pods, client.InNamespace(req.Namespace), client.MatchingLabels(rs.Spec.Template.Labels))
129+
err = a.List(ctx, pods, client.InNamespace(req.Namespace), client.MatchingLabels(rs.Spec.Template.Labels))
130130
if err != nil {
131131
return controllers.Result{}, err
132132
}

‎examples/builtins/controller.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,25 @@ import (
2020
"context"
2121
"fmt"
2222

23-
"github.com/go-logr/logr"
24-
2523
appsv1 "k8s.io/api/apps/v1"
2624
"k8s.io/apimachinery/pkg/api/errors"
2725
"sigs.k8s.io/controller-runtime/pkg/client"
26+
"sigs.k8s.io/controller-runtime/pkg/log"
2827
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2928
)
3029

3130
// reconcileReplicaSet reconciles ReplicaSets
3231
type reconcileReplicaSet struct {
3332
// client can be used to retrieve objects from the APIServer.
3433
client client.Client
35-
log logr.Logger
3634
}
3735

3836
// Implement reconcile.Reconciler so the controller can reconcile objects
3937
var _ reconcile.Reconciler = &reconcileReplicaSet{}
4038

41-
func (r *reconcileReplicaSet) Reconcile(request reconcile.Request) (reconcile.Result, error) {
39+
func (r *reconcileReplicaSet) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
4240
// set up a convenient log object so we don't have to type request over and over again
43-
log := r.log.WithValues("request", request)
41+
log := log.FromContext(ctx)
4442

4543
// Fetch the ReplicaSet from the cache
4644
rs := &appsv1.ReplicaSet{}

‎examples/builtins/main.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,20 @@ import (
2525
"sigs.k8s.io/controller-runtime/pkg/client/config"
2626
"sigs.k8s.io/controller-runtime/pkg/controller"
2727
"sigs.k8s.io/controller-runtime/pkg/handler"
28-
logf "sigs.k8s.io/controller-runtime/pkg/log"
28+
"sigs.k8s.io/controller-runtime/pkg/log"
2929
"sigs.k8s.io/controller-runtime/pkg/log/zap"
3030
"sigs.k8s.io/controller-runtime/pkg/manager"
3131
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
3232
"sigs.k8s.io/controller-runtime/pkg/source"
3333
"sigs.k8s.io/controller-runtime/pkg/webhook"
3434
)
3535

36-
var log = logf.Log.WithName("example-controller")
36+
func init() {
37+
log.SetLogger(zap.New())
38+
}
3739

3840
func main() {
39-
logf.SetLogger(zap.New())
40-
entryLog := log.WithName("entrypoint")
41+
entryLog := log.Log.WithName("entrypoint")
4142

4243
// Setup a Manager
4344
entryLog.Info("setting up manager")
@@ -50,7 +51,7 @@ func main() {
5051
// Setup a new controller to reconcile ReplicaSets
5152
entryLog.Info("Setting up controller")
5253
c, err := controller.New("foo-controller", mgr, controller.Options{
53-
Reconciler: &reconcileReplicaSet{client: mgr.GetClient(), log: log.WithName("reconciler")},
54+
Reconciler: &reconcileReplicaSet{client: mgr.GetClient()},
5455
})
5556
if err != nil {
5657
entryLog.Error(err, "unable to set up individual controller")

‎examples/crd/main.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,22 @@ import (
2929
ctrl "sigs.k8s.io/controller-runtime"
3030
api "sigs.k8s.io/controller-runtime/examples/crd/pkg"
3131
"sigs.k8s.io/controller-runtime/pkg/client"
32+
"sigs.k8s.io/controller-runtime/pkg/log"
3233
"sigs.k8s.io/controller-runtime/pkg/log/zap"
3334
)
3435

3536
var (
3637
setupLog = ctrl.Log.WithName("setup")
37-
recLog = ctrl.Log.WithName("reconciler")
3838
)
3939

4040
type reconciler struct {
4141
client.Client
4242
scheme *runtime.Scheme
4343
}
4444

45-
func (r *reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
46-
log := recLog.WithValues("chaospod", req.NamespacedName)
45+
func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
46+
log := log.FromContext(ctx).WithValues("chaospod", req.NamespacedName)
4747
log.V(1).Info("reconciling chaos pod")
48-
ctx := context.Background()
4948

5049
var chaospod api.ChaosPod
5150
if err := r.Get(ctx, req.NamespacedName, &chaospod); err != nil {

‎pkg/builder/controller.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,12 @@ func (blder *Builder) WithOptions(options controller.Options) *Builder {
130130
return blder
131131
}
132132

133+
// WithLogger overrides the controller options's logger used.
134+
func (blder *Builder) WithLogger(log logr.Logger) *Builder {
135+
blder.log = log
136+
return blder
137+
}
138+
133139
// Named sets the name of the controller to the given name. The name shows up
134140
// in metrics, among other things, and thus should be a prometheus compatible name
135141
// (underscores and alphanumeric characters only).
@@ -140,12 +146,6 @@ func (blder *Builder) Named(name string) *Builder {
140146
return blder
141147
}
142148

143-
// WithLogger overrides the controller options's logger used.
144-
func (blder *Builder) WithLogger(log logr.Logger) *Builder {
145-
blder.log = log
146-
return blder
147-
}
148-
149149
// Complete builds the Application ControllerManagedBy.
150150
func (blder *Builder) Complete(r reconcile.Reconciler) error {
151151
_, err := blder.Build(r)

‎pkg/builder/controller_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import (
4444

4545
type typedNoop struct{}
4646

47-
func (typedNoop) Reconcile(reconcile.Request) (reconcile.Result, error) {
47+
func (typedNoop) Reconcile(context.Context, reconcile.Request) (reconcile.Result, error) {
4848
return reconcile.Result{}, nil
4949
}
5050

@@ -60,7 +60,9 @@ var _ = Describe("application", func() {
6060
close(stop)
6161
})
6262

63-
noop := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { return reconcile.Result{}, nil })
63+
noop := reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
64+
return reconcile.Result{}, nil
65+
})
6466

6567
Describe("New", func() {
6668
It("should return success if given valid objects", func() {
@@ -302,7 +304,7 @@ func doReconcileTest(nameSuffix string, stop chan struct{}, blder *Builder, mgr
302304

303305
By("Creating the application")
304306
ch := make(chan reconcile.Request)
305-
fn := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) {
307+
fn := reconcile.Func(func(_ context.Context, req reconcile.Request) (reconcile.Result, error) {
306308
defer GinkgoRecover()
307309
if !strings.HasSuffix(req.Name, nameSuffix) {
308310
// From different test, ignore this request. Etcd is shared across tests.

‎pkg/builder/example_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,25 +79,24 @@ type ReplicaSetReconciler struct {
7979
// * Read the ReplicaSet
8080
// * Read the Pods
8181
// * Set a Label on the ReplicaSet with the Pod count
82-
func (a *ReplicaSetReconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) {
82+
func (a *ReplicaSetReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
8383
// Read the ReplicaSet
8484
rs := &appsv1.ReplicaSet{}
85-
err := a.Get(context.TODO(), req.NamespacedName, rs)
85+
err := a.Get(ctx, req.NamespacedName, rs)
8686
if err != nil {
8787
return reconcile.Result{}, err
8888
}
8989

9090
// List the Pods matching the PodTemplate Labels
9191
pods := &corev1.PodList{}
92-
err = a.List(context.TODO(), pods, client.InNamespace(req.Namespace),
93-
client.MatchingLabels(rs.Spec.Template.Labels))
92+
err = a.List(ctx, pods, client.InNamespace(req.Namespace), client.MatchingLabels(rs.Spec.Template.Labels))
9493
if err != nil {
9594
return reconcile.Result{}, err
9695
}
9796

9897
// Update the ReplicaSet
9998
rs.Labels["pod-count"] = fmt.Sprintf("%v", len(pods.Items))
100-
err = a.Update(context.TODO(), rs)
99+
err = a.Update(ctx, rs)
101100
if err != nil {
102101
return reconcile.Result{}, err
103102
}

‎pkg/controller/controller.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ type Options struct {
4343
// The overall is a token bucket and the per-item is exponential.
4444
RateLimiter ratelimiter.RateLimiter
4545

46-
// Log is the logger used for this controller.
46+
// Log is the logger used for this controller and passed to each reconciliation
47+
// request via the context field.
4748
Log logr.Logger
4849
}
4950

@@ -91,6 +92,10 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
9192
return nil, fmt.Errorf("must specify Name for Controller")
9293
}
9394

95+
if options.Log == nil {
96+
options.Log = mgr.GetLogger()
97+
}
98+
9499
if options.MaxConcurrentReconciles <= 0 {
95100
options.MaxConcurrentReconciles = 1
96101
}
@@ -117,6 +122,6 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
117122
MaxConcurrentReconciles: options.MaxConcurrentReconciles,
118123
SetFields: mgr.SetFields,
119124
Name: name,
120-
Log: options.Log.WithName("controller").WithValues("controller", name),
125+
Log: options.Log.WithName("controller").WithName(name),
121126
}, nil
122127
}

‎pkg/controller/controller_integration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ var _ = Describe("controller", func() {
6262
By("Creating the Controller")
6363
instance, err := controller.New("foo-controller", cm, controller.Options{
6464
Reconciler: reconcile.Func(
65-
func(request reconcile.Request) (reconcile.Result, error) {
65+
func(_ context.Context, request reconcile.Request) (reconcile.Result, error) {
6666
reconciled <- request
6767
return reconcile.Result{}, nil
6868
}),

‎pkg/controller/controller_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package controller_test
1818

1919
import (
20+
"context"
2021
"fmt"
2122
rt "runtime"
2223

@@ -33,7 +34,7 @@ import (
3334
var _ = Describe("controller.Controller", func() {
3435
var stop chan struct{}
3536

36-
rec := reconcile.Func(func(reconcile.Request) (reconcile.Result, error) {
37+
rec := reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
3738
return reconcile.Result{}, nil
3839
})
3940
BeforeEach(func() {
@@ -123,7 +124,7 @@ var _ inject.Client = &failRec{}
123124

124125
type failRec struct{}
125126

126-
func (*failRec) Reconcile(reconcile.Request) (reconcile.Result, error) {
127+
func (*failRec) Reconcile(context.Context, reconcile.Request) (reconcile.Result, error) {
127128
return reconcile.Result{}, nil
128129
}
129130

‎pkg/controller/example_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package controller_test
1818

1919
import (
20+
"context"
2021
"os"
2122

2223
corev1 "k8s.io/api/core/v1"
@@ -41,7 +42,7 @@ var (
4142
// manager.Manager will be used to Start the Controller, and will provide it a shared Cache and Client.
4243
func ExampleNew() {
4344
_, err := controller.New("pod-controller", mgr, controller.Options{
44-
Reconciler: reconcile.Func(func(o reconcile.Request) (reconcile.Result, error) {
45+
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
4546
// Your business logic to implement the API by creating, updating, deleting objects goes here.
4647
return reconcile.Result{}, nil
4748
}),
@@ -59,7 +60,7 @@ func ExampleController() {
5960
// Create a new Controller that will call the provided Reconciler function in response
6061
// to events.
6162
c, err := controller.New("pod-controller", mgr, controller.Options{
62-
Reconciler: reconcile.Func(func(o reconcile.Request) (reconcile.Result, error) {
63+
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
6364
// Your business logic to implement the API by creating, updating, deleting objects goes here.
6465
return reconcile.Result{}, nil
6566
}),
@@ -90,7 +91,7 @@ func ExampleController_unstructured() {
9091
// Create a new Controller that will call the provided Reconciler function in response
9192
// to events.
9293
c, err := controller.New("pod-controller", mgr, controller.Options{
93-
Reconciler: reconcile.Func(func(o reconcile.Request) (reconcile.Result, error) {
94+
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
9495
// Your business logic to implement the API by creating, updating, deleting objects goes here.
9596
return reconcile.Result{}, nil
9697
}),
@@ -129,7 +130,7 @@ func ExampleNewUnmanaged() {
129130
// Configure creates a new controller but does not add it to the supplied
130131
// manager.
131132
c, err := controller.NewUnmanaged("pod-controller", mgr, controller.Options{
132-
Reconciler: reconcile.Func(func(_ reconcile.Request) (reconcile.Result, error) {
133+
Reconciler: reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
133134
return reconcile.Result{}, nil
134135
}),
135136
})

‎pkg/internal/controller/controller.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package controller
1818

1919
import (
20+
"context"
2021
"fmt"
2122
"sync"
2223
"time"
@@ -27,6 +28,7 @@ import (
2728
"k8s.io/client-go/util/workqueue"
2829
"sigs.k8s.io/controller-runtime/pkg/handler"
2930
ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics"
31+
logf "sigs.k8s.io/controller-runtime/pkg/log"
3032
"sigs.k8s.io/controller-runtime/pkg/predicate"
3133
"sigs.k8s.io/controller-runtime/pkg/reconcile"
3234
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
@@ -86,8 +88,10 @@ type watchDescription struct {
8688
}
8789

8890
// Reconcile implements reconcile.Reconciler
89-
func (c *Controller) Reconcile(r reconcile.Request) (reconcile.Result, error) {
90-
return c.Do.Reconcile(r)
91+
func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
92+
log := c.Log.WithValues("name", req.Name, "namespace", req.Namespace)
93+
ctx = logf.IntoContext(ctx, log)
94+
return c.Do.Reconcile(ctx, req)
9195
}
9296

9397
// Watch implements controller.Controller
@@ -229,12 +233,15 @@ func (c *Controller) reconcileHandler(obj interface{}) bool {
229233
}
230234

231235
log := c.Log.WithValues("name", req.Name, "namespace", req.Namespace)
236+
ctx := logf.IntoContext(context.Background(), log)
232237

233238
// RunInformersAndControllers the syncHandler, passing it the namespace/Name string of the
234239
// resource to be synced.
235-
if result, err := c.Do.Reconcile(req); err != nil {
240+
if result, err := c.Do.Reconcile(ctx, req); err != nil {
236241
c.Queue.AddRateLimited(req)
237-
log.Error(err, "Reconciler error")
242+
if log.V(3).Enabled() {
243+
log.Error(err, "Reconciler error")
244+
}
238245
ctrlmetrics.ReconcileErrors.WithLabelValues(c.Name).Inc()
239246
ctrlmetrics.ReconcileTotal.WithLabelValues(c.Name, "error").Inc()
240247
return false
@@ -258,7 +265,7 @@ func (c *Controller) reconcileHandler(obj interface{}) bool {
258265
c.Queue.Forget(obj)
259266

260267
// TODO(directxman12): What does 1 mean? Do we want level constants? Do we want levels at all?
261-
log.V(1).Info("Successfully Reconciled")
268+
log.V(5).Info("Successfully Reconciled")
262269

263270
ctrlmetrics.ReconcileTotal.WithLabelValues(c.Name, "success").Inc()
264271
// Return true, don't take a break

‎pkg/internal/controller/controller_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,10 @@ var _ = Describe("controller", func() {
7878

7979
Describe("Reconciler", func() {
8080
It("should call the Reconciler function", func() {
81-
ctrl.Do = reconcile.Func(func(reconcile.Request) (reconcile.Result, error) {
81+
ctrl.Do = reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
8282
return reconcile.Result{Requeue: true}, nil
8383
})
84-
result, err := ctrl.Reconcile(
84+
result, err := ctrl.Reconcile(context.Background(),
8585
reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "foo", Name: "bar"}})
8686
Expect(err).NotTo(HaveOccurred())
8787
Expect(result).To(Equal(reconcile.Result{Requeue: true}))
@@ -304,7 +304,7 @@ var _ = Describe("controller", func() {
304304
})
305305

306306
It("should continue to process additional queue items after the first", func(done Done) {
307-
ctrl.Do = reconcile.Func(func(reconcile.Request) (reconcile.Result, error) {
307+
ctrl.Do = reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
308308
defer GinkgoRecover()
309309
Fail("Reconciler should not have been called")
310310
return reconcile.Result{}, nil
@@ -766,7 +766,7 @@ func (f *fakeReconciler) AddResult(res reconcile.Result, err error) {
766766
f.results <- fakeReconcileResultPair{Result: res, Err: err}
767767
}
768768

769-
func (f *fakeReconciler) Reconcile(r reconcile.Request) (reconcile.Result, error) {
769+
func (f *fakeReconciler) Reconcile(_ context.Context, r reconcile.Request) (reconcile.Result, error) {
770770
res := <-f.results
771771
if f.Requests != nil {
772772
f.Requests <- r

0 commit comments

Comments
 (0)