From 60a7e50ad37bb470a38c83c36ae071adf3152ec8 Mon Sep 17 00:00:00 2001 From: Cong Liu Date: Mon, 16 Mar 2020 19:14:20 -0700 Subject: [PATCH] Allow default broker controller to reconcile unset brokerclass. --- .../eventing/v1alpha1/broker/controller.go | 12 +++++------ .../eventing/v1alpha1/broker/reconciler.go | 20 +++++++++---------- .../v1alpha1/broker/stub/controller.go | 5 +++-- pkg/reconciler/broker/broker.go | 3 --- pkg/reconciler/broker/broker_test.go | 6 ++++-- pkg/reconciler/broker/controller.go | 9 +++++---- .../generators/reconciler_controller.go | 4 ++-- .../generators/reconciler_controller_stub.go | 5 +++-- .../generators/reconciler_reconciler.go | 12 +++++------ 9 files changed, 39 insertions(+), 37 deletions(-) diff --git a/pkg/client/injection/reconciler/eventing/v1alpha1/broker/controller.go b/pkg/client/injection/reconciler/eventing/v1alpha1/broker/controller.go index 9469beb176f..5b29fd923bb 100644 --- a/pkg/client/injection/reconciler/eventing/v1alpha1/broker/controller.go +++ b/pkg/client/injection/reconciler/eventing/v1alpha1/broker/controller.go @@ -47,7 +47,7 @@ const ( // 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 but the internal reconciler. -func NewImpl(ctx context.Context, r Interface, classValue string, optionsFns ...controller.OptionsFn) *controller.Impl { +func NewImpl(ctx context.Context, r Interface, classFilter func(interface{}) bool, optionsFns ...controller.OptionsFn) *controller.Impl { logger := logging.FromContext(ctx) // Check the options function input. It should be 0 or 1. @@ -77,11 +77,11 @@ func NewImpl(ctx context.Context, r Interface, classValue string, optionsFns ... } rec := &reconcilerImpl{ - Client: injectionclient.Get(ctx), - Lister: brokerInformer.Lister(), - Recorder: recorder, - reconciler: r, - classValue: classValue, + Client: injectionclient.Get(ctx), + Lister: brokerInformer.Lister(), + Recorder: recorder, + reconciler: r, + classFilter: classFilter, } impl := controller.NewImpl(rec, logger, defaultQueueName) diff --git a/pkg/client/injection/reconciler/eventing/v1alpha1/broker/reconciler.go b/pkg/client/injection/reconciler/eventing/v1alpha1/broker/reconciler.go index 69046da54ff..d5ba5cc97ce 100644 --- a/pkg/client/injection/reconciler/eventing/v1alpha1/broker/reconciler.go +++ b/pkg/client/injection/reconciler/eventing/v1alpha1/broker/reconciler.go @@ -82,25 +82,25 @@ type reconcilerImpl struct { // reconciler is the implementation of the business logic of the resource. reconciler Interface - // classValue is the resource annotation[eventing.knative.dev/broker.class] instance value this reconciler instance filters on. - classValue string + // classFilter is the filter function for annotation[eventing.knative.dev/broker.class] instance value this reconciler instance filters on. + classFilter func(interface{}) bool } // Check that our Reconciler implements controller.Reconciler var _ controller.Reconciler = (*reconcilerImpl)(nil) -func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versioned.Interface, lister eventingv1alpha1.BrokerLister, recorder record.EventRecorder, r Interface, classValue string, options ...controller.Options) controller.Reconciler { +func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versioned.Interface, lister eventingv1alpha1.BrokerLister, recorder record.EventRecorder, r Interface, classFilter func(interface{}) bool, options ...controller.Options) controller.Reconciler { // Check the options function input. It should be 0 or 1. if len(options) > 1 { logger.Fatalf("up to one options struct is supported, found %d", len(options)) } rec := &reconcilerImpl{ - Client: client, - Lister: lister, - Recorder: recorder, - reconciler: r, - classValue: classValue, + Client: client, + Lister: lister, + Recorder: recorder, + reconciler: r, + classFilter: classFilter, } for _, opts := range options { @@ -141,10 +141,10 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { return err } - if classValue, found := original.GetAnnotations()[ClassAnnotationKey]; !found || classValue != r.classValue { + if !r.classFilter(original) { logger.Debugw("Skip reconciling resource, class annotation value does not match reconciler instance value.", zap.String("classKey", ClassAnnotationKey), - zap.String("issue", classValue+"!="+r.classValue)) + zap.String("classValue", original.GetAnnotations()[ClassAnnotationKey])) return nil } diff --git a/pkg/client/injection/reconciler/eventing/v1alpha1/broker/stub/controller.go b/pkg/client/injection/reconciler/eventing/v1alpha1/broker/stub/controller.go index 6af233931ac..cf05dec9d19 100644 --- a/pkg/client/injection/reconciler/eventing/v1alpha1/broker/stub/controller.go +++ b/pkg/client/injection/reconciler/eventing/v1alpha1/broker/stub/controller.go @@ -42,13 +42,14 @@ func NewController( brokerInformer := broker.Get(ctx) classValue := "default" // TODO: update this to the appropriate value. - classFilter := reconciler.AnnotationFilterFunc(v1alpha1broker.ClassAnnotationKey, classValue, false /*allowUnset*/) + allowUnset := false // TODO: update this to specific matching unset ClassAnnotationKey or not. + classFilter := reconciler.AnnotationFilterFunc(v1alpha1broker.ClassAnnotationKey, classValue, allowUnset /*allowUnset*/) // TODO: setup additional informers here. // TODO: remember to use the classFilter from above to filter appropriately. r := &Reconciler{} - impl := v1alpha1broker.NewImpl(ctx, r, classValue) + impl := v1alpha1broker.NewImpl(ctx, r, classFilter) logger.Info("Setting up event handlers.") diff --git a/pkg/reconciler/broker/broker.go b/pkg/reconciler/broker/broker.go index ab8a2f32fed..ef7eafcef55 100644 --- a/pkg/reconciler/broker/broker.go +++ b/pkg/reconciler/broker/broker.go @@ -81,9 +81,6 @@ type Reconciler struct { // Dynamic tracker to track AddressableTypes. In particular, it tracks Trigger subscribers. addressableTracker duck.ListableTracker uriResolver *resolver.URIResolver - - // If specified, only reconcile brokers with these labels - brokerClass string } // Check that our Reconciler implements Interface diff --git a/pkg/reconciler/broker/broker_test.go b/pkg/reconciler/broker/broker_test.go index 0c144e5a22d..7105994f4a7 100644 --- a/pkg/reconciler/broker/broker_test.go +++ b/pkg/reconciler/broker/broker_test.go @@ -38,6 +38,7 @@ import ( messagingv1alpha1 "knative.dev/eventing/pkg/apis/messaging/v1alpha1" "knative.dev/eventing/pkg/client/injection/ducks/duck/v1alpha1/channelable" "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1alpha1/broker" + brokerreconciler "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1alpha1/broker" "knative.dev/eventing/pkg/duck" "knative.dev/eventing/pkg/reconciler" "knative.dev/eventing/pkg/reconciler/broker/resources" @@ -53,6 +54,7 @@ import ( "knative.dev/pkg/configmap" "knative.dev/pkg/controller" logtesting "knative.dev/pkg/logging/testing" + pkgreconciler "knative.dev/pkg/reconciler" "knative.dev/pkg/resolver" "knative.dev/pkg/system" @@ -1459,9 +1461,9 @@ func TestReconcile(t *testing.T) { channelableTracker: duck.NewListableTracker(ctx, channelable.Get, func(types.NamespacedName) {}, 0), addressableTracker: duck.NewListableTracker(ctx, v1a1addr.Get, func(types.NamespacedName) {}, 0), uriResolver: resolver.NewURIResolver(ctx, func(types.NamespacedName) {}), - brokerClass: eventing.ChannelBrokerClassValue, } - return broker.NewReconciler(ctx, r.Logger, r.EventingClientSet, listers.GetBrokerLister(), r.Recorder, r, eventing.ChannelBrokerClassValue) + classFilter := pkgreconciler.AnnotationFilterFunc(brokerreconciler.ClassAnnotationKey, eventing.ChannelBrokerClassValue, true) + return broker.NewReconciler(ctx, r.Logger, r.EventingClientSet, listers.GetBrokerLister(), r.Recorder, r, classFilter) }, false, diff --git a/pkg/reconciler/broker/controller.go b/pkg/reconciler/broker/controller.go index eb7cbf219d9..51aab2bd769 100644 --- a/pkg/reconciler/broker/controller.go +++ b/pkg/reconciler/broker/controller.go @@ -90,10 +90,11 @@ func NewController( ingressServiceAccountName: env.IngressServiceAccount, filterImage: env.FilterImage, filterServiceAccountName: env.FilterServiceAccount, - brokerClass: env.BrokerClass, } - - impl := brokerreconciler.NewImpl(ctx, r, env.BrokerClass) + classValue := env.BrokerClass + allowUnset := true + classFilter := pkgreconciler.AnnotationFilterFunc(brokerreconciler.ClassAnnotationKey, classValue, allowUnset /*allowUnset*/) + impl := brokerreconciler.NewImpl(ctx, r, classFilter) r.Logger.Info("Setting up event handlers") @@ -103,7 +104,7 @@ func NewController( r.uriResolver = resolver.NewURIResolver(ctx, impl.EnqueueKey) brokerInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ - FilterFunc: pkgreconciler.AnnotationFilterFunc(brokerreconciler.ClassAnnotationKey, env.BrokerClass, false /*allowUnset*/), + FilterFunc: classFilter, Handler: controller.HandleAll(impl.Enqueue), }) diff --git a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller.go b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller.go index a41e3a1e204..da2b6f4a8ab 100644 --- a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller.go +++ b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller.go @@ -164,7 +164,7 @@ const ( // the queue through an implementation of {{.controllerReconciler|raw}}, delegating to // the provided Interface and optional Finalizer methods. OptionsFn is used to return // {{.controllerOptions|raw}} to be used but the internal reconciler. -func NewImpl(ctx {{.contextContext|raw}}, r Interface{{if .hasClass}}, classValue string{{end}}, optionsFns ...{{.controllerOptionsFn|raw}}) *{{.controllerImpl|raw}} { +func NewImpl(ctx {{.contextContext|raw}}, r Interface{{if .hasClass}}, classFilter func(interface{}) bool{{end}}, optionsFns ...{{.controllerOptionsFn|raw}}) *{{.controllerImpl|raw}} { logger := {{.loggingFromContext|raw}}(ctx) // Check the options function input. It should be 0 or 1. @@ -198,7 +198,7 @@ func NewImpl(ctx {{.contextContext|raw}}, r Interface{{if .hasClass}}, classValu Lister: {{.type|lowercaseSingular}}Informer.Lister(), Recorder: recorder, reconciler: r, - {{if .hasClass}}classValue: classValue,{{end}} + {{if .hasClass}}classFilter: classFilter,{{end}} } impl := {{.controllerNewImpl|raw}}(rec, logger, defaultQueueName) diff --git a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go index 4c745e43fd7..6e645834093 100644 --- a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go +++ b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go @@ -120,14 +120,15 @@ func NewController( {{if .hasClass}} classValue := "default" // TODO: update this to the appropriate value. - classFilter := {{.annotationFilterFunc|raw}}({{.classAnnotationKey|raw}}, classValue, false /*allowUnset*/) + allowUnset := false // TODO: update this to specific matching unset ClassAnnotationKey or not. + classFilter := {{.annotationFilterFunc|raw}}({{.classAnnotationKey|raw}}, classValue, allowUnset /*allowUnset*/) {{end}} // TODO: setup additional informers here. {{if .hasClass}}// TODO: remember to use the classFilter from above to filter appropriately.{{end}} r := &Reconciler{} - impl := {{.reconcilerNewImpl|raw}}(ctx, r{{if .hasClass}}, classValue{{end}}) + impl := {{.reconcilerNewImpl|raw}}(ctx, r{{if .hasClass}}, classFilter{{end}}) logger.Info("Setting up event handlers.") diff --git a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go index c3e08bdbf37..a434f971a8b 100644 --- a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go +++ b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go @@ -198,8 +198,8 @@ type reconcilerImpl struct { reconciler Interface {{if .hasClass}} - // classValue is the resource annotation[{{ .class }}] instance value this reconciler instance filters on. - classValue string + // classFilter is the filter function for annotation[eventing.knative.dev/broker.class] instance value this reconciler instance filters on. + classFilter func(interface{}) bool {{end}} } @@ -209,7 +209,7 @@ var _ controller.Reconciler = (*reconcilerImpl)(nil) ` var reconcilerNewReconciler = ` -func NewReconciler(ctx {{.contextContext|raw}}, logger *{{.zapSugaredLogger|raw}}, client {{.clientsetInterface|raw}}, lister {{.resourceLister|raw}}, recorder {{.recordEventRecorder|raw}}, r Interface{{if .hasClass}}, classValue string{{end}}, options ...{{.controllerOptions|raw}} ) {{.controllerReconciler|raw}} { +func NewReconciler(ctx {{.contextContext|raw}}, logger *{{.zapSugaredLogger|raw}}, client {{.clientsetInterface|raw}}, lister {{.resourceLister|raw}}, recorder {{.recordEventRecorder|raw}}, r Interface{{if .hasClass}}, classFilter func(interface{}) bool{{end}}, options ...{{.controllerOptions|raw}} ) {{.controllerReconciler|raw}} { // Check the options function input. It should be 0 or 1. if len(options) > 1 { logger.Fatalf("up to one options struct is supported, found %d", len(options)) @@ -220,7 +220,7 @@ func NewReconciler(ctx {{.contextContext|raw}}, logger *{{.zapSugaredLogger|raw} Lister: lister, Recorder: recorder, reconciler: r, - {{if .hasClass}}classValue: classValue,{{end}} + {{if .hasClass}}classFilter: classFilter,{{end}} } for _, opts := range options { @@ -263,10 +263,10 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro return err } {{if .hasClass}} - if classValue, found := original.GetAnnotations()[ClassAnnotationKey]; !found || classValue != r.classValue { + if !r.classFilter(original) { logger.Debugw("Skip reconciling resource, class annotation value does not match reconciler instance value.", zap.String("classKey", ClassAnnotationKey), - zap.String("issue", classValue+"!="+r.classValue)) + zap.String("classValue", original.GetAnnotations()[ClassAnnotationKey])) return nil } {{end}}