plugin/file: track zone mtime for reload_by_mtime (#8556)

* plugin/file: track zone mtime for reload_by_mtime

Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com>

* plugin/file: capture initial mtime before parsing

Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com>

* plugin/file: keep unloaded zones retryable after open error

Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com>

---------

Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com>

plugin/file/reload.go

@@ -21,6 +21,7 @@ func (z *Zone) Reload(t *transfer.Transfer) error {
 			case <-tick.C:
 				zFile := z.File()
 				serial := z.SOASerialIfDefined()
+				var fileMtime time.Time
 
 				if z.ReloadByMtime {
 					fi, err := os.Stat(zFile)
@@ -28,9 +29,13 @@ func (z *Zone) Reload(t *transfer.Transfer) error {
 						log.Errorf("Failed to stat zone %q in %q: %v", z.origin, zFile, err)
 						continue
 					}
-					if !fi.ModTime().After(z.file_mtime) {
+					z.RLock()
+					lastMtime := z.file_mtime
+					z.RUnlock()
+					if !fi.ModTime().After(lastMtime) {
 						continue
 					}
+					fileMtime = fi.ModTime()
 					serial = 0 // force reload of the zone
 				}
 
@@ -50,6 +55,11 @@ func (z *Zone) Reload(t *transfer.Transfer) error {
 				}
 
 				z.setData(zone.Apex, zone.Tree)
+				if z.ReloadByMtime {
+					z.Lock()
+					z.file_mtime = fileMtime
+					z.Unlock()
+				}
 
 				log.Infof("Successfully reloaded zone %q in %q with %d SOA serial", z.origin, zFile, zone.SOA.Serial)
 				if t != nil {

plugin/file/reload_test.go

@@ -307,6 +307,43 @@ func TestZoneReloadByMtime(t *testing.T) {
 			t.Fatalf("Zone should still serve queries after file deletion, got result %d", res)
 		}
 	})
+
+	// Test 5: A successful reload advances the mtime baseline.
+	t.Run("MtimeBaselineAdvancesAfterReload", func(t *testing.T) {
+		z, fileName, cleanup := prepareMtimeZone(t, reloadZoneTest)
+		defer cleanup()
+
+		if err := os.WriteFile(fileName, []byte(reloadZone2Test), 0644); err != nil {
+			t.Fatalf("Failed to write new zone data: %s", err)
+		}
+
+		var rrs []dns.RR
+		for start := time.Now(); time.Since(start) < 2*time.Second; {
+			var err error
+			rrs, err = z.ApexIfDefined()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(rrs) == 3 {
+				break
+			}
+			time.Sleep(2 * time.Millisecond)
+		}
+		if len(rrs) != 3 {
+			t.Fatalf("Expected 3 RRs after reload, got %d", len(rrs))
+		}
+
+		fi, err := os.Stat(fileName)
+		if err != nil {
+			t.Fatalf("Failed to stat zone: %s", err)
+		}
+		z.RLock()
+		mtime := z.file_mtime
+		z.RUnlock()
+		if !mtime.Equal(fi.ModTime()) {
+			t.Fatalf("file mtime = %s after reload, want %s", mtime, fi.ModTime())
+		}
+	})
 }
 
 // prepareMtimeZone creates a zone with mtime-based reload enabled.

plugin/file/setup.go

@@ -2,6 +2,7 @@ package file
 
 import (
 	"errors"
+	"io"
 	"os"
 	"path/filepath"
 	"time"
@@ -69,6 +70,10 @@ func setup(c *caddy.Controller) error {
 }
 
 func fileParse(c *caddy.Controller) (Zones, fall.F, error) {
+	return fileParseWithParser(c, Parse)
+}
+
+func fileParseWithParser(c *caddy.Controller, parse func(io.Reader, string, string, int64) (*Zone, error)) (Zones, fall.F, error) {
 	z := make(map[string]*Zone)
 	names := []string{}
 	fall := fall.F{}
@@ -95,6 +100,12 @@ func fileParse(c *caddy.Controller) (Zones, fall.F, error) {
 		if err != nil {
 			openErr = err
 		}
+		var openedFileInfo os.FileInfo
+		if err == nil {
+			// Capture the version represented by the opened reader, before
+			// parsing can race with a replacement at the same pathname.
+			openedFileInfo, _ = reader.Stat()
+		}
 
 		err = func() error {
 			defer reader.Close()
@@ -103,7 +114,7 @@ func fileParse(c *caddy.Controller) (Zones, fall.F, error) {
 				z[origins[i]] = NewZone(origins[i], fileName)
 				if openErr == nil {
 					reader.Seek(0, 0)
-					zone, err := Parse(reader, origins[i], fileName, 0)
+					zone, err := parse(reader, origins[i], fileName, 0)
 					if err != nil {
 						return err
 					}
@@ -147,6 +158,9 @@ func fileParse(c *caddy.Controller) (Zones, fall.F, error) {
 			z[origins[i]].ReloadInterval = reload
 			z[origins[i]].Upstream = upstream.New()
 			z[origins[i]].ReloadByMtime = reload_by_mtime
+			if reload_by_mtime && openErr == nil && openedFileInfo != nil {
+				z[origins[i]].file_mtime = openedFileInfo.ModTime()
+			}
 		}
 	}
 

plugin/file/setup_test.go

@@ -1,8 +1,10 @@
 package file
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -247,3 +249,131 @@ func TestFileParseRelativeZones(t *testing.T) {
 		}
 	}
 }
+
+func TestFileParseReloadByMtimeInitializesMtime(t *testing.T) {
+	name, rm, err := test.TempFile(".", dbMiekNL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer rm()
+
+	c := caddy.NewTestController("dns", "file "+name+" miek.nl. {\n\treload_by_mtime\n}")
+	zones, _, err := fileParse(c)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fi, err := os.Stat(name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	z := zones.Z["miek.nl."]
+	if z.file_mtime.IsZero() {
+		t.Fatal("file mtime was not initialized for reload_by_mtime")
+	}
+	if !z.file_mtime.Equal(fi.ModTime()) {
+		t.Fatalf("file mtime = %s, want %s", z.file_mtime, fi.ModTime())
+	}
+}
+
+func TestFileParseReloadByMtimeMissingFileLeavesBaselineUnset(t *testing.T) {
+	name := filepath.Join(t.TempDir(), "missing.db")
+	c := caddy.NewTestController("dns", "file "+name+" example.org. {\n\treload_by_mtime\n}")
+	zones, _, err := fileParse(c)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !zones.Z["example.org."].file_mtime.IsZero() {
+		t.Fatal("missing initial file must leave the mtime baseline unset")
+	}
+}
+
+func TestFileParseReloadByMtimeRecoversLaterZoneAfterMissingFile(t *testing.T) {
+	dir := t.TempDir()
+	missing := filepath.Join(dir, "missing.db")
+	existing := filepath.Join(dir, "existing.db")
+	if err := os.WriteFile(existing, []byte(dbMiekNL), 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	corefile := fmt.Sprintf(`file %s example.org. {
+	reload 10ms
+	reload_by_mtime
+}
+file %s miek.nl. {
+	reload 10ms
+	reload_by_mtime
+}`, missing, existing)
+	zones, _, err := fileParse(caddy.NewTestController("dns", corefile))
+	if err != nil {
+		t.Fatal(err)
+	}
+	z := zones.Z["miek.nl."]
+	if z == nil {
+		t.Fatal("later zone was not configured")
+	}
+	if err := z.Reload(nil); err != nil {
+		t.Fatal(err)
+	}
+	defer z.OnShutdown()
+
+	const wantSerial = 1282630057
+	for start := time.Now(); time.Since(start) < 2*time.Second; {
+		if z.SOASerialIfDefined() == wantSerial {
+			return
+		}
+		time.Sleep(2 * time.Millisecond)
+	}
+	t.Fatalf("later zone never loaded: SOA serial = %d, want %d", z.SOASerialIfDefined(), wantSerial)
+}
+
+func TestFileParseReloadByMtimeUsesOpenedFileMtime(t *testing.T) {
+	name, rm, err := test.TempFile(".", dbRelative)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer rm()
+
+	initialMtime := time.Now().Add(-2 * time.Hour).Truncate(time.Second)
+	if err := os.Chtimes(name, initialMtime, initialMtime); err != nil {
+		t.Fatal(err)
+	}
+
+	c := caddy.NewTestController("dns", "file "+name+" example.org. {\n\treload_by_mtime\n}")
+	zones, _, err := fileParseWithParser(c, func(reader io.Reader, origin, fileName string, serial int64) (*Zone, error) {
+		// Simulate the opened reader consuming the old zone before a writer
+		// replaces its contents while the initial load is still in progress.
+		oldContents, err := io.ReadAll(reader)
+		if err != nil {
+			return nil, err
+		}
+		newContents := strings.Replace(dbRelative, "192.0.2.1", "192.0.2.2", 1)
+		if err := os.WriteFile(fileName, []byte(newContents), 0644); err != nil {
+			return nil, err
+		}
+		newMtime := initialMtime.Add(time.Hour)
+		if err := os.Chtimes(fileName, newMtime, newMtime); err != nil {
+			return nil, err
+		}
+		return Parse(bytes.NewReader(oldContents), origin, fileName, serial)
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	z := zones.Z["example.org."]
+	entry, ok := z.Search("foo.example.org.")
+	if !ok || len(entry.Type(dns.TypeA)) != 1 || entry.Type(dns.TypeA)[0].(*dns.A).A.String() != "192.0.2.1" {
+		t.Fatal("initial load did not retain the old zone record")
+	}
+	if !z.file_mtime.Equal(initialMtime) {
+		t.Fatalf("initial mtime = %s, want opened file's %s", z.file_mtime, initialMtime)
+	}
+	fi, err := os.Stat(name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !fi.ModTime().After(z.file_mtime) {
+		t.Fatalf("changed zone mtime %s must be newer than loaded baseline %s", fi.ModTime(), z.file_mtime)
+	}
+}