diff --git a/plugin/ready/list.go b/plugin/ready/list.go index 49999a89b..0a98eb03d 100644 --- a/plugin/ready/list.go +++ b/plugin/ready/list.go @@ -9,8 +9,7 @@ import ( // list is a structure that holds the plugins that signals readiness for this server block. type list struct { sync.RWMutex - rs []Readiness - names []string + rs map[string]Readiness // 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 @@ -24,36 +23,38 @@ func (l *list) Reset() { l.Lock() defer l.Unlock() l.rs = nil - l.names = nil } // Append adds a new readiness to l. func (l *list) Append(r Readiness, name string) { l.Lock() defer l.Unlock() - l.rs = append(l.rs, r) - l.names = append(l.names, name) + + if l.rs == nil { + l.rs = make(map[string]Readiness) + } + l.rs[name] = r } // Ready return true when all plugins ready, if the returned value is false the string // contains a comma separated list of plugins that are not ready. func (l *list) Ready() (bool, string) { - l.RLock() - defer l.RUnlock() + l.Lock() + defer l.Unlock() ok := true s := []string{} - for i, r := range l.rs { + for name, r := range l.rs { if r == nil { continue } if r.Ready() { if !l.keepReadiness { - l.rs[i] = nil + l.rs[name] = nil } continue } ok = false - s = append(s, l.names[i]) + s = append(s, name) } if ok { return true, "" diff --git a/plugin/ready/setup.go b/plugin/ready/setup.go index 5e3b4233b..a7bf2c620 100644 --- a/plugin/ready/setup.go +++ b/plugin/ready/setup.go @@ -11,6 +11,7 @@ import ( func init() { plugin.Register("ready", setup) } func setup(c *caddy.Controller) error { + plugins.Reset() addr, monType, err := parse(c) if err != nil { return plugin.Error("ready", err) @@ -32,7 +33,6 @@ func setup(c *caddy.Controller) error { c.OnRestartFailed(func() error { return uniqAddr.ForEach() }) c.OnStartup(func() error { - plugins.Reset() for _, p := range dnsserver.GetConfig(c).Handlers() { if r, ok := p.(Readiness); ok { plugins.Append(r, p.Name()) diff --git a/test/reload_test.go b/test/reload_test.go index 35edaec0f..18e165e39 100644 --- a/test/reload_test.go +++ b/test/reload_test.go @@ -342,6 +342,7 @@ func TestReloadUnreadyPlugin(t *testing.T) { // Add/Register a perpetually unready plugin dnsserver.Directives = append([]string{"unready"}, dnsserver.Directives...) u := new(unready) + u.name = "unready" plugin.Register("unready", func(c *caddy.Controller) error { dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler { u.next = next @@ -381,6 +382,66 @@ func TestReloadUnreadyPlugin(t *testing.T) { c1.Stop() } +// TestReloadTwoServerBlocksUnreadyPlugin tests that the ready plugin properly resets the list of readiness implementors during a reload +// when there are multiple server blocks. +func TestReloadTwoServerBlocksUnreadyPlugin(t *testing.T) { + // Add/Register a perpetually unready plugin + dnsserver.Directives = append([]string{"unready1", "unready2"}, dnsserver.Directives...) + u1 := new(unready) + u2 := new(unready) + u1.name = "unready1" + u2.name = "unready2" + plugin.Register("unready1", func(c *caddy.Controller) error { + dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler { + u1.next = next + return u1 + }) + return nil + }) + plugin.Register("unready2", func(c *caddy.Controller) error { + dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler { + u2.next = next + return u2 + }) + return nil + }) + corefile := `cluster.local.:0 { + unready1 + whoami + ready 127.0.0.1:53185 + } + .:0 { + unready2 + whoami + ready 127.0.0.1:53185 + }` + + coreInput := NewInput(corefile) + + c, err := CoreDNSServer(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + + c1, err := c.Restart(coreInput) + if err != nil { + t.Fatal(err) + } + + resp, err := http.Get("http://127.0.0.1:53185/ready") + if err != nil { + t.Fatal(err) + } + bod, _ := io.ReadAll(resp.Body) + resp.Body.Close() + uName := u1.Name() + "," + u2.Name() + if string(bod) != uName { + t.Errorf("Expected /ready endpoint response body %q, got %q", uName, bod) + } + + c1.Stop() +} + // TestReloadConcurrentRestartAndStop ensures there is no deadlock when a restart // races with a shutdown (issue #7314). func TestReloadConcurrentRestartAndStop(t *testing.T) { @@ -436,11 +497,12 @@ func TestReloadConcurrentRestartAndStop(t *testing.T) { type unready struct { next plugin.Handler + name string } func (u *unready) Ready() bool { return false } -func (u *unready) Name() string { return "unready" } +func (u *unready) Name() string { return u.name } func (u *unready) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { return u.next.ServeDNS(ctx, w, r)