Introduce `NewContext`, deprecate `NewImplFull`. (#2222)

* Introduce `NewContext`, deprecate `NewImplFull`.

Our generated `NewImpl` methods have long taken `context.Context`, but despite many iterations the forms we expose from our `controller` package never have.  This change contains several elements:
1. Expose a new `NewContext` method that takes `context.Context` in addition to the current `NewImplFull` signature.
2. Call `NewContext` instead of the deprecated `NewImpl` from our generated controller code.
3. Call `NewContext` from all our webhook reconcilers.

* Add a Tracker to controller.Impl to cut down on downstream boilerplate.
This commit is contained in:
Matt Moore 2021-08-21 14:00:34 -07:00 committed by GitHub
parent 766b8f7bd2
commit 9c7fd8e14f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 76 additions and 31 deletions

View File

@ -50,7 +50,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -101,7 +101,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "apiextensions.k8s.io.CustomResourceDefinition"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -50,7 +50,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -101,7 +101,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "apiextensions.k8s.io.CustomResourceDefinition"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -48,7 +48,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -99,7 +99,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "apps.Deployment"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -48,7 +48,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -99,7 +99,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "apps.Deployment"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -48,7 +48,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -99,7 +99,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "apps.Deployment"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -48,7 +48,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -99,7 +99,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "core.Namespace"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -48,7 +48,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -99,7 +99,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "core.Secret"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -48,7 +48,7 @@ const (
// NewImpl returns a controller.Impl that handles queuing and feeding work from
// the queue through an implementation of controller.Reconciler, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return
// controller.Options to be used by the internal reconciler.
// controller.ControllerOptions to be used by the internal reconciler.
func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsFn) *controller.Impl {
logger := logging.FromContext(ctx)
@ -99,7 +99,7 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF
zap.String(logkey.Kind, "extensions.Deployment"),
)
impl := controller.NewImpl(rec, logger, ctrTypeName)
impl := controller.NewContext(ctx, rec, controller.ControllerOptions{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -80,9 +80,9 @@ func (g *reconcilerControllerGenerator) GenerateType(c *generator.Context, t *ty
Package: "knative.dev/pkg/controller",
Name: "Reconciler",
}),
"controllerNewImpl": c.Universe.Function(types.Name{
"controllerNewContext": c.Universe.Function(types.Name{
Package: "knative.dev/pkg/controller",
Name: "NewImpl",
Name: "NewContext",
}),
"loggingFromContext": c.Universe.Function(types.Name{
Package: "knative.dev/pkg/logging",
@ -134,7 +134,7 @@ func (g *reconcilerControllerGenerator) GenerateType(c *generator.Context, t *ty
}),
"controllerOptions": c.Universe.Type(types.Name{
Package: "knative.dev/pkg/controller",
Name: "Options",
Name: "ControllerOptions",
}),
"controllerOptionsFn": c.Universe.Type(types.Name{
Package: "knative.dev/pkg/controller",
@ -257,7 +257,7 @@ func NewImpl(ctx {{.contextContext|raw}}, r Interface{{if .hasClass}}, classValu
)
impl := {{.controllerNewImpl|raw}}(rec, logger, ctrTypeName)
impl := {{.controllerNewContext|raw}}(ctx, rec, {{ .controllerOptions|raw }}{WorkQueueName: ctrTypeName, Logger: logger})
agentName := defaultControllerAgentName
// Pass impl to the options. Save any optional results.

View File

@ -41,6 +41,7 @@ import (
"knative.dev/pkg/logging"
"knative.dev/pkg/logging/logkey"
"knative.dev/pkg/reconciler"
"knative.dev/pkg/tracker"
)
const (
@ -216,6 +217,10 @@ type Impl struct {
// StatsReporter is used to send common controller metrics.
statsReporter StatsReporter
// Tracker allows reconcilers to associate a reference with particular key,
// such that when the reference changes the key is queued for reconciliation.
Tracker tracker.Interface
}
// ControllerOptions encapsulates options for creating a new controller,
@ -242,7 +247,14 @@ func NewImplWithStats(r Reconciler, logger *zap.SugaredLogger, workQueueName str
}
// NewImplFull accepts the full set of options available to all controllers.
// Deprecated: use NewContext instead.
func NewImplFull(r Reconciler, options ControllerOptions) *Impl {
return NewContext(context.TODO(), r, options)
}
// NewContext instantiates an instance of our controller that will feed work to the
// provided Reconciler as it is enqueued.
func NewContext(ctx context.Context, r Reconciler, options ControllerOptions) *Impl {
if options.RateLimiter == nil {
options.RateLimiter = workqueue.DefaultControllerRateLimiter()
}
@ -252,7 +264,7 @@ func NewImplFull(r Reconciler, options ControllerOptions) *Impl {
if options.Concurrency == 0 {
options.Concurrency = DefaultThreadsPerController
}
return &Impl{
i := &Impl{
Name: options.WorkQueueName,
Reconciler: r,
workQueue: newTwoLaneWorkQueue(options.WorkQueueName, options.RateLimiter),
@ -260,6 +272,14 @@ func NewImplFull(r Reconciler, options ControllerOptions) *Impl {
statsReporter: options.Reporter,
Concurrency: options.Concurrency,
}
if t := GetTracker(ctx); t != nil {
i.Tracker = t
} else {
i.Tracker = tracker.New(i.EnqueueKey, GetTrackerLease(ctx))
}
return i
}
// WorkQueue permits direct access to the work queue.
@ -816,6 +836,25 @@ func GetTrackerLease(ctx context.Context) time.Duration {
return 3 * GetResyncPeriod(ctx)
}
// trackerKey is used to associate tracker.Interface with contexts.
type trackerKey struct{}
// WithTracker attaches the given tracker.Interface to the provided context
// in the returned context.
func WithTracker(ctx context.Context, t tracker.Interface) context.Context {
return context.WithValue(ctx, trackerKey{}, t)
}
// GetTracker attempts to look up the tracker.Interface on a given context.
// It may return null if none is found.
func GetTracker(ctx context.Context) tracker.Interface {
untyped := ctx.Value(trackerKey{})
if untyped == nil {
return nil
}
return untyped.(tracker.Interface)
}
// erKey is used to associate record.EventRecorders with contexts.
type erKey struct{}

View File

@ -58,7 +58,10 @@ Update `NewController` as follows:
```go
"knative.dev/pkg/controller"
...
impl := controller.NewImpl(c, logger, "NameOfController")
impl := controller.NewContext(ctx, c, controller.ControllerOptions{
Logger: logger,
WorkQueueName: "NameOfController",
})
```
becomes
@ -139,7 +142,10 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
ServiceLister: svcInformer.Lister(),
}
logger = logger.Named("NameOfController")
impl := controller.NewImpl(c, logger, "NameOfController")
impl := controller.NewContext(ctx, c, controller.ControllerOptions{
Logger: logger,
WorkQueueName: "NameOfController",
})
// Set up event handlers.
svcInformer.Informer().AddEventHandler(...)
@ -421,7 +427,7 @@ kindreconciler "knative.dev/<repo>/pkg/client/injection/reconciler/<clientgroup>
Controller related artifacts:
- `NewImpl` - gets an injection based client and lister for `<kind>`, sets up
Kubernetes Event recorders, and delegates to `controller.NewImpl` for queue
Kubernetes Event recorders, and delegates to `controller.NewContext` for queue
management.
```go

View File

@ -67,7 +67,7 @@ func NewController(
}
const queueName = "WebhookCertificates"
c := controller.NewImpl(wh, logging.FromContext(ctx).Named(queueName), queueName)
c := controller.NewContext(ctx, wh, controller.ControllerOptions{WorkQueueName: queueName, Logger: logging.FromContext(ctx).Named(queueName)})
// Reconcile when the cert bundle changes.
secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{

View File

@ -74,7 +74,7 @@ func NewAdmissionController(
}
const queueName = "ConfigMapWebhook"
c := controller.NewImpl(wh, logging.FromContext(ctx).Named(queueName), queueName)
c := controller.NewContext(ctx, wh, controller.ControllerOptions{WorkQueueName: queueName, Logger: logging.FromContext(ctx).Named(queueName)})
// Reconcile when the named ValidatingWebhookConfiguration changes.
vwhInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{

View File

@ -143,7 +143,7 @@ func NewController(
scheme.Scheme, corev1.EventSource{Component: controllerAgentName}),
}
logger = logger.Named("GithubBindings")
impl := controller.NewImpl(c, logger, "GithubBindings")
impl := controller.NewContext(ctx, wh, controller.ControllerOptions{WorkQueueName: "GithubBinding", Logger: logger})
logger.Info("Setting up event handlers")

View File

@ -87,7 +87,7 @@ func NewAdmissionController(
// Construct the reconciler for the mutating webhook configuration.
wh := NewReconciler(name, path, options.SecretName, client, mwhInformer.Lister(), secretInformer.Lister(), withContext, reconcilerOptions...)
c := controller.NewImpl(wh, logging.FromContext(ctx).Named(name), name)
c := controller.NewContext(ctx, wh, controller.ControllerOptions{WorkQueueName: name, Logger: logging.FromContext(ctx).Named(name)})
// Enqueue a sentinel when we become leader.
wh.PromoteFunc = func(bkt pkgreconciler.Bucket, enq func(pkgreconciler.Bucket, types.NamespacedName)) error {

View File

@ -119,7 +119,7 @@ func NewConversionController(
const queueName = "ConversionWebhook"
logger := logging.FromContext(ctx)
c := controller.NewImpl(r, logger.Named(queueName), queueName)
c := controller.NewContext(ctx, r, controller.ControllerOptions{WorkQueueName: queueName, Logger: logger.Named(queueName)})
// Reconciler when the named CRDs change.
for _, gkc := range kinds {

View File

@ -75,7 +75,7 @@ func NewAdmissionController(
logger := logging.FromContext(ctx)
const queueName = "DefaultingWebhook"
c := controller.NewImpl(wh, logger.Named(queueName), queueName)
c := controller.NewContext(ctx, wh, controller.ControllerOptions{WorkQueueName: queueName, Logger: logger.Named(queueName)})
// Reconcile when the named MutatingWebhookConfiguration changes.
mwhInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{

View File

@ -90,7 +90,7 @@ func NewAdmissionController(
logger := logging.FromContext(ctx)
const queueName = "ValidationWebhook"
c := controller.NewImpl(wh, logger.Named(queueName), queueName)
c := controller.NewContext(ctx, wh, controller.ControllerOptions{WorkQueueName: queueName, Logger: logger.Named(queueName)})
// Reconcile when the named ValidatingWebhookConfiguration changes.
vwhInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{