plugin/file: make non-existent file non-fatal (#2955)

* plugin/file: make non-existent file non-fatal

If the zone file being loaded doesn't exist *and* reload is enabled,
just wait the file to pop up in the normal Reload routine.

If reload is set to 0s; we keep this a fatal error on startup. Aslo fix
the ticker in z.Reload(): remove the per second ticks and just use the
reload interval for the ticker.

Brush up the documentation a bit as well.

Fixes: #2951

Signed-off-by: Miek Gieben <miek@miek.nl>

* Stickler and test compile

Signed-off-by: Miek Gieben <miek@miek.nl>

* Remove there too

Signed-off-by: Miek Gieben <miek@miek.nl>

* Cant README test these because zone files dont exist

Signed-off-by: Miek Gieben <miek@miek.nl>

7 files changed, 37 insertions, 34 deletions

diff --git a/plugin/file/README.md b/plugin/file/README.md
index b8f9b50d5..5322a8ac6 100644
--- a/plugin/file/README.md
+++ b/plugin/file/README.md
@@ -6,7 +6,7 @@
 
 ## Description
 
-The file plugin is used for an "old-style" DNS server. It serves from a preloaded file that exists
+The *file* plugin is used for an "old-style" DNS server. It serves from a preloaded file that exists
 on disk. If the zone file contains signatures (i.e., is signed using DNSSEC), correct DNSSEC answers
 are returned. Only NSEC is supported! If you use this setup *you* are responsible for re-signing the
 zonefile.
@@ -44,7 +44,7 @@ file DBFILE [ZONES... ] {
 Load the `example.org` zone from `example.org.signed` and allow transfers to the internet, but send
 notifies to 10.240.1.1
 
-~~~ corefile
+~~~ txt
 example.org {
     file example.org.signed {
         transfer to *
@@ -55,7 +55,7 @@ example.org {
 
 Or use a single zone file for multiple zones:
 
-~~~
+~~~ txt
 . {
     file example.org.signed example.org example.net {
         transfer to *
@@ -67,7 +67,7 @@ Or use a single zone file for multiple zones:
 Note that if you have a configuration like the following you may run into a problem of the origin
 not being correctly recognized:
 
-~~~
+~~~ txt
 . {
     file db.example.org
 }
@@ -78,7 +78,7 @@ which, in this case, is the root zone. Any contents of `db.example.org` will the
 origin set; this may or may not do what you want.
 It's better to be explicit here and specify the correct origin. This can be done in two ways:
 
-~~~
+~~~ txt
 . {
     file db.example.org example.org
 }
@@ -86,8 +86,12 @@ It's better to be explicit here and specify the correct origin. This can be done
 
 Or
 
-~~~
+~~~ txt
 example.org {
     file db.example.org
 }
 ~~~
+
+## Also See
+
+See the *loadbalance* plugin if you need simple record shuffling.
diff --git a/plugin/file/file.go b/plugin/file/file.go
index 2fe4b1644..bc582cfaa 100644
--- a/plugin/file/file.go
+++ b/plugin/file/file.go
@@ -129,12 +129,15 @@ func Parse(f io.Reader, origin, fileName string, serial int64) (*Zone, error) {
 			return nil, err
 		}
 
-		if !seenSOA && serial >= 0 {
+		if !seenSOA {
 			if s, ok := rr.(*dns.SOA); ok {
-				if s.Serial == uint32(serial) { // same serial
+				seenSOA = true
+
+				// -1 is valid serial is we failed to load the file on startup.
+
+				if serial >= 0 && s.Serial == uint32(serial) { // same serial
 					return nil, &serialErr{err: "no change in SOA serial", origin: origin, zone: fileName, serial: serial}
 				}
-				seenSOA = true
 			}
 		}
 
diff --git a/plugin/file/reload.go b/plugin/file/reload.go
index e73c5b87d..ce5c81335 100644
--- a/plugin/file/reload.go
+++ b/plugin/file/reload.go
@@ -5,15 +5,12 @@ import (
 	"time"
 )
 
-// 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.NoReload is true, no reloading will be done.
 func (z *Zone) Reload() error {
 	if z.ReloadInterval == 0 {
 		return nil
 	}
-	tick := time.NewTicker(TickTime)
+	tick := time.NewTicker(z.ReloadInterval)
 
 	go func() {
 
@@ -21,13 +18,6 @@ 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 1139b8a44..196565cac 100644
--- a/plugin/file/reload_test.go
+++ b/plugin/file/reload_test.go
@@ -29,7 +29,6 @@ func TestZoneReload(t *testing.T) {
 		t.Fatalf("Failed to parse zone: %s", err)
 	}
 
-	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 38ba79621..9be09fe8d 100644
--- a/plugin/file/setup.go
+++ b/plugin/file/setup.go
@@ -57,6 +57,9 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 
 	config := dnsserver.GetConfig(c)
 
+	var openErr error
+	reload := 1 * time.Minute
+
 	for c.Next() {
 		// file db.file [zones...]
 		if !c.NextArg() {
@@ -77,22 +80,23 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 
 		reader, err := os.Open(fileName)
 		if err != nil {
-			// bail out
-			return Zones{}, err
+			openErr = err
 		}
 
 		for i := range origins {
 			origins[i] = plugin.Host(origins[i]).Normalize()
-			zone, err := Parse(reader, origins[i], fileName, 0)
-			if err == nil {
-				z[origins[i]] = zone
-			} else {
-				return Zones{}, err
+			z[origins[i]] = NewZone(origins[i], fileName)
+			if openErr == nil {
+				zone, err := Parse(reader, origins[i], fileName, 0)
+				if err == nil {
+					z[origins[i]] = zone
+				} else {
+					return Zones{}, err
+				}
 			}
 			names = append(names, origins[i])
 		}
 
-		reload := 1 * time.Minute
 		upstr := upstream.New()
 		t := []string{}
 		var e error
@@ -129,5 +133,13 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 			}
 		}
 	}
+	if openErr != nil {
+		if reload == 0 {
+			// reload hasn't been set make this a fatal error
+			return Zones{}, plugin.Error("file", openErr)
+		}
+		log.Warningf("Failed to open %q: trying again in %s", openErr, reload)
+
+	}
 	return Zones{Z: z, Names: names}, nil
 }
diff --git a/plugin/file/zone.go b/plugin/file/zone.go
index b24cd91e8..186fdf8d0 100644
--- a/plugin/file/zone.go
+++ b/plugin/file/zone.go
@@ -30,7 +30,6 @@ type Zone struct {
 	Expired      *bool
 
 	ReloadInterval time.Duration
-	LastReloaded   time.Time
 	reloadMu       sync.RWMutex
 	reloadShutdown chan bool
 	Upstream       *upstream.Upstream // Upstream for looking up external names during the resolution process.
@@ -53,7 +52,6 @@ func NewZone(name, file string) *Zone {
 		Tree:           &tree.Tree{},
 		Expired:        new(bool),
 		reloadShutdown: make(chan bool),
-		LastReloaded:   time.Now(),
 	}
 	*z.Expired = false
 	return z
diff --git a/test/file_reload_test.go b/test/file_reload_test.go
index 91372ecc3..e61003b8b 100644
--- a/test/file_reload_test.go
+++ b/test/file_reload_test.go
@@ -5,15 +5,12 @@ import (
 	"testing"
 	"time"
 
-	"github.com/coredns/coredns/plugin/file"
 	"github.com/coredns/coredns/plugin/test"
 
 	"github.com/miekg/dns"
 )
 
 func TestZoneReload(t *testing.T) {
-	file.TickTime = 1 * time.Second
-
 	name, rm, err := test.TempFile(".", exampleOrg)
 	if err != nil {
 		t.Fatalf("Failed to create zone: %s", err)