add goroutine to check hosts file for updates (#1180)

* add goroutine to check hosts file for updates

* rename parseFile to parseReader, remove extra error check

plugin/hosts/README.md

@@ -17,7 +17,8 @@ hosts [FILE [ZONES...]] {
 ~~~
 
 * **FILE** the hosts file to read and parse. If the path is relative the path from the *root*
-  directive will be prepended to it. Defaults to /etc/hosts if omitted
+  directive will be prepended to it. Defaults to /etc/hosts if omitted. We scan the file for changes 
+  every 5 seconds.
 * **ZONES** zones it should be authoritative for. If empty, the zones from the configuration block
    are used.
 * **INLINE** the hosts file contents inlined in Corefile. If there are any lines before fallthrough

plugin/hosts/hosts_test.go

@@ -3,7 +3,6 @@ package hosts
 import (
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	"github.com/coredns/coredns/plugin/test"
@@ -13,8 +12,8 @@ import (
 )
 
 func TestLookupA(t *testing.T) {
-	h := Hosts{Next: test.ErrorHandler(), Hostsfile: &Hostsfile{expire: time.Now().Add(1 * time.Hour), Origins: []string{"."}}}
-	h.Parse(strings.NewReader(hostsExample))
+	h := Hosts{Next: test.ErrorHandler(), Hostsfile: &Hostsfile{Origins: []string{"."}}}
+	h.parseReader(strings.NewReader(hostsExample))
 
 	ctx := context.TODO()
 

plugin/hosts/hostsfile.go

@@ -19,8 +19,6 @@ import (
 	"github.com/coredns/coredns/plugin"
 )
 
-const cacheMaxAge = 5 * time.Second
-
 func parseLiteralIP(addr string) net.IP {
 	if i := strings.Index(addr, "%"); i >= 0 {
 		// discard ipv6 zone
@@ -34,13 +32,7 @@ func absDomainName(b string) string {
 	return plugin.Name(b).Normalize()
 }
 
-// Hostsfile contains known host entries.
-type Hostsfile struct {
-	sync.RWMutex
-
-	// list of zones we are authoritive for
-	Origins []string
-
+type hostsMap struct {
 	// Key for the list of literal IP addresses must be a host
 	// name. It would be part of DNS labels, a FQDN or an absolute
 	// FQDN.
@@ -52,71 +44,80 @@ type Hostsfile struct {
 	// including IPv6 address with zone identifier.
 	// We don't support old-classful IP address notation.
 	byAddr map[string][]string
+}
 
-	// inline saves the hosts file is inlined in Corefile
-	// We need a copy here as we want to use inline to override
-	// the default /etc/hosts
-	inline []string
+func newHostsMap() *hostsMap {
+	return &hostsMap{
+		byNameV4: make(map[string][]net.IP),
+		byNameV6: make(map[string][]net.IP),
+		byAddr:   make(map[string][]string),
+	}
+}
 
-	expire time.Time
+// Hostsfile contains known host entries.
+type Hostsfile struct {
+	sync.RWMutex
+
+	// list of zones we are authoritive for
+	Origins []string
+
+	// hosts maps for lookups
+	hmap *hostsMap
+
+	// inline saves the hosts file that is inlined in a Corefile.
+	// We need a copy here as we want to use it to initialize the maps for parse.
+	inline *hostsMap
+
+	// path to the hosts file
 	path string
+
+	// mtime and size are only read and modified by a single goroutine
 	mtime time.Time
 	size  int64
 }
 
-// ReadHosts determines if the cached data needs to be updated based on the size and modification time of the hostsfile.
-func (h *Hostsfile) ReadHosts() {
-	now := time.Now()
-
-	// Not deferring h.RUnlock() because we may need to remove the read lock and aquire a write lock
-	h.RLock()
-	if now.Before(h.expire) && len(h.byAddr) > 0 {
-		h.RUnlock()
-		return
-	}
-	stat, err := os.Stat(h.path)
-	if err == nil && h.mtime.Equal(stat.ModTime()) && h.size == stat.Size() {
-		h.RUnlock()
-		h.Lock()
-		h.expire = now.Add(cacheMaxAge)
-		h.Unlock()
-		return
-	}
-
-	var file *os.File
-	if file, _ = os.Open(h.path); file == nil {
-		// If this is the first time then we will try to parse inline
-		if len(h.byAddr) == 0 && len(h.inline) > 0 {
-			h.Parse(nil)
-		}
-		h.RUnlock()
+// readHosts determines if the cached data needs to be updated based on the size and modification time of the hostsfile.
+func (h *Hostsfile) readHosts() {
+	file, err := os.Open(h.path)
+	if err != nil {
+		// We already log a warning if the file doesn't exist or can't be opened on setup. No need to return the error here.
 		return
 	}
 	defer file.Close()
 
-	h.RUnlock()
+	stat, err := file.Stat()
+	if err == nil && h.mtime.Equal(stat.ModTime()) && h.size == stat.Size() {
+		return
+	}
+
 	h.Lock()
 	defer h.Unlock()
-	h.Parse(file)
+	h.parseReader(file)
 
 	// Update the data cache.
-	h.expire = now.Add(cacheMaxAge)
 	h.mtime = stat.ModTime()
 	h.size = stat.Size()
 }
 
-// Parse reads the hostsfile and populates the byName and byAddr maps.
-func (h *Hostsfile) Parse(file io.Reader) {
-	hsv4 := make(map[string][]net.IP)
-	hsv6 := make(map[string][]net.IP)
-	is := make(map[string][]string)
-
-	var readers []io.Reader
-	if file != nil {
-		readers = append(readers, file)
+func (h *Hostsfile) initInline(inline []string) {
+	if len(inline) == 0 {
+		return
 	}
-	readers = append(readers, strings.NewReader(strings.Join(h.inline, "\n")))
-	scanner := bufio.NewScanner(io.MultiReader(readers...))
+
+	hmap := newHostsMap()
+	h.inline = h.parse(strings.NewReader(strings.Join(inline, "\n")), hmap)
+	*h.hmap = *h.inline
+}
+
+func (h *Hostsfile) parseReader(r io.Reader) {
+	h.hmap = h.parse(r, h.inline)
+}
+
+// Parse reads the hostsfile and populates the byName and byAddr maps.
+func (h *Hostsfile) parse(r io.Reader, override *hostsMap) *hostsMap {
+	hmap := newHostsMap()
+
+	scanner := bufio.NewScanner(r)
 	for scanner.Scan() {
 		line := scanner.Bytes()
 		if i := bytes.Index(line, []byte{'#'}); i >= 0 {
@@ -140,18 +141,31 @@ func (h *Hostsfile) Parse(file io.Reader) {
 			}
 			switch ver {
 			case 4:
-				hsv4[name] = append(hsv4[name], addr)
+				hmap.byNameV4[name] = append(hmap.byNameV4[name], addr)
 			case 6:
-				hsv6[name] = append(hsv6[name], addr)
+				hmap.byNameV6[name] = append(hmap.byNameV6[name], addr)
 			default:
 				continue
 			}
-			is[addr.String()] = append(is[addr.String()], name)
+			hmap.byAddr[addr.String()] = append(hmap.byAddr[addr.String()], name)
 		}
 	}
-	h.byNameV4 = hsv4
-	h.byNameV6 = hsv6
-	h.byAddr = is
+
+	if override == nil {
+		return hmap
+	}
+
+	for name := range override.byNameV4 {
+		hmap.byNameV4[name] = append(hmap.byNameV4[name], override.byNameV4[name]...)
+	}
+	for name := range override.byNameV4 {
+		hmap.byNameV6[name] = append(hmap.byNameV6[name], override.byNameV6[name]...)
+	}
+	for addr := range override.byAddr {
+		hmap.byAddr[addr] = append(hmap.byAddr[addr], override.byAddr[addr]...)
+	}
+
+	return hmap
 }
 
 // ipVersion returns what IP version was used textually
@@ -169,11 +183,10 @@ func ipVersion(s string) int {
 
 // LookupStaticHostV4 looks up the IPv4 addresses for the given host from the hosts file.
 func (h *Hostsfile) LookupStaticHostV4(host string) []net.IP {
-	h.ReadHosts()
 	h.RLock()
 	defer h.RUnlock()
-	if len(h.byNameV4) != 0 {
-		if ips, ok := h.byNameV4[absDomainName(host)]; ok {
+	if len(h.hmap.byNameV4) != 0 {
+		if ips, ok := h.hmap.byNameV4[absDomainName(host)]; ok {
 			ipsCp := make([]net.IP, len(ips))
 			copy(ipsCp, ips)
 			return ipsCp
@@ -184,11 +197,10 @@ func (h *Hostsfile) LookupStaticHostV4(host string) []net.IP {
 
 // LookupStaticHostV6 looks up the IPv6 addresses for the given host from the hosts file.
 func (h *Hostsfile) LookupStaticHostV6(host string) []net.IP {
-	h.ReadHosts()
 	h.RLock()
 	defer h.RUnlock()
-	if len(h.byNameV6) != 0 {
-		if ips, ok := h.byNameV6[absDomainName(host)]; ok {
+	if len(h.hmap.byNameV6) != 0 {
+		if ips, ok := h.hmap.byNameV6[absDomainName(host)]; ok {
 			ipsCp := make([]net.IP, len(ips))
 			copy(ipsCp, ips)
 			return ipsCp
@@ -199,15 +211,14 @@ func (h *Hostsfile) LookupStaticHostV6(host string) []net.IP {
 
 // LookupStaticAddr looks up the hosts for the given address from the hosts file.
 func (h *Hostsfile) LookupStaticAddr(addr string) []string {
-	h.ReadHosts()
 	h.RLock()
 	defer h.RUnlock()
 	addr = parseLiteralIP(addr).String()
 	if addr == "" {
 		return nil
 	}
-	if len(h.byAddr) != 0 {
-		if hosts, ok := h.byAddr[addr]; ok {
+	if len(h.hmap.byAddr) != 0 {
+		if hosts, ok := h.hmap.byAddr[addr]; ok {
 			hostsCp := make([]string, len(hosts))
 			copy(hostsCp, hosts)
 			return hostsCp

plugin/hosts/hostsfile_test.go

@@ -9,12 +9,11 @@ import (
 	"reflect"
 	"strings"
 	"testing"
-	"time"
 )
 
 func testHostsfile(file string) *Hostsfile {
-	h := &Hostsfile{expire: time.Now().Add(1 * time.Hour), Origins: []string{"."}}
-	h.Parse(strings.NewReader(file))
+	h := &Hostsfile{Origins: []string{"."}}
+	h.parseReader(strings.NewReader(file))
 	return h
 }
 

plugin/hosts/setup.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path"
 	"strings"
+	"time"
 
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin"
@@ -25,6 +26,30 @@ func setup(c *caddy.Controller) error {
 		return plugin.Error("hosts", err)
 	}
 
+	parseChan := make(chan bool)
+
+	c.OnStartup(func() error {
+		h.readHosts()
+
+		go func() {
+			ticker := time.NewTicker(5 * time.Second)
+			for {
+				select {
+				case <-parseChan:
+					return
+				case <-ticker.C:
+					h.readHosts()
+				}
+			}
+		}()
+		return nil
+	})
+
+	c.OnShutdown(func() error {
+		close(parseChan)
+		return nil
+	})
+
 	dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler {
 		h.Next = next
 		return h
@@ -35,12 +60,15 @@ func setup(c *caddy.Controller) error {
 
 func hostsParse(c *caddy.Controller) (Hosts, error) {
 	var h = Hosts{
-		Hostsfile: &Hostsfile{path: "/etc/hosts"},
+		Hostsfile: &Hostsfile{
+			path: "/etc/hosts",
+			hmap: newHostsMap(),
+		},
 	}
-	defer h.ReadHosts()
 
 	config := dnsserver.GetConfig(c)
 
+	inline := []string{}
 	for c.Next() {
 		args := c.RemainingArgs()
 		if len(args) >= 1 {
@@ -86,12 +114,15 @@ func hostsParse(c *caddy.Controller) (Hosts, error) {
 			default:
 				if !h.Fallthrough {
 					line := strings.Join(append([]string{c.Val()}, c.RemainingArgs()...), " ")
-					h.inline = append(h.inline, line)
+					inline = append(inline, line)
 					continue
 				}
 				return h, c.Errf("unknown property '%s'", c.Val())
 			}
 		}
 	}
+
+	h.initInline(inline)
+
 	return h, nil
 }

plugin/hosts/setup_test.go

@@ -141,7 +141,7 @@ func TestHostsInlineParse(t *testing.T) {
 				t.Fatalf("Test %d expected fallthrough of %v, got %v", i, test.expectedFallthrough, h.Fallthrough)
 			}
 			for k, expectedVal := range test.expectedbyAddr {
-				if val, ok := h.byAddr[k]; !ok {
+				if val, ok := h.hmap.byAddr[k]; !ok {
 					t.Fatalf("Test %d expected %v, got no entry", i, k)
 				} else {
 					if len(expectedVal) != len(val) {