plugin/health: make reload work (#1585)

* plugin/health: make reload work

Remove the once.Do from the startup, so we can re-bind the HTTP
listener. Also clarify the usage of health in multiple server blocks
(this is not the best approach - but there isn't a generic solution at
this point).

Manual tested as we lack testing infra, i.e kill -SIGUSR1 and some
CURLing of the health endpoint.

* Readme test fix

* update

* dont need this

plugin/health/README.md

@@ -8,8 +8,7 @@
 
 By enabling *health* any plugin that implements
 [healt.Healther interface](https://godoc.org/github.com/coredns/coredns/plugin/health#Healther)
-will be queried for it's health. The combined
+will be queried for it's health. The combined health is exported, by default, on port 8080/health .
-health is exported, by default, on port 8080/health .
 
 ## Syntax
 
@@ -20,8 +19,8 @@ health [ADDRESS]
 Optionally takes an address; the default is `:8080`. The health path is fixed to `/health`. The
 health endpoint returns a 200 response code and the word "OK" when CoreDNS is healthy. It returns
 a 503. *health* periodically (1s) polls plugin that exports health information. If any of the
-plugin signals that it is unhealthy, the server will go unhealthy too. Each plugin that
+plugin signals that it is unhealthy, the server will go unhealthy too. Each plugin that supports
-supports health checks has a section "Health" in their README.
+health checks has a section "Health" in their README.
 
 More options can be set with this extended syntax:
 
@@ -34,6 +33,21 @@ health [ADDRESS] {
 * Where `lameduck` will make the process unhealthy then *wait* for **DURATION** before the process
   shuts down.
 
+If you have multiple Server Block and need to export health for each of the plugins, you must run
+health endpoints on different ports:
+
+~~~ corefile
+com {
+    whoami
+    health :8080
+}
+
+net {
+    erratic
+    health :8081
+}
+~~~
+
 ## Plugins
 
 Any plugin that implements the Healther interface will be used to report health.
@@ -60,7 +74,7 @@ Set a lameduck duration of 1 second:
 
 ~~~ corefile
 . {
-    health localhost:8091 {
+    health localhost:8092 {
         lameduck 1s
     }
 }

plugin/health/health.go

@@ -10,8 +10,6 @@ import (
 	"time"
 )
 
-var once sync.Once
-
 // Health implements healthchecks by polling plugins.
 type health struct {
 	Addr     string
@@ -39,33 +37,26 @@ func (h *health) OnStartup() error {
 		h.Addr = defAddr
 	}
 
-	once.Do(func() {
+	ln, err := net.Listen("tcp", h.Addr)
-		ln, err := net.Listen("tcp", h.Addr)
+	if err != nil {
-		if err != nil {
+		return err
-			log.Printf("[ERROR] Failed to start health handler: %s", err)
+	}
+
+	h.ln = ln
+	h.mux = http.NewServeMux()
+
+	h.mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) {
+		if h.Ok() {
+			w.WriteHeader(http.StatusOK)
+			io.WriteString(w, ok)
 			return
 		}
-
+		w.WriteHeader(http.StatusServiceUnavailable)
-		h.ln = ln
-
-		h.mux = http.NewServeMux()
-
-		h.mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) {
-			if h.Ok() {
-				w.WriteHeader(http.StatusOK)
-				io.WriteString(w, ok)
-				return
-			}
-			w.WriteHeader(http.StatusServiceUnavailable)
-		})
-
-		go func() {
-			http.Serve(h.ln, h.mux)
-		}()
-		go func() {
-			h.overloaded()
-		}()
 	})
+
+	go func() { http.Serve(h.ln, h.mux) }()
+	go func() { h.overloaded() }()
+
 	return nil
 }
 

plugin/health/setup.go

@@ -68,7 +68,7 @@ func setup(c *caddy.Controller) error {
 	})
 
 	c.OnStartup(h.OnStartup)
-	c.OnFinalShutdown(h.OnShutdown)
+	c.OnShutdown(h.OnShutdown)
 
 	// Don't do AddPlugin, as health is not *really* a plugin just a separate webserver running.
 	return nil