address feedback on PR 48

#48

Author: droot

pkg/cache/internal/informers_map.go

@@ -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

pkg/controller/controller.go

@@ -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")
 	}
@@ -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)
 }

pkg/doc.go

@@ -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.
 
@@ -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
 

pkg/handler/enqueue_owner.go

@@ -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
 

pkg/internal/controller/controller.go

@@ -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
@@ -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()
 
@@ -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