-
Notifications
You must be signed in to change notification settings - Fork 1.2k
✨Add predicates as variadic args for Owns, For, and Watches func #602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -37,15 +37,15 @@ var getGvk = apiutil.GVKForObject | |||||
|
||||||
// Builder builds a Controller. | ||||||
type Builder struct { | ||||||
apiType runtime.Object | ||||||
mgr manager.Manager | ||||||
predicates []predicate.Predicate | ||||||
managedObjects []runtime.Object | ||||||
watchRequest []watchRequest | ||||||
config *rest.Config | ||||||
ctrl controller.Controller | ||||||
ctrlOptions controller.Options | ||||||
name string | ||||||
apiType simpleWatch | ||||||
mgr manager.Manager | ||||||
globalPredicates []predicate.Predicate | ||||||
managedObjects []simpleWatch | ||||||
watchRequest []watchRequest | ||||||
config *rest.Config | ||||||
ctrl controller.Controller | ||||||
ctrlOptions controller.Options | ||||||
name string | ||||||
} | ||||||
|
||||||
// ControllerManagedBy returns a new controller builder that will be started by the provided Manager | ||||||
|
@@ -63,32 +63,46 @@ func (blder *Builder) ForType(apiType runtime.Object) *Builder { | |||||
return blder.For(apiType) | ||||||
} | ||||||
|
||||||
// simpleWatch represents the information required to construct a "simple" For | ||||||
// or Owns watch. | ||||||
type simpleWatch struct { | ||||||
item runtime.Object | ||||||
predicates []predicate.Predicate | ||||||
} | ||||||
|
||||||
// For defines the type of Object being *reconciled*, and configures the ControllerManagedBy to respond to create / delete / | ||||||
// update events by *reconciling the object*. | ||||||
// This is the equivalent of calling | ||||||
// Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{}) | ||||||
func (blder *Builder) For(apiType runtime.Object) *Builder { | ||||||
blder.apiType = apiType | ||||||
func (blder *Builder) For(reconciledAPIType runtime.Object, prct ...predicate.Predicate) *Builder { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
No need for shorter names |
||||||
blder.apiType = simpleWatch{ | ||||||
item: reconciledAPIType, | ||||||
predicates: prct, | ||||||
} | ||||||
return blder | ||||||
} | ||||||
|
||||||
// Owns defines types of Objects being *generated* by the ControllerManagedBy, and configures the ControllerManagedBy to respond to | ||||||
// create / delete / update events by *reconciling the owner object*. This is the equivalent of calling | ||||||
// Watches(&source.Kind{Type: <ForType-apiType>}, &handler.EnqueueRequestForOwner{OwnerType: apiType, IsController: true}) | ||||||
func (blder *Builder) Owns(apiType runtime.Object) *Builder { | ||||||
blder.managedObjects = append(blder.managedObjects, apiType) | ||||||
func (blder *Builder) Owns(apiType runtime.Object, prct ...predicate.Predicate) *Builder { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DirectXMan12 Is this the best way to add this functionality? Having varargs here prevents us to add more functionality in the future if we wanted to expose more configurable options. We could opt instead for a generic interface, similar for what we do for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, it is a functional options pattern and it could be better in that case at the beginning I just do not want to make it to complicated because I thought I will not break a contract |
||||||
blder.managedObjects = append(blder.managedObjects, simpleWatch{item: apiType, predicates: prct}) | ||||||
return blder | ||||||
} | ||||||
|
||||||
// watchRequest represents the information needed to construct a | ||||||
// manually-defined (non-"simple") watch. | ||||||
type watchRequest struct { | ||||||
src source.Source | ||||||
eventhandler handler.EventHandler | ||||||
predicates []predicate.Predicate | ||||||
} | ||||||
|
||||||
// Watches exposes the lower-level ControllerManagedBy Watches functions through the builder. Consider using | ||||||
// Owns or For instead of Watches directly. | ||||||
func (blder *Builder) Watches(src source.Source, eventhandler handler.EventHandler) *Builder { | ||||||
blder.watchRequest = append(blder.watchRequest, watchRequest{src: src, eventhandler: eventhandler}) | ||||||
// Specified predicates are registered only for given source. | ||||||
func (blder *Builder) Watches(src source.Source, eventhandler handler.EventHandler, prct ...predicate.Predicate) *Builder { | ||||||
blder.watchRequest = append(blder.watchRequest, watchRequest{src: src, eventhandler: eventhandler, predicates: prct}) | ||||||
return blder | ||||||
} | ||||||
|
||||||
|
@@ -102,9 +116,10 @@ func (blder *Builder) WithConfig(config *rest.Config) *Builder { | |||||
|
||||||
// WithEventFilter sets the event filters, to filter which create/update/delete/generic events eventually | ||||||
// trigger reconciliations. For example, filtering on whether the resource version has changed. | ||||||
// Given predicate is added for all watched objects. | ||||||
// Defaults to the empty list. | ||||||
func (blder *Builder) WithEventFilter(p predicate.Predicate) *Builder { | ||||||
blder.predicates = append(blder.predicates, p) | ||||||
blder.globalPredicates = append(blder.globalPredicates, p) | ||||||
return blder | ||||||
} | ||||||
|
||||||
|
@@ -157,28 +172,33 @@ func (blder *Builder) Build(r reconcile.Reconciler) (controller.Controller, erro | |||||
|
||||||
func (blder *Builder) doWatch() error { | ||||||
// Reconcile type | ||||||
src := &source.Kind{Type: blder.apiType} | ||||||
src := &source.Kind{Type: blder.apiType.item} | ||||||
hdler := &handler.EnqueueRequestForObject{} | ||||||
err := blder.ctrl.Watch(src, hdler, blder.predicates...) | ||||||
allPredicates := append(blder.globalPredicates, blder.apiType.predicates...) | ||||||
err := blder.ctrl.Watch(src, hdler, allPredicates...) | ||||||
if err != nil { | ||||||
return err | ||||||
} | ||||||
|
||||||
// Watches the managed types | ||||||
for _, obj := range blder.managedObjects { | ||||||
src := &source.Kind{Type: obj} | ||||||
src := &source.Kind{Type: obj.item} | ||||||
hdler := &handler.EnqueueRequestForOwner{ | ||||||
OwnerType: blder.apiType, | ||||||
OwnerType: blder.apiType.item, | ||||||
IsController: true, | ||||||
} | ||||||
if err := blder.ctrl.Watch(src, hdler, blder.predicates...); err != nil { | ||||||
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...) | ||||||
allPredicates = append(allPredicates, obj.predicates...) | ||||||
Comment on lines
+190
to
+191
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be simplified with allPredicates := append(blder.globalPredicates, w.predicates...) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as here: #602 (comment) |
||||||
if err := blder.ctrl.Watch(src, hdler, allPredicates...); err != nil { | ||||||
return err | ||||||
} | ||||||
} | ||||||
|
||||||
// Do the watch requests | ||||||
for _, w := range blder.watchRequest { | ||||||
if err := blder.ctrl.Watch(w.src, w.eventhandler, blder.predicates...); err != nil { | ||||||
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...) | ||||||
allPredicates = append(allPredicates, w.predicates...) | ||||||
Comment on lines
+199
to
+200
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be simplified with
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was like that at the beginning but @DirectXMan12 is right, it should be copied: #602 (comment) |
||||||
if err := blder.ctrl.Watch(w.src, w.eventhandler, allPredicates...); err != nil { | ||||||
return err | ||||||
} | ||||||
|
||||||
|
@@ -196,7 +216,7 @@ func (blder *Builder) getControllerName() (string, error) { | |||||
if blder.name != "" { | ||||||
return blder.name, nil | ||||||
} | ||||||
gvk, err := getGvk(blder.apiType, blder.mgr.GetScheme()) | ||||||
gvk, err := getGvk(blder.apiType.item, blder.mgr.GetScheme()) | ||||||
if err != nil { | ||||||
return "", err | ||||||
} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.