Skip to content

Commit

Permalink
Allow default broker controller to reconcile unset brokerclass.
Browse files Browse the repository at this point in the history
  • Loading branch information
liu-cong committed Mar 17, 2020
1 parent 0c409f8 commit 60a7e50
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 37 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions pkg/reconciler/broker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions pkg/reconciler/broker/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"

Expand Down Expand Up @@ -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,
Expand Down
9 changes: 5 additions & 4 deletions pkg/reconciler/broker/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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),
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
}
Expand All @@ -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))
Expand All @@ -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 {
Expand Down Expand Up @@ -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}}
Expand Down

0 comments on commit 60a7e50

Please sign in to comment.