Skip to content
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

🌱 Minor improvements to godoc, code style in builder pkg #2414

Merged
merged 1 commit into from Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 25 additions & 26 deletions pkg/builder/controller.go
Expand Up @@ -41,14 +41,14 @@ import (
var newController = controller.New
var getGvk = apiutil.GVKForObject

// project represents other forms that the we can use to
// project represents other forms that we can use to
// send/receive a given resource (metadata-only, unstructured, etc).
type objectProjection int

const (
// projectAsNormal doesn't change the object from the form given.
projectAsNormal objectProjection = iota
// projectAsMetadata turns this into an metadata-only watch.
// projectAsMetadata turns this into a metadata-only watch.
projectAsMetadata
)

Expand All @@ -69,7 +69,7 @@ func ControllerManagedBy(m manager.Manager) *Builder {
return &Builder{mgr: m}
}

// ForInput represents the information set by For method.
// ForInput represents the information set by the For method.
type ForInput struct {
object client.Object
predicates []predicate.Predicate
Expand Down Expand Up @@ -124,7 +124,7 @@ func (blder *Builder) Owns(object client.Object, opts ...OwnsOption) *Builder {
// WatchesInput represents the information set by Watches method.
type WatchesInput struct {
src source.Source
eventhandler handler.EventHandler
eventHandler handler.EventHandler
predicates []predicate.Predicate
objectProjection objectProjection
}
Expand All @@ -133,16 +133,16 @@ type WatchesInput struct {
// update events by *reconciling the object* with the given EventHandler.
//
// This is the equivalent of calling
// WatchesRawSource(source.Kind(scheme, object), eventhandler, opts...).
func (blder *Builder) Watches(object client.Object, eventhandler handler.EventHandler, opts ...WatchesOption) *Builder {
// WatchesRawSource(source.Kind(cache, object), eventHandler, opts...).
func (blder *Builder) Watches(object client.Object, eventHandler handler.EventHandler, opts ...WatchesOption) *Builder {
src := source.Kind(blder.mgr.GetCache(), object)
return blder.WatchesRawSource(src, eventhandler, opts...)
return blder.WatchesRawSource(src, eventHandler, opts...)
}

// WatchesMetadata is the same as Watches, but forces the internal cache to only watch PartialObjectMetadata.
//
// This is useful when watching lots of objects, really big objects, or objects for which you only know
// the GVK, but not the structure. You'll need to pass metav1.PartialObjectMetadata to the client
// the GVK, but not the structure. You'll need to pass metav1.PartialObjectMetadata to the client
// when fetching objects in your reconciler, otherwise you'll end up with a duplicate structured or unstructured cache.
//
// When watching a resource with metadata only, for example the v1.Pod, you should not Get and List using the v1.Pod type.
Expand All @@ -166,18 +166,18 @@ func (blder *Builder) Watches(object client.Object, eventhandler handler.EventHa
// In the first case, controller-runtime will create another cache for the
// concrete type on top of the metadata cache; this increases memory
// consumption and leads to race conditions as caches are not in sync.
func (blder *Builder) WatchesMetadata(object client.Object, eventhandler handler.EventHandler, opts ...WatchesOption) *Builder {
func (blder *Builder) WatchesMetadata(object client.Object, eventHandler handler.EventHandler, opts ...WatchesOption) *Builder {
opts = append(opts, OnlyMetadata)
return blder.Watches(object, eventhandler, opts...)
return blder.Watches(object, eventHandler, opts...)
}

// WatchesRawSource exposes the lower-level ControllerManagedBy Watches functions through the builder.
// Specified predicates are registered only for given source.
//
// STOP! Consider using For(...), Owns(...), Watches(...), WatchesMetadata(...) instead.
// This method is only exposed for more advanced use cases, most users should use higher level functions.
func (blder *Builder) WatchesRawSource(src source.Source, eventhandler handler.EventHandler, opts ...WatchesOption) *Builder {
input := WatchesInput{src: src, eventhandler: eventhandler}
// This method is only exposed for more advanced use cases, most users should use one of the higher level functions.
func (blder *Builder) WatchesRawSource(src source.Source, eventHandler handler.EventHandler, opts ...WatchesOption) *Builder {
input := WatchesInput{src: src, eventHandler: eventHandler}
for _, opt := range opts {
opt.ApplyToWatches(&input)
}
Expand All @@ -187,15 +187,15 @@ func (blder *Builder) WatchesRawSource(src source.Source, eventhandler handler.E
}

// 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.
// 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.globalPredicates = append(blder.globalPredicates, p)
return blder
}

// WithOptions overrides the controller options use in doController. Defaults to empty.
// WithOptions overrides the controller options used in doController. Defaults to empty.
func (blder *Builder) WithOptions(options controller.Options) *Builder {
blder.ctrlOptions = options
return blder
Expand All @@ -207,7 +207,7 @@ func (blder *Builder) WithLogConstructor(logConstructor func(*reconcile.Request)
return blder
}

// Named sets the name of the controller to the given name. The name shows up
// Named sets the name of the controller to the given name. The name shows up
// in metrics, among other things, and thus should be a prometheus compatible name
// (underscores and alphanumeric characters only).
//
Expand Down Expand Up @@ -274,7 +274,8 @@ func (blder *Builder) doWatch() error {
}
src := source.Kind(blder.mgr.GetCache(), obj)
hdler := &handler.EnqueueRequestForObject{}
allPredicates := append(blder.globalPredicates, blder.forInput.predicates...)
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just aligning to the way we do it below. I played a bit around with it, I don't think it makes a difference

allPredicates = append(allPredicates, blder.forInput.predicates...)
if err := blder.ctrl.Watch(src, hdler, allPredicates...); err != nil {
return err
}
Expand Down Expand Up @@ -311,19 +312,17 @@ func (blder *Builder) doWatch() error {
return errors.New("there are no watches configured, controller will never get triggered. Use For(), Owns() or Watches() to set them up")
}
for _, w := range blder.watchesInput {
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
sbueringer marked this conversation as resolved.
Show resolved Hide resolved
allPredicates = append(allPredicates, w.predicates...)

// If the source of this watch is of type Kind, project it.
if srckind, ok := w.src.(*internalsource.Kind); ok {
typeForSrc, err := blder.project(srckind.Type, w.objectProjection)
if srcKind, ok := w.src.(*internalsource.Kind); ok {
typeForSrc, err := blder.project(srcKind.Type, w.objectProjection)
if err != nil {
return err
}
srckind.Type = typeForSrc
srcKind.Type = typeForSrc
}

if err := blder.ctrl.Watch(w.src, w.eventhandler, allPredicates...); err != nil {
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
allPredicates = append(allPredicates, w.predicates...)
if err := blder.ctrl.Watch(w.src, w.eventHandler, allPredicates...); err != nil {
return err
}
}
Expand All @@ -349,7 +348,7 @@ func (blder *Builder) doController(r reconcile.Reconciler) error {
}

// Retrieve the GVK from the object we're reconciling
// to prepopulate logger information, and to optionally generate a default name.
// to pre-populate logger information, and to optionally generate a default name.
var gvk schema.GroupVersionKind
hasGVK := blder.forInput.object != nil
if hasGVK {
Expand Down
10 changes: 5 additions & 5 deletions pkg/builder/options.go
Expand Up @@ -28,7 +28,7 @@ type ForOption interface {
ApplyToFor(*ForInput)
}

// OwnsOption is some configuration that modifies options for a owns request.
// OwnsOption is some configuration that modifies options for an owns request.
type OwnsOption interface {
// ApplyToOwns applies this configuration to the given owns input.
ApplyToOwns(*OwnsInput)
Expand Down Expand Up @@ -79,8 +79,8 @@ var _ WatchesOption = &Predicates{}

// {{{ For & Owns Dual-Type options

// asProjection configures the projection (currently only metadata) on the input.
// Currently only metadata is supported. We might want to expand
// projectAs configures the projection on the input.
// Currently only OnlyMetadata is supported. We might want to expand
// this to arbitrary non-special local projections in the future.
type projectAs objectProjection

Expand All @@ -101,9 +101,9 @@ func (p projectAs) ApplyToWatches(opts *WatchesInput) {

var (
// OnlyMetadata tells the controller to *only* cache metadata, and to watch
// the API server in metadata-only form. This is useful when watching
// the API server in metadata-only form. This is useful when watching
// lots of objects, really big objects, or objects for which you only know
// the GVK, but not the structure. You'll need to pass
// the GVK, but not the structure. You'll need to pass
// metav1.PartialObjectMetadata to the client when fetching objects in your
// reconciler, otherwise you'll end up with a duplicate structured or
// unstructured cache.
Expand Down
37 changes: 19 additions & 18 deletions pkg/builder/webhook.go
Expand Up @@ -36,17 +36,17 @@ import (

// WebhookBuilder builds a Webhook.
type WebhookBuilder struct {
apiType runtime.Object
withDefaulter admission.CustomDefaulter
withValidator admission.CustomValidator
gvk schema.GroupVersionKind
mgr manager.Manager
config *rest.Config
recoverPanic bool
logConstructor func(base logr.Logger, req *admission.Request) logr.Logger
apiType runtime.Object
customDefaulter admission.CustomDefaulter
customValidator admission.CustomValidator
gvk schema.GroupVersionKind
mgr manager.Manager
config *rest.Config
recoverPanic bool
logConstructor func(base logr.Logger, req *admission.Request) logr.Logger
}

// WebhookManagedBy allows inform its manager.Manager.
// WebhookManagedBy returns a new webhook builder.
func WebhookManagedBy(m manager.Manager) *WebhookBuilder {
return &WebhookBuilder{mgr: m}
}
Expand All @@ -61,15 +61,15 @@ func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder {
return blder
}

// WithDefaulter takes a admission.WithDefaulter interface, a MutatingWebhook will be wired for this type.
// WithDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook will be wired for this type.
func (blder *WebhookBuilder) WithDefaulter(defaulter admission.CustomDefaulter) *WebhookBuilder {
blder.withDefaulter = defaulter
blder.customDefaulter = defaulter
return blder
}

// WithValidator takes a admission.WithValidator interface, a ValidatingWebhook will be wired for this type.
// WithValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type.
func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) *WebhookBuilder {
blder.withValidator = validator
blder.customValidator = validator
return blder
}

Expand All @@ -79,7 +79,7 @@ func (blder *WebhookBuilder) WithLogConstructor(logConstructor func(base logr.Lo
return blder
}

// RecoverPanic indicates whether the panic caused by webhook should be recovered.
// RecoverPanic indicates whether panics caused by the webhook should be recovered.
func (blder *WebhookBuilder) RecoverPanic() *WebhookBuilder {
blder.recoverPanic = true
return blder
Expand Down Expand Up @@ -129,12 +129,12 @@ func (blder *WebhookBuilder) registerWebhooks() error {
return err
}

// Create webhook(s) for each type
blder.gvk, err = apiutil.GVKForObject(typ, blder.mgr.GetScheme())
if err != nil {
return err
}

// Register webhook(s) for type
blder.registerDefaultingWebhook()
blder.registerValidatingWebhook()

Expand All @@ -145,7 +145,7 @@ func (blder *WebhookBuilder) registerWebhooks() error {
return nil
}

// registerDefaultingWebhook registers a defaulting webhook if th.
// registerDefaultingWebhook registers a defaulting webhook if necessary.
func (blder *WebhookBuilder) registerDefaultingWebhook() {
mwh := blder.getDefaultingWebhook()
if mwh != nil {
Expand All @@ -164,7 +164,7 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
}

func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
if defaulter := blder.withDefaulter; defaulter != nil {
if defaulter := blder.customDefaulter; defaulter != nil {
return admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter).WithRecoverPanic(blder.recoverPanic)
}
if defaulter, ok := blder.apiType.(admission.Defaulter); ok {
Expand All @@ -176,6 +176,7 @@ func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
return nil
}

// registerValidatingWebhook registers a validating webhook if necessary.
func (blder *WebhookBuilder) registerValidatingWebhook() {
vwh := blder.getValidatingWebhook()
if vwh != nil {
Expand All @@ -194,7 +195,7 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
}

func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
if validator := blder.withValidator; validator != nil {
if validator := blder.customValidator; validator != nil {
return admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, validator).WithRecoverPanic(blder.recoverPanic)
}
if validator, ok := blder.apiType.(admission.Validator); ok {
Expand Down