plugin/forward using pkg/up (#1493)

* plugin/forward: on demand healtchecking

Only start doing health checks when we encouner an error (any error).
This uses the new pluing/pkg/up package to abstract away the actual
checking. This reduces the LOC quite a bit; does need more testing, unit
testing and tcpdumping a bit.

* fix tests

* Fix readme

* Use pkg/up for healthchecks

* remove unused channel

* more cleanups

* update readme

* * Again do go generate and go build; still referencing the wrong forward
  repo? Anyway fixed.
* Use pkg/up for doing the healtchecks to cut back on unwanted queries
  * Change up.Func to return an error instead of a boolean.
  * Drop the string target argument as it doesn't make sense.
* Add healthcheck test on failing to get an upstream answer.

TODO(miek): double check Forward and Lookup and how they interact with
HC, and if we correctly call close() on those

* actual test

* Tests here

* more tests

* try getting rid of host

* Get rid of the host indirection

* Finish removing hosts

* moar testing

* import fmt

* field is not used

* docs

* move some stuff

* bring back health_check

* maxfails=0 test

* git and merging, bah

* review

2 files changed, 10 insertions, 9 deletions

diff --git a/plugin/pkg/up/up.go b/plugin/pkg/up/up.go
index af8de5ed5..06971eef7 100644
--- a/plugin/pkg/up/up.go
+++ b/plugin/pkg/up/up.go
@@ -17,8 +17,8 @@ type Probe struct {
 	inprogress bool
 }
 
-// Func is used to determine if a target is alive. If so this function must return true.
-type Func func(target string) bool
+// Func is used to determine if a target is alive. If so this function must return nil.
+type Func func() error
 
 // New returns a pointer to an intialized Probe.
 func New() *Probe {
@@ -32,9 +32,9 @@ func (p *Probe) Do(f Func) { p.do <- f }
 func (p *Probe) Stop() { p.stop <- true }
 
 // Start will start the probe manager, after which probes can be initialized with Do.
-func (p *Probe) Start(target string, interval time.Duration) { go p.start(target, interval) }
+func (p *Probe) Start(interval time.Duration) { go p.start(interval) }
 
-func (p *Probe) start(target string, interval time.Duration) {
+func (p *Probe) start(interval time.Duration) {
 	for {
 		select {
 		case <-p.stop:
@@ -52,9 +52,10 @@ func (p *Probe) start(target string, interval time.Duration) {
 			// we return from the goroutine and we can accept another Func to run.
 			go func() {
 				for {
-					if ok := f(target); ok {
+					if err := f(); err == nil {
 						break
 					}
+					// TODO(miek): little bit of exponential backoff here?
 					time.Sleep(interval)
 				}
 				p.Lock()
diff --git a/plugin/pkg/up/up_test.go b/plugin/pkg/up/up_test.go
index cb56658d1..eeaecea3b 100644
--- a/plugin/pkg/up/up_test.go
+++ b/plugin/pkg/up/up_test.go
@@ -12,20 +12,20 @@ func TestUp(t *testing.T) {
 	wg := sync.WaitGroup{}
 	hits := int32(0)
 
-	upfunc := func(s string) bool {
+	upfunc := func() error {
 		atomic.AddInt32(&hits, 1)
 		// Sleep tiny amount so that our other pr.Do() calls hit the lock.
 		time.Sleep(3 * time.Millisecond)
 		wg.Done()
-		return true
+		return nil
 	}
 
-	pr.Start("nonexistent", 5*time.Millisecond)
+	pr.Start(5 * time.Millisecond)
 	defer pr.Stop()
 
 	// These functions AddInt32 to the same hits variable, but we only want to wait when
 	// upfunc finishes, as that only calls Done() on the waitgroup.
-	upfuncNoWg := func(s string) bool { atomic.AddInt32(&hits, 1); return true }
+	upfuncNoWg := func() error { atomic.AddInt32(&hits, 1); return nil }
 	wg.Add(1)
 	pr.Do(upfunc)
 	pr.Do(upfuncNoWg)