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>

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.

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
 			}
 		}
 

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 {

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)

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
+			openErr = err
-			return Zones{}, err
 		}
 
 		for i := range origins {
 			origins[i] = plugin.Host(origins[i]).Normalize()
-			zone, err := Parse(reader, origins[i], fileName, 0)
+			z[origins[i]] = NewZone(origins[i], fileName)
-			if err == nil {
+			if openErr == nil {
-				z[origins[i]] = zone
+				zone, err := Parse(reader, origins[i], fileName, 0)
-			} else {
+				if err == nil {
-				return Zones{}, err
+					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
 }

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

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)