Skip to content

Commit

Permalink
TODOs around stopping informers
Browse files Browse the repository at this point in the history
  • Loading branch information
jbeda committed Nov 4, 2016
1 parent 453bb17 commit e0c6bf1
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
9 changes: 7 additions & 2 deletions pkg/client/cache/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
type Config struct {
// The queue for your objects; either a FIFO or
// a DeltaFIFO. Your Process() function should accept
// the output of this Oueue's Pop() method.
// the output of this Queue's Pop() method.
Queue

// Something that can list and watch your objects.
Expand Down Expand Up @@ -121,6 +121,11 @@ func (c *Controller) Requeue(obj interface{}) error {
// TODO: Consider doing the processing in parallel. This will require a little thought
// to make sure that we don't end up processing the same object multiple times
// concurrently.
//
// TODO: Plumb through the stopCh here (and down to the queue) so that this can
// actually exit when the controller is stopped. Or just give up on this stuff
// ever being stoppable. Converting this whole package to use Context would
// also be helpful.
func (c *Controller) processLoop() {
for {
obj, err := c.config.Queue.Pop(PopProcessFunc(c.config.Process))
Expand All @@ -134,7 +139,7 @@ func (c *Controller) processLoop() {
}

// ResourceEventHandler can handle notifications for events that happen to a
// resource. The events are informational only, so you can't return an
// resource. The events are informational only, so you can't return an
// error.
// * OnAdd is called when an object is added.
// * OnUpdate is called when an object is modified. Note that oldObj is the
Expand Down
3 changes: 3 additions & 0 deletions pkg/client/cache/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,9 @@ func TestHammerController(t *testing.T) {
time.Sleep(100 * time.Millisecond)
close(stop)

// TODO: Verify that no goroutines were leaked here and that everything shut
// down cleanly.

outputSetLock.Lock()
t.Logf("got: %#v", outputSet)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/client/cache/reflector.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import (

// Reflector watches a specified resource and causes all changes to be reflected in the given store.
type Reflector struct {
// name identifies this reflector. By default it will be a file:line if possible.
// name identifies this reflector. By default it will be a file:line if possible.
name string

// The type of object we expect to place in the store.
Expand Down Expand Up @@ -108,7 +108,7 @@ func NewNamedReflector(name string, lw ListerWatcher, expectedType interface{},
return r
}

// internalPackages are packages that ignored when creating a default reflector name. These packages are in the common
// internalPackages are packages that ignored when creating a default reflector name. These packages are in the common
// call chains to NewReflector, so they'd be low entropy names for reflectors
var internalPackages = []string{"kubernetes/pkg/client/cache/", "/runtime/asm_"}

Expand Down

0 comments on commit e0c6bf1

Please sign in to comment.