Configurable zone reload interval in file plugin (#2110)

* Configurable zone reload interval in file plugin

* passing reload config from auto plugin to file plugin. removed noReload property from Zone struct. fixed tests based on short file reload hack

12 files changed, 53 insertions, 20 deletions

diff --git a/plugin/auto/README.md b/plugin/auto/README.md
index 4bb36f359..7e3eedb77 100644
--- a/plugin/auto/README.md
+++ b/plugin/auto/README.md
@@ -16,6 +16,7 @@ zonefile. New or changed zones are automatically picked up from disk.
 ~~~
 auto [ZONES...] {
     directory DIR [REGEXP ORIGIN_TEMPLATE [TIMEOUT]]
+    reload DURATION
     no_reload
     upstream [ADDRESS...]
 }
@@ -31,8 +32,10 @@ are used.
   name `db.example.com`, the extracted origin will be `example.com`. **TIMEOUT** specifies how often
   CoreDNS should scan the directory; the default is every 60 seconds. This value is in seconds.
   The minimum value is 1 second.
-* `no_reload` by default CoreDNS will try to reload a zone every minute and reloads if the
-  SOA's serial has changed. This option disables that behavior.
+* `reload` interval to perform reload of zone if SOA version changes. Default is one minute. 
+  Value of `0` means to not scan for changes and reload. eg. `30s` checks zonefile every 30 seconds 
+  and reloads zone when serial changes.
+* `no_reload` deprecated. Sets reload to 0.
 * `upstream` defines upstream resolvers to be used resolve external names found (think CNAMEs)
   pointing to external names. **ADDRESS** can be an IP address, an IP:port or a string pointing to
   a file that is structured as /etc/resolv.conf. If no **ADDRESS** is given, CoreDNS will resolve CNAMEs
diff --git a/plugin/auto/auto.go b/plugin/auto/auto.go
index 0092e6e00..512a7f812 100644
--- a/plugin/auto/auto.go
+++ b/plugin/auto/auto.go
@@ -32,7 +32,7 @@ type (
 
 		// In the future this should be something like ZoneMeta that contains all this stuff.
 		transferTo []string
-		noReload   bool
+		ReloadInterval time.Duration
 		upstream   upstream.Upstream // Upstream for looking up names during the resolution process.
 
 		duration time.Duration
diff --git a/plugin/auto/setup.go b/plugin/auto/setup.go
index 8791b0738..425b2aace 100644
--- a/plugin/auto/setup.go
+++ b/plugin/auto/setup.go
@@ -144,8 +144,15 @@ func autoParse(c *caddy.Controller) (Auto, error) {
 					a.loader.duration = time.Duration(i) * time.Second
 				}
 
+			case "reload":
+				d, err := time.ParseDuration(c.RemainingArgs()[0])
+				if err != nil {
+					return a, plugin.Error("file", err)
+				}
+				a.loader.ReloadInterval = d
+
 			case "no_reload":
-				a.loader.noReload = true
+				a.loader.ReloadInterval = 0
 
 			case "upstream":
 				args := c.RemainingArgs()
diff --git a/plugin/auto/walk.go b/plugin/auto/walk.go
index 9b60cb9be..92676654e 100644
--- a/plugin/auto/walk.go
+++ b/plugin/auto/walk.go
@@ -52,7 +52,7 @@ func (a Auto) Walk() error {
 			return nil
 		}
 
-		zo.NoReload = a.loader.noReload
+		zo.ReloadInterval = a.loader.ReloadInterval
 		zo.Upstream = a.loader.upstream
 		zo.TransferTo = a.loader.transferTo
 
diff --git a/plugin/file/README.md b/plugin/file/README.md
index 1643251c2..8f7bdbf97 100644
--- a/plugin/file/README.md
+++ b/plugin/file/README.md
@@ -27,6 +27,7 @@ If you want to round robin A and AAAA responses look at the *loadbalance* plugin
 ~~~
 file DBFILE [ZONES... ] {
     transfer to ADDRESS...
+    reload DURATION
     no_reload
     upstream [ADDRESS...]
 }
@@ -36,8 +37,10 @@ file DBFILE [ZONES... ] {
   the direction. **ADDRESS** must be denoted in CIDR notation (127.0.0.1/32 etc.) or just as plain
   addresses. The special wildcard `*` means: the entire internet (only valid for 'transfer to').
   When an address is specified a notify message will be send whenever the zone is reloaded.
-* `no_reload` by default CoreDNS will try to reload a zone every minute and reloads if the
-  SOA's serial has changed. This option disables that behavior.
+* `reload` interval to perform reload of zone if SOA version changes. Default is one minute. 
+  Value of `0` means to not scan for changes and reload. eg. `30s` checks zonefile every 30 seconds 
+  and reloads zone when serial changes.
+* `no_reload` deprecated. Sets reload to 0.
 * `upstream` defines upstream resolvers to be used resolve external names found (think CNAMEs)
   pointing to external names. This is only really useful when CoreDNS is configured as a proxy, for
   normal authoritative serving you don't need *or* want to use this. **ADDRESS** can be an IP
diff --git a/plugin/file/lookup.go b/plugin/file/lookup.go
index 31248f314..d15ff5b6a 100644
--- a/plugin/file/lookup.go
+++ b/plugin/file/lookup.go
@@ -30,11 +30,11 @@ func (z *Zone) Lookup(state request.Request, qname string) ([]dns.RR, []dns.RR,
 	qtype := state.QType()
 	do := state.Do()
 
-	if !z.NoReload {
+	if 0 < z.ReloadInterval {
 		z.reloadMu.RLock()
 	}
 	defer func() {
-		if !z.NoReload {
+		if 0 < z.ReloadInterval {
 			z.reloadMu.RUnlock()
 		}
 	}()
diff --git a/plugin/file/reload.go b/plugin/file/reload.go
index 7c0fe8544..512ea68c9 100644
--- a/plugin/file/reload.go
+++ b/plugin/file/reload.go
@@ -5,15 +5,14 @@ import (
 	"time"
 )
 
-// TickTime is the default time we use to reload zone. Exported to be tweaked in tests.
-var TickTime = 1 * time.Minute
+// TickTime is clock resolution. By default ticks every second. Handler checks if reloadInterval has been reached on every tick.
+var TickTime = 1 * time.Second
 
 // Reload reloads a zone when it is changed on disk. If z.NoRoload is true, no reloading will be done.
 func (z *Zone) Reload() error {
-	if z.NoReload {
+	if z.ReloadInterval == 0 {
 		return nil
 	}
-
 	tick := time.NewTicker(TickTime)
 
 	go func() {
@@ -22,6 +21,13 @@ func (z *Zone) Reload() error {
 			select {
 
 			case <-tick.C:
+				if z.LastReloaded.Add(z.ReloadInterval).After(time.Now()) {
+					//reload interval not reached yet
+					continue
+				}
+				//saving timestamp of last attempted reload
+				z.LastReloaded = time.Now()
+
 				zFile := z.File()
 				reader, err := os.Open(zFile)
 				if err != nil {
diff --git a/plugin/file/reload_test.go b/plugin/file/reload_test.go
index e98153662..2f7b6cf0f 100644
--- a/plugin/file/reload_test.go
+++ b/plugin/file/reload_test.go
@@ -29,6 +29,7 @@ func TestZoneReload(t *testing.T) {
 	}
 
 	TickTime = 500 * time.Millisecond
+	z.ReloadInterval = 500 * time.Millisecond
 	z.Reload()
 	time.Sleep(time.Second)
 
diff --git a/plugin/file/setup.go b/plugin/file/setup.go
index 457ea5348..86eef6b96 100644
--- a/plugin/file/setup.go
+++ b/plugin/file/setup.go
@@ -3,6 +3,7 @@ package file
 import (
 	"os"
 	"path"
+	"time"
 
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin"
@@ -91,7 +92,7 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 			names = append(names, origins[i])
 		}
 
-		noReload := false
+		reload := 1 * time.Minute
 		upstr := upstream.Upstream{}
 		t := []string{}
 		var e error
@@ -104,8 +105,15 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 					return Zones{}, e
 				}
 
+			case "reload":
+				d, err := time.ParseDuration(c.RemainingArgs()[0])
+				if err != nil {
+					return Zones{}, plugin.Error("file", err)
+				}
+				reload = d
+
 			case "no_reload":
-				noReload = true
+				reload = 0
 
 			case "upstream":
 				args := c.RemainingArgs()
@@ -122,7 +130,7 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 				if t != nil {
 					z[origin].TransferTo = append(z[origin].TransferTo, t...)
 				}
-				z[origin].NoReload = noReload
+				z[origin].ReloadInterval = reload
 				z[origin].Upstream = upstr
 			}
 		}
diff --git a/plugin/file/shutdown.go b/plugin/file/shutdown.go
index cecd76e9e..9aa598948 100644
--- a/plugin/file/shutdown.go
+++ b/plugin/file/shutdown.go
@@ -2,7 +2,7 @@ package file
 
 // OnShutdown shuts down any running go-routines for this zone.
 func (z *Zone) OnShutdown() error {
-	if !z.NoReload {
+	if 0 < z.ReloadInterval {
 		z.reloadShutdown <- true
 	}
 	return nil
diff --git a/plugin/file/zone.go b/plugin/file/zone.go
index 5aa72175f..ecfd74faa 100644
--- a/plugin/file/zone.go
+++ b/plugin/file/zone.go
@@ -6,6 +6,7 @@ import (
 	"path"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/coredns/coredns/plugin/file/tree"
 	"github.com/coredns/coredns/plugin/pkg/upstream"
@@ -27,7 +28,8 @@ type Zone struct {
 	TransferFrom []string
 	Expired      *bool
 
-	NoReload       bool
+	ReloadInterval time.Duration
+	LastReloaded   time.Time
 	reloadMu       sync.RWMutex
 	reloadShutdown chan bool
 	Upstream       upstream.Upstream // Upstream for looking up names during the resolution process
@@ -50,6 +52,7 @@ func NewZone(name, file string) *Zone {
 		Tree:           &tree.Tree{},
 		Expired:        new(bool),
 		reloadShutdown: make(chan bool),
+		LastReloaded:   time.Now(),
 	}
 	*z.Expired = false
 
@@ -161,7 +164,7 @@ func (z *Zone) TransferAllowed(state request.Request) bool {
 // All returns all records from the zone, the first record will be the SOA record,
 // otionally followed by all RRSIG(SOA)s.
 func (z *Zone) All() []dns.RR {
-	if !z.NoReload {
+	if z.ReloadInterval > 0 {
 		z.reloadMu.RLock()
 		defer z.reloadMu.RUnlock()
 	}
diff --git a/test/file_reload_test.go b/test/file_reload_test.go
index 01877f693..2f07d9a8e 100644
--- a/test/file_reload_test.go
+++ b/test/file_reload_test.go
@@ -24,7 +24,9 @@ func TestZoneReload(t *testing.T) {
 
 	// Corefile with two stanzas
 	corefile := `example.org:0 {
-       file ` + name + `
+       file ` + name + ` {
+           reload 1s
+       }
 }
 
 example.net:0 {