middleware/file: add test for reload (#361)

This add a highlevel integration test for zone reloading. It also
fixes a data race in the actual reloading process.

middleware/file/lookup.go

@@ -26,13 +26,31 @@ const (
 // Three sets of records are returned, one for the answer, one for authority  and one for the additional section.
 func (z *Zone) Lookup(qname string, qtype uint16, do bool) ([]dns.RR, []dns.RR, []dns.RR, Result) {
 	if qtype == dns.TypeSOA {
+		if !z.NoReload {
+			z.reloadMu.RLock()
+		}
 		return z.lookupSOA(do)
+		if !z.NoReload {
+			z.reloadMu.RUnlock()
+		}
 	}
 	if qtype == dns.TypeNS && qname == z.origin {
+		if !z.NoReload {
+			z.reloadMu.RLock()
+		}
 		return z.lookupNS(do)
+		if !z.NoReload {
+			z.reloadMu.RUnlock()
+		}
 	}
 
+	if !z.NoReload {
+		z.reloadMu.RLock()
+	}
 	elem, res := z.Tree.Search(qname, qtype)
+	if !z.NoReload {
+		z.reloadMu.RUnlock()
+	}
 	if elem == nil {
 		if res == tree.EmptyNonTerminal {
 			return z.emptyNonTerminal(qname, do)

middleware/file/zone.go

@@ -163,22 +163,24 @@ func (z *Zone) Reload() error {
 			select {
 			case event := <-watcher.Events:
 				if event.Op == fsnotify.Write && path.Clean(event.Name) == z.file {
+
 					reader, err := os.Open(z.file)
 					if err != nil {
 						log.Printf("[ERROR] Failed to open `%s' for `%s': %v", z.file, z.origin, err)
 						continue
 					}
-					z.reloadMu.Lock()
 					zone, err := Parse(reader, z.origin, z.file)
 					if err != nil {
 						log.Printf("[ERROR] Failed to parse `%s': %v", z.origin, err)
-						z.reloadMu.Unlock()
 						continue
 					}
+
 					// copy elements we need
+					z.reloadMu.Lock()
 					z.Apex = zone.Apex
 					z.Tree = zone.Tree
 					z.reloadMu.Unlock()
+
 					log.Printf("[INFO] Successfully reloaded zone `%s'", z.origin)
 					z.Notify()
 				}

test/file_reload_test.go

@@ -0,0 +1,75 @@
+package test
+
+import (
+	"io/ioutil"
+	"log"
+	"testing"
+	"time"
+
+	"github.com/miekg/coredns/middleware/proxy"
+	"github.com/miekg/coredns/middleware/test"
+	"github.com/miekg/coredns/request"
+	"github.com/miekg/dns"
+)
+
+func TestZoneReload(t *testing.T) {
+	log.SetOutput(ioutil.Discard)
+
+	name, rm, err := TempFile(".", exampleOrg)
+	if err != nil {
+		t.Fatalf("Failed to created zone: %s", err)
+	}
+	defer rm()
+
+	// Corefile with two stanzas
+	corefile := `example.org:0 {
+       file ` + name + `
+}
+
+example.net:0 {
+	file ` + name + `
+}
+`
+	i, err := CoreDNSServer(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+
+	udp, _ := CoreDNSServerPorts(i, 0)
+	if udp == "" {
+		t.Fatalf("Could not get UDP listening port")
+	}
+	defer i.Stop()
+
+	p := proxy.New([]string{udp})
+	state := request.Request{W: &test.ResponseWriter{}, Req: new(dns.Msg)}
+
+	resp, err := p.Lookup(state, "example.org.", dns.TypeA)
+	if err != nil {
+		t.Fatal("Expected to receive reply, but didn't")
+	}
+	if len(resp.Answer) != 2 {
+		t.Fatal("Expected two RR in answer section got %d", len(resp.Answer))
+	}
+
+	// Remove RR from the Apex
+	ioutil.WriteFile(name, []byte(exampleOrgUpdated), 0644)
+
+	time.Sleep(1 * time.Second) // fsnotify
+
+	resp, err = p.Lookup(state, "example.org.", dns.TypeA)
+	if err != nil {
+		t.Fatal("Expected to receive reply, but didn't")
+	}
+
+	if len(resp.Answer) != 1 {
+		t.Fatal("Expected two RR in answer section got %d", len(resp.Answer))
+	}
+}
+
+const exampleOrgUpdated = `; example.org test file
+example.org.		IN	SOA	sns.dns.icann.org. noc.dns.icann.org. 2016082541 7200 3600 1209600 3600
+example.org.		IN	NS	b.iana-servers.net.
+example.org.		IN	NS	a.iana-servers.net.
+example.org.		IN	A	127.0.0.2
+`