Do not interrupt querying readiness probes for plugins (#6975)

* Do not interrupt querying readiness probes for plugins

Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>

* Add monitor param for ready plugin

Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>

* Update ready docs

Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>

* Update ready docs

Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>

---------

Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>

plugin/ready/README.md

@@ -8,8 +8,7 @@
 
 By enabling *ready* an HTTP endpoint on port 8181 will return 200 OK, when all plugins that are able
 to signal readiness have done so. If some are not ready yet the endpoint will return a 503 with the
-body containing the list of plugins that are not ready. Once a plugin has signaled it is ready it
+body containing the list of plugins that are not ready.
-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. This also means that the
@@ -19,12 +18,20 @@ their readiness reported as the union of their respective readinesses.
 ## Syntax
 
 ~~~
-ready [ADDRESS]
+ready [ADDRESS] {
+    monitor until-ready|continuously
+}
 ~~~
 
 *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 *and* the list of plugins that are not ready. 
+By default, once a plugin has signaled it is ready it will not be queried again.
+
+The *ready* directive can include an optional `monitor` parameter, defaulting to `until-ready`. The following values are supported:
+
+* `until-ready` - once a plugin signals it is ready, it will not be checked again. This mode assumes stability after the initial readiness confirmation.
+* `continuously` - in this mode, plugins are continuously monitored for readiness. This means a plugin may transition between ready and not ready states, providing real-time status updates.
 
 ## Plugins
 

plugin/ready/list.go

@@ -11,6 +11,12 @@ type list struct {
 	sync.RWMutex
 	rs    []Readiness
 	names []string
+
+	// keepReadiness indicates whether the readiness status of plugins should be retained
+	// after they have been confirmed as ready. When set to false, the plugin readiness
+	// status will be reset to nil to conserve resources, assuming ready plugins don't
+	// need continuous monitoring.
+	keepReadiness bool
 }
 
 // Reset resets l
@@ -40,13 +46,14 @@ func (l *list) Ready() (bool, string) {
 		if r == nil {
 			continue
 		}
-		if !r.Ready() {
+		if r.Ready() {
-			ok = false
+			if !l.keepReadiness {
-			s = append(s, l.names[i])
-		} else {
-			// if ok, this plugin is ready and will not be queried anymore.
 				l.rs[i] = nil
 			}
+			continue
+		}
+		ok = false
+		s = append(s, l.names[i])
 	}
 	if ok {
 		return true, ""

plugin/ready/ready.go

@@ -50,15 +50,15 @@ func (rd *ready) onStartup() error {
 			io.WriteString(w, "Shutting down")
 			return
 		}
-		ok, todo := plugins.Ready()
+		ready, notReadyPlugins := plugins.Ready()
-		if ok {
+		if ready {
 			w.WriteHeader(http.StatusOK)
 			io.WriteString(w, http.StatusText(http.StatusOK))
 			return
 		}
-		log.Infof("Still waiting on: %q", todo)
+		log.Infof("Plugins not ready: %q", notReadyPlugins)
 		w.WriteHeader(http.StatusServiceUnavailable)
-		io.WriteString(w, todo)
+		io.WriteString(w, notReadyPlugins)
 	})
 
 	go func() { http.Serve(rd.ln, rd.mux) }()

plugin/ready/ready_test.go

@@ -67,3 +67,57 @@ func TestReady(t *testing.T) {
 	}
 	response.Body.Close()
 }
+
+func TestReady_Continuously(t *testing.T) {
+	rd := &ready{Addr: ":0"}
+	e := &erratic.Erratic{}
+	plugins.Append(e, "erratic")
+	plugins.keepReadiness = true
+
+	if err := rd.onStartup(); err != nil {
+		t.Fatalf("Unable to startup the readiness server: %v", err)
+	}
+
+	defer rd.onFinalShutdown()
+
+	address := fmt.Sprintf("http://%s/ready", rd.ln.Addr().String())
+
+	response, err := http.Get(address)
+	if err != nil {
+		t.Fatalf("Unable to query %s: %v", address, err)
+	}
+	if response.StatusCode != http.StatusServiceUnavailable {
+		t.Errorf("Invalid status code: expecting %d, got %d", 503, response.StatusCode)
+	}
+	response.Body.Close()
+
+	// make it ready by giving erratic 3 queries.
+	m := new(dns.Msg)
+	m.SetQuestion("example.org.", dns.TypeA)
+	e.ServeDNS(context.TODO(), &test.ResponseWriter{}, m)
+	e.ServeDNS(context.TODO(), &test.ResponseWriter{}, m)
+	e.ServeDNS(context.TODO(), &test.ResponseWriter{}, m)
+
+	response, err = http.Get(address)
+	if err != nil {
+		t.Fatalf("Unable to query %s: %v", address, err)
+	}
+	if response.StatusCode != http.StatusOK {
+		t.Errorf("Invalid status code: expecting %d, got %d", 200, response.StatusCode)
+	}
+	response.Body.Close()
+
+	// make erratic not-ready by giving it more queries, this should change the process readiness
+	e.ServeDNS(context.TODO(), &test.ResponseWriter{}, m)
+	e.ServeDNS(context.TODO(), &test.ResponseWriter{}, m)
+	e.ServeDNS(context.TODO(), &test.ResponseWriter{}, m)
+
+	response, err = http.Get(address)
+	if err != nil {
+		t.Fatalf("Unable to query %s: %v", address, err)
+	}
+	if response.StatusCode != http.StatusServiceUnavailable {
+		t.Errorf("Invalid status code: expecting %d, got %d", 503, response.StatusCode)
+	}
+	response.Body.Close()
+}

plugin/ready/setup.go

@@ -11,10 +11,17 @@ import (
 func init() { plugin.Register("ready", setup) }
 
 func setup(c *caddy.Controller) error {
-	addr, err := parse(c)
+	addr, monType, err := parse(c)
 	if err != nil {
 		return plugin.Error("ready", err)
 	}
+
+	if monType == monitorTypeContinuously {
+		plugins.keepReadiness = true
+	} else {
+		plugins.keepReadiness = false
+	}
+
 	rd := &ready{Addr: addr}
 
 	uniqAddr.Set(addr, rd.onStartup)
@@ -48,12 +55,25 @@ func setup(c *caddy.Controller) error {
 	return nil
 }
 
-func parse(c *caddy.Controller) (string, error) {
+// monitorType represents the type of monitoring behavior for the readiness plugin.
+type monitorType string
+
+const (
+	// monitorTypeUntilReady indicates the monitoring should continue until the system is ready.
+	monitorTypeUntilReady monitorType = "until-ready"
+
+	// monitorTypeContinuously indicates the monitoring should continue indefinitely.
+	monitorTypeContinuously monitorType = "continuously"
+)
+
+func parse(c *caddy.Controller) (string, monitorType, error) {
 	addr := ":8181"
+	monType := monitorTypeUntilReady
+
 	i := 0
 	for c.Next() {
 		if i > 0 {
-			return "", plugin.ErrOnce
+			return "", "", plugin.ErrOnce
 		}
 		i++
 		args := c.RemainingArgs()
@@ -63,11 +83,38 @@ func parse(c *caddy.Controller) (string, error) {
 		case 1:
 			addr = args[0]
 			if _, _, e := net.SplitHostPort(addr); e != nil {
-				return "", e
+				return "", "", e
 			}
 		default:
-			return "", c.ArgErr()
+			return "", "", c.ArgErr()
+		}
+
+		for c.NextBlock() {
+			switch c.Val() {
+			case "monitor":
+				args := c.RemainingArgs()
+				if len(args) != 1 {
+					return "", "", c.ArgErr()
+				}
+
+				var err error
+				monType, err = parseMonitorType(c, args[0])
+				if err != nil {
+					return "", "", err
 				}
 			}
-	return addr, nil
+		}
+	}
+	return addr, monType, nil
+}
+
+func parseMonitorType(c *caddy.Controller, arg string) (monitorType, error) {
+	switch arg {
+	case "until-ready":
+		return monitorTypeUntilReady, nil
+	case "continuously":
+		return monitorTypeContinuously, nil
+	default:
+		return "", c.Errf("monitor type '%s' not supported", arg)
+	}
 }

plugin/ready/setup_test.go

@@ -9,17 +9,81 @@ import (
 func TestSetupReady(t *testing.T) {
 	tests := []struct {
 		input string
+
+		expectedAddr        string
+		expectedMonitorType monitorType
+
 		shouldErr bool
 	}{
-		{`ready`, false},
+		{
-		{`ready localhost:1234`, false},
+			input:               `ready`,
-		{`ready localhost:1234 b`, true},
+			expectedAddr:        ":8181",
-		{`ready bla`, true},
+			expectedMonitorType: monitorTypeUntilReady,
-		{`ready bla bla`, true},
+			shouldErr:           false,
+		},
+		{
+			input:               `ready localhost:1234`,
+			expectedAddr:        "localhost:1234",
+			expectedMonitorType: monitorTypeUntilReady,
+			shouldErr:           false,
+		},
+		{
+			input: `
+ready { 
+	monitor until-ready
+}`,
+			expectedAddr:        ":8181",
+			expectedMonitorType: monitorTypeUntilReady,
+			shouldErr:           false,
+		},
+		{
+			input: `
+ready { 
+	monitor continuously 
+}`,
+			expectedAddr:        ":8181",
+			expectedMonitorType: monitorTypeContinuously,
+			shouldErr:           false,
+		},
+		{
+			input: `
+ready localhost:1234 { 
+	monitor continuously 
+}`,
+			expectedAddr:        "localhost:1234",
+			expectedMonitorType: monitorTypeContinuously,
+			shouldErr:           false,
+		},
+		{
+			input: `
+ready localhost:1234 { 
+	monitor 404 
+}`,
+			shouldErr: true,
+		},
+		{
+			input:     `ready localhost:1234 b`,
+			shouldErr: true,
+		},
+		{
+			input:     `ready bla`,
+			shouldErr: true,
+		},
+		{
+			input:     `ready bla bla`,
+			shouldErr: true,
+		},
 	}
 
 	for i, test := range tests {
-		_, err := parse(caddy.NewTestController("dns", test.input))
+		actualAddress, actualMonitorType, err := parse(caddy.NewTestController("dns", test.input))
+
+		if actualAddress != test.expectedAddr {
+			t.Errorf("Test %d: Expected address %s but found %s for input %s", i, test.expectedAddr, actualAddress, test.input)
+		}
+		if actualMonitorType != test.expectedMonitorType {
+			t.Errorf("Test %d: Expected monitor type %s but found %s for input %s", i, test.expectedMonitorType, actualMonitorType, test.input)
+		}
 
 		if test.shouldErr && err == nil {
 			t.Errorf("Test %d: Expected error but found none for input %s", i, test.input)