Skip to content

address feedback on PR 48 #63

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

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/cache/internal/informers_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ func (ip *InformersMap) Get(gvk schema.GroupVersionKind, obj runtime.Object) (*M
})
i = &MapEntry{
Informer: ni,
Reader: CacheReader{indexer: ni.GetIndexer(), groupVersionKind: gvk}}
Reader: CacheReader{indexer: ni.GetIndexer(), groupVersionKind: gvk},
}
ip.informersByGVK[gvk] = i

// Start the Informer if need by
Expand Down
18 changes: 9 additions & 9 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ type Controller interface {
//
// Watch may be provided one or more Predicates to filter events before they are given to the EventHandler.
// Events will be passed to the EventHandler iff all provided Predicates evaluate to true.
Watch(src source.Source, evthdler handler.EventHandler, prct ...predicate.Predicate) error
Watch(src source.Source, eventhandler handler.EventHandler, predicates ...predicate.Predicate) error

// Start starts the controller. Start blocks until stop is closed or a controller has an error starting.
Start(stop <-chan struct{}) error
}

// New returns a new Controller registered with the Manager. The Manager will ensure that shared Caches have
// been synced before the Controller is Started.
func New(name string, mrg manager.Manager, options Options) (Controller, error) {
func New(name string, mgr manager.Manager, options Options) (Controller, error) {
if options.Reconciler == nil {
return nil, fmt.Errorf("must specify Reconciler")
}
Expand All @@ -72,23 +72,23 @@ func New(name string, mrg manager.Manager, options Options) (Controller, error)
}

// Inject dependencies into Reconciler
if err := mrg.SetFields(options.Reconciler); err != nil {
if err := mgr.SetFields(options.Reconciler); err != nil {
return nil, err
}

// Create controller with dependencies set
c := &controller.Controller{
Do: options.Reconciler,
Cache: mrg.GetCache(),
Config: mrg.GetConfig(),
Scheme: mrg.GetScheme(),
Client: mrg.GetClient(),
Recorder: mrg.GetRecorder(name),
Cache: mgr.GetCache(),
Config: mgr.GetConfig(),
Scheme: mgr.GetScheme(),
Client: mgr.GetClient(),
Recorder: mgr.GetRecorder(name),
Queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), name),
MaxConcurrentReconciles: options.MaxConcurrentReconciles,
Name: name,
}

// Add the controller as a Manager components
return c, mrg.Add(c)
return c, mgr.Add(c)
}
8 changes: 5 additions & 3 deletions pkg/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ ReplicaSet with controller=true.
* Reconciler contains all of the business logic of a Controller.

* Reconciler typically works on a single object type. - e.g. it will only reconcile ReplicaSets. For separate
types use separate Controllers.
types use separate Controllers. If you wish to trigger reconciles from other objects, you can provide
a mapping (e.g. owner references) that maps the object that triggers the reconcile to the object being reconciled.

* Reconciler is provided the Name / Namespace of the object to reconcile.

Expand Down Expand Up @@ -108,11 +109,12 @@ Predicate
predicate.Predicate is an optional argument to Controller.Watch that filters events. This allows common filters to be
reused and composed.

* Predicate takes and event and returns a bool (true to enqueue)
* Predicate takes an event and returns a bool (true to enqueue)

* Predicates are optional arguments

* Users SHOULD use the provided Predicate implementations, but MAY implement additional Predicates.
* Users SHOULD use the provided Predicate implementations, but MAY implement additional
Predicates e.g. generation changed, label selectors changed etc.

PodController Diagram

Expand Down
2 changes: 1 addition & 1 deletion pkg/handler/enqueue_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (e *EnqueueRequestForOwner) parseOwnerTypeGroupKind(scheme *runtime.Scheme)
}
// Expect only 1 kind. If there is more than one kind this is probably an edge case such as ListOptions.
if len(kinds) != 1 {
err := fmt.Errorf("Expected exactly 1 kind for OwnerType")
err := fmt.Errorf("Expected exactly 1 kind for OwnerType %T, but found %s kinds", e.OwnerType, kinds)
log.Error(err, "", "OwnerType", e.OwnerType, "Kinds", kinds)
return err

Expand Down
8 changes: 4 additions & 4 deletions pkg/internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type Controller struct {
// the Queue for processing
Queue workqueue.RateLimitingInterface

// SetFields is used to SetFields dependencies into other objects such as Sources, EventHandlers and Predicates
// SetFields is used to inject dependencies into other objects such as Sources, EventHandlers and Predicates
SetFields func(i interface{}) error

// mu is used to synchronize Controller setup
Expand Down Expand Up @@ -126,7 +126,7 @@ func (c *Controller) Start(stop <-chan struct{}) error {
c.mu.Lock()
defer c.mu.Unlock()

// TODO)(pwittrock): Reconsider HandleCrash
// TODO(pwittrock): Reconsider HandleCrash
defer utilruntime.HandleCrash()
defer c.Queue.ShutDown()

Expand All @@ -146,10 +146,10 @@ func (c *Controller) Start(stop <-chan struct{}) error {
}

if c.JitterPeriod == 0 {
c.JitterPeriod = time.Second
c.JitterPeriod = 1 * time.Second
}

// Launch two workers to process resources
// Launch workers to process resources
log.Info("Starting workers", "Controller", c.Name, "WorkerCount", c.MaxConcurrentReconciles)
for i := 0; i < c.MaxConcurrentReconciles; i++ {
// Process work items
Expand Down