From a1c97f82a6f27ae1f4489a1dc5d0bc6fa4cce9ed Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Sun, 9 Jun 2019 08:10:15 +0100
Subject: [PATCH] plugin/ready: fix starts and restarts (#2814)

Add OnRestartFailed to the ready plugin and some various cleanups.

Document slightly better how things are supposed to work with multiple
`ready`'s in the multiple Server Blocks.

All manually tested with this Corefile:
~~~
. {
    log
    ready
}

example.org {
    log
    chaos
    ready
}
~~~
And then `kill -SIGUSR1` and curling the ready endpoint. This works
well, the FailedReload is triggered by adding a syntax error in the
Corefile.

See #2659

Signed-off-by: Miek Gieben <miek@miek.nl>
---
 plugin/pkg/uniq/uniq.go |  2 +-
 plugin/ready/README.md  |  6 ++++--
 plugin/ready/ready.go   |  8 --------
 plugin/ready/setup.go   | 27 +++++++++++++++------------
 4 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/plugin/pkg/uniq/uniq.go b/plugin/pkg/uniq/uniq.go
index c3fdb5211..5f95e41d2 100644
--- a/plugin/pkg/uniq/uniq.go
+++ b/plugin/pkg/uniq/uniq.go
@@ -28,7 +28,7 @@ func (u U) Unset(key string) {
 	delete(u.u, key)
 }
 
-// ForEach iterates for u executes f for each element that is 'todo' and sets it to 'done'.
+// ForEach iterates over u and executes f for each element that is 'todo' and sets it to 'done'.
 func (u U) ForEach() error {
 	for k, v := range u.u {
 		if v.state == todo {
diff --git a/plugin/ready/README.md b/plugin/ready/README.md
index f5aa3cc22..d4a9bc5fe 100644
--- a/plugin/ready/README.md
+++ b/plugin/ready/README.md
@@ -12,7 +12,9 @@ body containing the list of plugins that are not ready. Once a plugin has signal
 will not be queried again.
 
 Each Server Block that enables the *ready* plugin will have the plugins *in that server block*
-report readiness into the /ready endpoint that runs on the same port.
+report readiness into the /ready endpoint that runs on the same port. This also means that the
+*same* plugin with different configurations (in potentialy *different* Server Blocks) will have
+their readiness reported as the union of their respective readinesses.
 
 ## Syntax
 
@@ -22,7 +24,7 @@ ready [ADDRESS]
 
 *ready* optionally takes an address; the default is `:8181`. The path is fixed to `/ready`. The
 readiness endpoint returns a 200 response code and the word "OK" when this server is ready. It
-returns a 503 otherwise.
+returns a 503 otherwise *and* the list of plugins that are not ready.
 
 ## Plugins
 
diff --git a/plugin/ready/ready.go b/plugin/ready/ready.go
index 692f3f81d..ff19b59f8 100644
--- a/plugin/ready/ready.go
+++ b/plugin/ready/ready.go
@@ -30,10 +30,6 @@ type ready struct {
 }
 
 func (rd *ready) onStartup() error {
-	if rd.Addr == "" {
-		rd.Addr = defAddr
-	}
-
 	ln, err := net.Listen("tcp", rd.Addr)
 	if err != nil {
 		return err
@@ -62,8 +58,6 @@ func (rd *ready) onStartup() error {
 	return nil
 }
 
-func (rd *ready) onRestart() error { return rd.onFinalShutdown() }
-
 func (rd *ready) onFinalShutdown() error {
 	rd.Lock()
 	defer rd.Unlock()
@@ -77,5 +71,3 @@ func (rd *ready) onFinalShutdown() error {
 	rd.done = false
 	return nil
 }
-
-const defAddr = ":8181"
diff --git a/plugin/ready/setup.go b/plugin/ready/setup.go
index 64967dbd4..7cf286b3d 100644
--- a/plugin/ready/setup.go
+++ b/plugin/ready/setup.go
@@ -21,22 +21,25 @@ func setup(c *caddy.Controller) error {
 	if err != nil {
 		return plugin.Error("ready", err)
 	}
-
 	rd := &ready{Addr: addr}
 
 	uniqAddr.Set(addr, rd.onStartup)
+	c.OnStartup(func() error { uniqAddr.Set(addr, rd.onStartup); return nil })
+	c.OnRestartFailed(func() error { uniqAddr.Set(addr, rd.onStartup); return nil })
 
-	c.OncePerServerBlock(func() error {
-		c.OnStartup(func() error {
-			return uniqAddr.ForEach()
-		})
-		return nil
-	})
+	c.OnStartup(func() error { return uniqAddr.ForEach() })
+	c.OnRestartFailed(func() error { return uniqAddr.ForEach() })
 
 	c.OnStartup(func() error {
-		// Each plugin in this server block will (if they support it) report readiness.
-		plugs := dnsserver.GetConfig(c).Handlers()
-		for _, p := range plugs {
+		for _, p := range dnsserver.GetConfig(c).Handlers() {
+			if r, ok := p.(Readiness); ok {
+				plugins.Append(r, p.Name())
+			}
+		}
+		return nil
+	})
+	c.OnRestartFailed(func() error {
+		for _, p := range dnsserver.GetConfig(c).Handlers() {
 			if r, ok := p.(Readiness); ok {
 				plugins.Append(r, p.Name())
 			}
@@ -44,14 +47,14 @@ func setup(c *caddy.Controller) error {
 		return nil
 	})
 
-	c.OnRestart(rd.onRestart)
+	c.OnRestart(rd.onFinalShutdown)
 	c.OnFinalShutdown(rd.onFinalShutdown)
 
 	return nil
 }
 
 func parse(c *caddy.Controller) (string, error) {
-	addr := ""
+	addr := ":8181"
 	i := 0
 	for c.Next() {
 		if i > 0 {