Skip to content

Commit

Permalink
update
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbueringer
sbueringer committed Mar 8, 2022
1 parent 1de701a commit 466549a
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 119 deletions.
3 changes: 1 addition & 2 deletions examples/builtins/main.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Expand Up @@ -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
)
Expand Down Expand Up @@ -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
Expand Down
29 changes: 23 additions & 6 deletions pkg/builder/controller.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
32 changes: 17 additions & 15 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, 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")
}
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, 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)
}
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, 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)
}
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, 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)
}
Expand All @@ -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")
Expand All @@ -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")
Expand Down
28 changes: 11 additions & 17 deletions pkg/controller/controller.go
View file
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Expand All @@ -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")
}
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions pkg/controller/controller_integration_test.go
View file
Expand Up @@ -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"
Expand All @@ -34,7 +33,6 @@ import (

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

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

Expand All @@ -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
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("", 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"))
})
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", nil, m, controller.Options{})
c, err := controller.New("foo", 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", 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"))
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", 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())
})
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", 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())

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", 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
Expand Down
9 changes: 4 additions & 5 deletions pkg/controller/example_test.go
View file
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}),
Expand Down

0 comments on commit 466549a

Please sign in to comment.