middleware/cache: split cache in positive and negative and use lru (#298)

Make the cache memory bounded, by using a LRU cache. Also split the
cache in a positive and negative one - each with its own controls.

Extend the cache stanza to allow for this:

    cache {
       positive limit [ttl]
       negative limit [ttl]
    }

is now possible. This also add a cache_test.go in the toplevel test/
directory that exercises the caching path.

Fixes #260

middleware/cache/README.md

@@ -4,20 +4,32 @@
 
 ## Syntax
 
-~~~
+~~~ txt
 cache [ttl] [zones...]
 ~~~
 
-* `ttl` max TTL in seconds. If not specified, the TTL of the reply (SOA minimum or minimum TTL in the
-  answer section) will be used.
+* `ttl` max TTL in seconds. If not specified, the maximum TTL will be used which is 1 hours for
+    positive responses and half an hour for negative ones.
 * `zones` zones it should cache for. If empty, the zones from the configuration block are used.
 
-Each element in the cache is cached according to its TTL. For the negative cache, the SOA's MinTTL
-value is used.
+Each element in the cache is cached according to its TTL (with `ttl` as the max).
+For the negative cache, the SOA's MinTTL value is used. A cache can contain up to 10,000 items by
+default.
+
+Or if you want more control:
+
+~~~ txt
+cache [ttl] [zones...] {
+    postive capacity [ttl]
+    negative capacity [ttl]
+}
+~~~
 
-A cache mostly makes sense with a middleware that is potentially slow (e.g., a proxy that retrieves an
-answer), or to minimize backend queries for middleware like etcd. Using a cache with the file
-middleware essentially doubles the memory load with no conceivable increase of query speed.
+* `ttl`  and `zones` as above.
+* `positive`, override the settings for caching positive responses, capacity indicates the maximum
+  number of packets we cache before we start evicting (LRU). Ttl overrides the cache maximum TTL.
+* `negative`, override the settings for caching negative responses, capacity indicates the maximum
+  number of packets we cache before we start evicting (LRU). Ttl overrides the cache maximum TTL.
 
 The minimum TTL allowed on resource records is 5 seconds.
 

middleware/cache/cache.go

@@ -2,61 +2,58 @@ package cache
 
 import (
 	"log"
+	"strconv"
 	"strings"
 	"time"
 
 	"github.com/miekg/coredns/middleware"
 	"github.com/miekg/coredns/middleware/pkg/response"
 
+	"github.com/hashicorp/golang-lru"
 	"github.com/miekg/dns"
-	gcache "github.com/patrickmn/go-cache"
 )
 
 // Cache is middleware that looks up responses in a cache and caches replies.
+// It has a positive and a negative cache.
 type Cache struct {
 	Next  middleware.Handler
 	Zones []string
-	cache *gcache.Cache
-	cap   time.Duration
-}
 
-// NewCache returns a new cache.
-func NewCache(ttl int, zones []string, next middleware.Handler) Cache {
-	return Cache{Next: next, Zones: zones, cache: gcache.New(defaultDuration, purgeDuration), cap: time.Duration(ttl) * time.Second}
+	ncache *lru.Cache
+	ncap   int
+	nttl   time.Duration
+
+	pcache *lru.Cache
+	pcap   int
+	pttl   time.Duration
 }
 
-func cacheKey(m *dns.Msg, t response.Type, do bool) string {
+// Return key under which we store the item.
+func key(m *dns.Msg, t response.Type, do bool) string {
 	if m.Truncated {
+		// TODO(miek): wise to store truncated responses?
+		return ""
+	}
+	if t == response.OtherError {
 		return ""
 	}
 
 	qtype := m.Question[0].Qtype
 	qname := strings.ToLower(m.Question[0].Name)
-	switch t {
-	case response.Success:
-		fallthrough
-	case response.Delegation:
-		return successKey(qname, qtype, do)
-	case response.NameError:
-		return nameErrorKey(qname, do)
-	case response.NoData:
-		return noDataKey(qname, qtype, do)
-	case response.OtherError:
-		return ""
+	return rawKey(qname, qtype, do)
+}
+
+func rawKey(qname string, qtype uint16, do bool) string {
+	if do {
+		return "1" + qname + "." + strconv.Itoa(int(qtype))
 	}
-	return ""
+	return "0" + qname + "." + strconv.Itoa(int(qtype))
 }
 
 // ResponseWriter is a response writer that caches the reply message.
 type ResponseWriter struct {
 	dns.ResponseWriter
-	cache *gcache.Cache
-	cap   time.Duration
-}
-
-// NewCachingResponseWriter returns a new ResponseWriter.
-func NewCachingResponseWriter(w dns.ResponseWriter, cache *gcache.Cache, cap time.Duration) *ResponseWriter {
-	return &ResponseWriter{w, cache, cap}
+	*Cache
 }
 
 // WriteMsg implements the dns.ResponseWriter interface.
@@ -67,42 +64,47 @@ func (c *ResponseWriter) WriteMsg(res *dns.Msg) error {
 		do = opt.Do()
 	}
 
-	key := cacheKey(res, mt, do)
-	c.set(res, key, mt)
+	key := key(res, mt, do)
+
+	duration := c.pttl
+	if mt == response.NameError || mt == response.NoData {
+		duration = c.nttl
+	}
 
-	if c.cap != 0 {
-		setCap(res, uint32(c.cap.Seconds()))
+	msgTTL := minMsgTTL(res, mt)
+	if msgTTL < duration {
+		duration = msgTTL
 	}
 
+	if key != "" {
+		c.set(res, key, mt, duration)
+	}
+
+	setMsgTTL(res, uint32(duration.Seconds()))
+
 	return c.ResponseWriter.WriteMsg(res)
 }
 
-func (c *ResponseWriter) set(m *dns.Msg, key string, mt response.Type) {
+func (c *ResponseWriter) set(m *dns.Msg, key string, mt response.Type, duration time.Duration) {
 	if key == "" {
 		log.Printf("[ERROR] Caching called with empty cache key")
 		return
 	}
 
-	duration := c.cap
 	switch mt {
 	case response.Success, response.Delegation:
-		if c.cap == 0 {
-			duration = minTTL(m.Answer, mt)
-		}
 		i := newItem(m, duration)
+		c.pcache.Add(key, i)
 
-		c.cache.Set(key, i, duration)
 	case response.NameError, response.NoData:
-		if c.cap == 0 {
-			duration = minTTL(m.Ns, mt)
-		}
 		i := newItem(m, duration)
+		c.ncache.Add(key, i)
 
-		c.cache.Set(key, i, duration)
 	case response.OtherError:
 		// don't cache these
+		// TODO(miek): what do we do with these?
 	default:
-		log.Printf("[WARNING] Caching called with unknown middleware MsgType: %d", mt)
+		log.Printf("[WARNING] Caching called with unknown classification: %d", mt)
 	}
 }
 
@@ -113,36 +115,10 @@ func (c *ResponseWriter) Write(buf []byte) (int, error) {
 	return n, err
 }
 
-// Hijack implements the dns.ResponseWriter interface.
-func (c *ResponseWriter) Hijack() {
-	c.ResponseWriter.Hijack()
-	return
-}
-
-func minTTL(rrs []dns.RR, mt response.Type) time.Duration {
-	if mt != response.Success && mt != response.NameError && mt != response.NoData {
-		return 0
-	}
-
-	minTTL := maxTTL
-	for _, r := range rrs {
-		switch mt {
-		case response.NameError, response.NoData:
-			if r.Header().Rrtype == dns.TypeSOA {
-				return time.Duration(r.(*dns.SOA).Minttl) * time.Second
-			}
-		case response.Success, response.Delegation:
-			if r.Header().Ttl < minTTL {
-				minTTL = r.Header().Ttl
-			}
-		}
-	}
-	return time.Duration(minTTL) * time.Second
-}
-
 const (
-	purgeDuration          = 1 * time.Minute
-	defaultDuration        = 20 * time.Minute
-	baseTTL                = 5 // minimum TTL that we will allow
-	maxTTL          uint32 = 2 * 3600
+	maxTTL  = 1 * time.Hour
+	maxNTTL = 30 * time.Minute
+	minTTL  = 5 * time.Second
+
+	defaultCap = 10000 // default capacity of the cache.
 )

middleware/cache/cache_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/miekg/coredns/middleware/pkg/response"
 	"github.com/miekg/coredns/middleware/test"
 
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/miekg/dns"
 )
 
@@ -26,21 +27,15 @@ var cacheTestCases = []cacheTestCase{
 		Case: test.Case{
 			Qname: "miek.nl.", Qtype: dns.TypeMX,
 			Answer: []dns.RR{
-				test.MX("miek.nl.	1800	IN	MX	1 aspmx.l.google.com."),
-				test.MX("miek.nl.	1800	IN	MX	10 aspmx2.googlemail.com."),
-				test.MX("miek.nl.	1800	IN	MX	10 aspmx3.googlemail.com."),
-				test.MX("miek.nl.	1800	IN	MX	5 alt1.aspmx.l.google.com."),
-				test.MX("miek.nl.	1800	IN	MX	5 alt2.aspmx.l.google.com."),
+				test.MX("miek.nl.	3600	IN	MX	1 aspmx.l.google.com."),
+				test.MX("miek.nl.	3600	IN	MX	10 aspmx2.googlemail.com."),
 			},
 		},
 		in: test.Case{
 			Qname: "miek.nl.", Qtype: dns.TypeMX,
 			Answer: []dns.RR{
-				test.MX("miek.nl.	1800	IN	MX	1 aspmx.l.google.com."),
-				test.MX("miek.nl.	1800	IN	MX	10 aspmx2.googlemail.com."),
-				test.MX("miek.nl.	1800	IN	MX	10 aspmx3.googlemail.com."),
-				test.MX("miek.nl.	1800	IN	MX	5 alt1.aspmx.l.google.com."),
-				test.MX("miek.nl.	1800	IN	MX	5 alt2.aspmx.l.google.com."),
+				test.MX("miek.nl.	3601	IN	MX	1 aspmx.l.google.com."),
+				test.MX("miek.nl.	3601	IN	MX	10 aspmx2.googlemail.com."),
 			},
 		},
 	},
@@ -65,14 +60,17 @@ func cacheMsg(m *dns.Msg, tc cacheTestCase) *dns.Msg {
 	return m
 }
 
-func newTestCache() (Cache, *ResponseWriter) {
-	c := NewCache(0, []string{"."}, nil)
-	crr := NewCachingResponseWriter(nil, c.cache, time.Duration(0))
+func newTestCache(ttl time.Duration) (*Cache, *ResponseWriter) {
+	c := &Cache{Zones: []string{"."}, pcap: defaultCap, ncap: defaultCap, pttl: ttl, nttl: ttl}
+	c.pcache, _ = lru.New(c.pcap)
+	c.ncache, _ = lru.New(c.ncap)
+
+	crr := &ResponseWriter{nil, c}
 	return c, crr
 }
 
 func TestCache(t *testing.T) {
-	c, crr := newTestCache()
+	c, crr := newTestCache(maxTTL)
 
 	for _, tc := range cacheTestCases {
 		m := tc.in.Msg()
@@ -80,14 +78,15 @@ func TestCache(t *testing.T) {
 		do := tc.in.Do
 
 		mt, _ := response.Classify(m)
-		key := cacheKey(m, mt, do)
-		crr.set(m, key, mt)
+		k := key(m, mt, do)
+		crr.set(m, k, mt, c.pttl)
 
 		name := middleware.Name(m.Question[0].Name).Normalize()
 		qtype := m.Question[0].Qtype
-		i, ok := c.get(name, qtype, do)
-		if !ok && !m.Truncated {
+		i, ok, _ := c.get(name, qtype, do)
+		if ok && m.Truncated {
 			t.Errorf("Truncated message should not have been cached")
+			continue
 		}
 
 		if ok {

middleware/cache/handler.go

 package cache
 
 import (
+	"time"
+
 	"github.com/miekg/coredns/middleware"
 	"github.com/miekg/coredns/request"
 
@@ -10,7 +12,7 @@ import (
 )
 
 // ServeDNS implements the middleware.Handler interface.
-func (c Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 	state := request.Request{W: w, Req: r}
 
 	qname := state.Name()
@@ -20,34 +22,36 @@ func (c Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 		return c.Next.ServeDNS(ctx, w, r)
 	}
 
-	do := state.Do() // might need more from OPT record?
+	do := state.Do() // might need more from OPT record? Like the actual bufsize?
+
+	if i, ok, expired := c.get(qname, qtype, do); ok && !expired {
 
-	if i, ok := c.get(qname, qtype, do); ok {
 		resp := i.toMsg(r)
 		state.SizeAndDo(resp)
 		w.WriteMsg(resp)
 
 		cacheHitCount.WithLabelValues(zone).Inc()
+
 		return dns.RcodeSuccess, nil
 	}
+
 	cacheMissCount.WithLabelValues(zone).Inc()
 
-	crr := NewCachingResponseWriter(w, c.cache, c.cap)
+	crr := &ResponseWriter{w, c}
 	return c.Next.ServeDNS(ctx, crr, r)
 }
 
-func (c Cache) get(qname string, qtype uint16, do bool) (*item, bool) {
-	nxdomain := nameErrorKey(qname, do)
-	if i, ok := c.cache.Get(nxdomain); ok {
-		return i.(*item), true
+func (c *Cache) get(qname string, qtype uint16, do bool) (*item, bool, bool) {
+	k := rawKey(qname, qtype, do)
+
+	if i, ok := c.ncache.Get(k); ok {
+		return i.(*item), ok, i.(*item).expired(time.Now())
 	}
 
-	// TODO(miek): delegation was added double check
-	successOrNoData := successKey(qname, qtype, do)
-	if i, ok := c.cache.Get(successOrNoData); ok {
-		return i.(*item), true
+	if i, ok := c.pcache.Get(k); ok {
+		return i.(*item), ok, i.(*item).expired(time.Now())
 	}
-	return nil, false
+	return nil, false, false
 }
 
 var (

middleware/cache/item.go

 package cache
 
 import (
-	"strconv"
 	"time"
 
+	"github.com/miekg/coredns/middleware/pkg/response"
 	"github.com/miekg/dns"
 )
 
@@ -44,7 +44,7 @@ func newItem(m *dns.Msg, d time.Duration) *item {
 	return i
 }
 
-// toMsg turns i into a message, it tailers to reply to m.
+// toMsg turns i into a message, it tailers the reply to m.
 func (i *item) toMsg(m *dns.Msg) *dns.Msg {
 	m1 := new(dns.Msg)
 	m1.SetReply(m)
@@ -58,44 +58,51 @@ func (i *item) toMsg(m *dns.Msg) *dns.Msg {
 	m1.Extra = i.Extra
 
 	ttl := int(i.origTTL) - int(time.Now().UTC().Sub(i.stored).Seconds())
-	if ttl < baseTTL {
-		ttl = baseTTL
+	if ttl < int(minTTL.Seconds()) {
+		ttl = int(minTTL.Seconds())
 	}
-	setCap(m1, uint32(ttl))
+	setMsgTTL(m1, uint32(ttl))
 	return m1
 }
 
-// setCap sets the ttl on all RRs in all sections.
-func setCap(m *dns.Msg, ttl uint32) {
+func (i *item) expired(now time.Time) bool {
+	ttl := int(i.origTTL) - int(now.UTC().Sub(i.stored).Seconds())
+	return ttl < 0
+}
+
+// setMsgTTL sets the ttl on all RRs in all sections.
+func setMsgTTL(m *dns.Msg, ttl uint32) {
 	for _, r := range m.Answer {
-		r.Header().Ttl = uint32(ttl)
+		r.Header().Ttl = ttl
 	}
 	for _, r := range m.Ns {
-		r.Header().Ttl = uint32(ttl)
+		r.Header().Ttl = ttl
 	}
 	for _, r := range m.Extra {
 		if r.Header().Rrtype == dns.TypeOPT {
 			continue
 		}
-		r.Header().Ttl = uint32(ttl)
+		r.Header().Ttl = ttl
 	}
 }
 
-// nodataKey returns a caching key for NODATA responses.
-func noDataKey(qname string, qtype uint16, do bool) string {
-	if do {
-		return "1" + qname + ".." + strconv.Itoa(int(qtype))
+func minMsgTTL(m *dns.Msg, mt response.Type) time.Duration {
+	if mt != response.Success && mt != response.NameError && mt != response.NoData {
+		return 0
 	}
-	return "0" + qname + ".." + strconv.Itoa(int(qtype))
-}
 
-// nameErrorKey returns a caching key for NXDOMAIN responses.
-func nameErrorKey(qname string, do bool) string {
-	if do {
-		return "1" + qname
+	minTTL := maxTTL
+	for _, r := range append(m.Answer, m.Ns...) {
+		switch mt {
+		case response.NameError, response.NoData:
+			if r.Header().Rrtype == dns.TypeSOA {
+				return time.Duration(r.(*dns.SOA).Minttl) * time.Second
+			}
+		case response.Success, response.Delegation:
+			if r.Header().Ttl < uint32(minTTL.Seconds()) {
+				minTTL = time.Duration(r.Header().Ttl) * time.Second
+			}
+		}
 	}
-	return "0" + qname
+	return minTTL
 }
-
-// successKey returns a caching key for successfull answers.
-func successKey(qname string, qtype uint16, do bool) string { return noDataKey(qname, qtype, do) }

middleware/cache/item_test.go

@@ -7,19 +7,14 @@ import (
 )
 
 func TestKey(t *testing.T) {
-	if noDataKey("miek.nl.", dns.TypeMX, false) != "0miek.nl...15" {
-		t.Errorf("failed to create correct key")
+	if x := rawKey("miek.nl.", dns.TypeMX, false); x != "0miek.nl..15" {
+		t.Errorf("failed to create correct key, got %s", x)
 	}
-	if noDataKey("miek.nl.", dns.TypeMX, true) != "1miek.nl...15" {
-		t.Errorf("failed to create correct key")
+	if x := rawKey("miek.nl.", dns.TypeMX, true); x != "1miek.nl..15" {
+		t.Errorf("failed to create correct key, got %s", x)
 	}
-	if nameErrorKey("miek.nl.", false) != "0miek.nl." {
-		t.Errorf("failed to create correct key")
-	}
-	if nameErrorKey("miek.nl.", true) != "1miek.nl." {
-		t.Errorf("failed to create correct key")
-	}
-	if noDataKey("miek.nl.", dns.TypeMX, false) != successKey("miek.nl.", dns.TypeMX, false) {
-		t.Errorf("nameErrorKey and successKey should be the same")
+	// rawKey does not lowercase.
+	if x := rawKey("miEK.nL.", dns.TypeMX, true); x != "1miEK.nL..15" {
+		t.Errorf("failed to create correct key, got %s", x)
 	}
 }

middleware/cache/setup.go

@@ -2,10 +2,12 @@ package cache
 
 import (
 	"strconv"
+	"time"
 
 	"github.com/miekg/coredns/core/dnsserver"
 	"github.com/miekg/coredns/middleware"
 
+	"github.com/hashicorp/golang-lru"
 	"github.com/mholt/caddy"
 )
 
@@ -16,51 +18,103 @@ func init() {
 	})
 }
 
-// Cache sets up the root file path of the server.
 func setup(c *caddy.Controller) error {
-	ttl, zones, err := cacheParse(c)
+	ca, err := cacheParse(c)
 	if err != nil {
 		return middleware.Error("cache", err)
 	}
 	dnsserver.GetConfig(c).AddMiddleware(func(next middleware.Handler) middleware.Handler {
-		return NewCache(ttl, zones, next)
+		ca.Next = next
+		return ca
 	})
 
 	return nil
 }
 
-func cacheParse(c *caddy.Controller) (int, []string, error) {
-	var (
-		err     error
-		ttl     int
-		origins []string
-	)
+func cacheParse(c *caddy.Controller) (*Cache, error) {
+
+	ca := &Cache{pcap: defaultCap, ncap: defaultCap, pttl: maxTTL, nttl: maxNTTL}
 
 	for c.Next() {
-		if c.Val() == "cache" {
-			// cache [ttl] [zones..]
-			origins = make([]string, len(c.ServerBlockKeys))
-			copy(origins, c.ServerBlockKeys)
-			args := c.RemainingArgs()
+		// cache [ttl] [zones..]
+		origins := make([]string, len(c.ServerBlockKeys))
+		copy(origins, c.ServerBlockKeys)
+		args := c.RemainingArgs()
+
+		if len(args) > 0 {
+			// first args may be just a number, then it is the ttl, if not it is a zone
+			ttl, err := strconv.Atoi(args[0])
+			if err == nil {
+				ca.pttl = time.Duration(ttl) * time.Second
+				ca.nttl = time.Duration(ttl) * time.Second
+				args = args[1:]
+			}
 			if len(args) > 0 {
-				origins = args
-				// first args may be just a number, then it is the ttl, if not it is a zone
-				t := origins[0]
-				ttl, err = strconv.Atoi(t)
-				if err == nil {
-					origins = origins[1:]
-					if len(origins) == 0 {
-						// There was *only* the ttl, revert back to server block
-						copy(origins, c.ServerBlockKeys)
+				copy(origins, args)
+			}
+		}
+
+		// Refinements? In an extra block.
+		for c.NextBlock() {
+			switch c.Val() {
+			// first number is cap, second is an new ttl
+			case "positive":
+				args := c.RemainingArgs()
+				if len(args) == 0 {
+					return nil, c.ArgErr()
+				}
+				pcap, err := strconv.Atoi(args[0])
+				if err != nil {
+					return nil, err
+				}
+				ca.pcap = pcap
+				if len(args) > 1 {
+					pttl, err := strconv.Atoi(args[1])
+					if err != nil {
+						return nil, err
+					}
+					ca.pttl = time.Duration(pttl) * time.Second
+				}
+			case "negative":
+				args := c.RemainingArgs()
+				if len(args) == 0 {
+					return nil, c.ArgErr()
+				}
+				ncap, err := strconv.Atoi(args[0])
+				if err != nil {
+					return nil, err
+				}
+				ca.ncap = ncap
+				if len(args) > 1 {
+					nttl, err := strconv.Atoi(args[1])
+					if err != nil {
+						return nil, err
 					}
+					ca.nttl = time.Duration(nttl) * time.Second
 				}
+			default:
+				return nil, c.ArgErr()
 			}
+		}
 
-			for i := range origins {
-				origins[i] = middleware.Host(origins[i]).Normalize()
-			}
-			return ttl, origins, nil
+		for i := range origins {
+			origins[i] = middleware.Host(origins[i]).Normalize()
+		}
+
+		var err error
+		ca.Zones = origins
+
+		ca.pcache, err = lru.New(ca.pcap)
+		if err != nil {
+			return nil, err
+		}
+		ca.ncache, err = lru.New(ca.ncap)
+		if err != nil {
+			return nil, err
 		}
+
+		return ca, nil
 	}
-	return 0, nil, nil
+
+	return nil, nil
 }

middleware/cache/setup_test.go

+package cache
+
+import (
+	"testing"
+	"time"
+
+	"github.com/mholt/caddy"
+)
+
+func TestSetup(t *testing.T) {
+	tests := []struct {
+		input        string
+		shouldErr    bool
+		expectedNcap int
+		expectedPcap int
+		expectedNttl time.Duration
+		expectedPttl time.Duration
+	}{
+		{`cache`, false, defaultCap, defaultCap, maxNTTL, maxTTL},
+		{`cache {}`, false, defaultCap, defaultCap, maxNTTL, maxTTL},
+		{`cache example.nl {
+			positive 10
+		}`, false, defaultCap, 10, maxNTTL, maxTTL},
+		{`cache example.nl {
+			positive 10
+			negative 10 15
+		}`, false, 10, 10, 15 * time.Second, maxTTL},
+		{`cache 25 example.nl {
+			positive 10
+			negative 10 15
+		}`, false, 10, 10, 15 * time.Second, 25 * time.Second},
+		{`cache aaa example.nl`, false, defaultCap, defaultCap, maxNTTL, maxTTL},
+
+		// fails
+		{`cache example.nl {
+			positive
+			negative 10 15
+		}`, true, defaultCap, defaultCap, maxTTL, maxTTL},
+		{`cache example.nl {
+			positive 15
+			negative aaa
+		}`, true, defaultCap, defaultCap, maxTTL, maxTTL},
+	}
+	for i, test := range tests {
+		c := caddy.NewTestController("dns", test.input)
+		ca, err := cacheParse(c)
+		if test.shouldErr && err == nil {
+			t.Errorf("Test %v: Expected error but found nil", i)
+			continue
+		} else if !test.shouldErr && err != nil {
+			t.Errorf("Test %v: Expected no error but found error: %v", i, err)
+			continue
+		}
+		if test.shouldErr && err != nil {
+			continue
+		}
+
+		if ca.ncap != test.expectedNcap {
+			t.Errorf("Test %v: Expected ncap %v but found: %v", i, test.expectedNcap, ca.ncap)
+		}
+		if ca.pcap != test.expectedPcap {
+			t.Errorf("Test %v: Expected pcap %v but found: %v", i, test.expectedPcap, ca.pcap)
+		}
+		if ca.nttl != test.expectedNttl {
+			t.Errorf("Test %v: Expected nttl %v but found: %v", i, test.expectedNttl, ca.nttl)
+		}
+		if ca.pttl != test.expectedPttl {
+			t.Errorf("Test %v: Expected pttl %v but found: %v", i, test.expectedPttl, ca.pttl)
+		}
+	}
+}

middleware/pkg/response/classify.go

@@ -18,6 +18,22 @@ const (
 	OtherError
 )
 
+func (t Type) String() string {
+	switch t {
+	case Success:
+		return "NOERROR"
+	case NameError:
+		return "NXDOMAIN"
+	case NoData:
+		return "NODATA"
+	case Delegation:
+		return "DELEGATION"
+	case OtherError:
+		return "OTHERERROR"
+	}
+	return ""
+}
+
 // Classify classifies a message, it returns the Type.
 func Classify(m *dns.Msg) (Type, *dns.OPT) {
 	opt := m.IsEdns0()

test/cache_test.go

+package test
+
+import (
+	"io/ioutil"
+	"log"
+	"testing"
+	"time"
+
+	"github.com/miekg/coredns/middleware/proxy"
+	"github.com/miekg/coredns/middleware/test"
+	"github.com/miekg/coredns/request"
+
+	"github.com/miekg/dns"
+)
+
+// This tests uses the exampleOrg zone as defined in proxy_test.go
+
+func TestLookupCache(t *testing.T) {
+	// Start auth. CoreDNS holding the auth zone.
+	name, rm, err := test.TempFile(t, ".", exampleOrg)
+	if err != nil {
+		t.Fatalf("failed to created zone: %s", err)
+	}
+	defer rm()
+
+	corefile := `example.org:0 {
+       file ` + name + `
+}
+`
+	i, err := CoreDNSServer(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+
+	udp, _ := CoreDNSServerPorts(i, 0)
+	if udp == "" {
+		t.Fatalf("Could not get UDP listening port")
+	}
+	defer i.Stop()
+
+	// Start caching proxy CoreDNS that we want to test.
+	corefile = `example.org:0 {
+	proxy . ` + udp + `
+	cache
+}
+`
+	i, err = CoreDNSServer(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+
+	udp, _ = CoreDNSServerPorts(i, 0)
+	if udp == "" {
+		t.Fatalf("Could not get UDP listening port")
+	}
+	defer i.Stop()
+
+	log.SetOutput(ioutil.Discard)
+
+	p := proxy.New([]string{udp})
+	state := request.Request{W: &test.ResponseWriter{}, Req: new(dns.Msg)}
+
+	resp, err := p.Lookup(state, "example.org.", dns.TypeA)
+	if err != nil {
+		t.Fatal("Expected to receive reply, but didn't")
+	}
+	// expect answer section with A record in it
+	if len(resp.Answer) == 0 {
+		t.Error("Expected to at least one RR in the answer section, got none")
+	}
+
+	ttl := resp.Answer[0].Header().Ttl
+
+	time.Sleep(2 * time.Second) // TODO(miek): meh.
+
+	resp, err = p.Lookup(state, "example.org.", dns.TypeA)
+	if err != nil {
+		t.Fatal("Expected to receive reply, but didn't")
+	}
+
+	// expect answer section with A record in it
+	if len(resp.Answer) == 0 {
+		t.Error("Expected to at least one RR in the answer section, got none")
+	}
+	newTTL := resp.Answer[0].Header().Ttl
+	if newTTL >= ttl {
+		t.Errorf("Expected TTL to be lower than: %d, got %d", ttl, newTTL)
+	}
+}

test/etcd_test.go

@@ -48,12 +48,12 @@ func TestEtcdStubAndProxyLookup(t *testing.T) {
 
 	ex, err := CoreDNSServer(corefile)
 	if err != nil {
-		t.Fatalf("could not get CoreDNS serving instance: %s", err)
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
 	}
 
 	udp, _ := CoreDNSServerPorts(ex, 0)
 	if udp == "" {
-		t.Fatalf("could not get udp listening port")
+		t.Fatalf("Could not get UDP listening port")
 	}
 	defer ex.Stop()
 

test/kubernetes_test.go

@@ -74,12 +74,12 @@ func TestKubernetesIntegration(t *testing.T) {
 func createTestServer(t *testing.T, corefile string) (*caddy.Instance, string) {
 	server, err := CoreDNSServer(corefile)
 	if err != nil {
-		t.Fatalf("could not get CoreDNS serving instance: %s", err)
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
 	}
 
 	udp, _ := CoreDNSServerPorts(server, 0)
 	if udp == "" {
-		t.Fatalf("could not get udp listening port")
+		t.Fatalf("Could not get UDP listening port")
 	}
 
 	return server, udp

test/middleware_dnssec_test.go

@@ -31,7 +31,7 @@ func TestLookupBalanceRewriteCacheDnssec(t *testing.T) {
 `
 	ex, err := CoreDNSServer(corefile)
 	if err != nil {
-		t.Fatalf("could not get CoreDNS serving instance: %s", err)
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
 	}
 
 	udp, _ := CoreDNSServerPorts(ex, 0)

test/middleware_test.go

@@ -27,7 +27,7 @@ func benchmarkLookupBalanceRewriteCache(b *testing.B) {
 
 	ex, err := CoreDNSServer(corefile)
 	if err != nil {
-		t.Fatalf("could not get CoreDNS serving instance: %s", err)
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
 	}
 	udp, _ := CoreDNSServerPorts(ex, 0)
 	defer ex.Stop()

test/proxy_test.go

@@ -34,12 +34,12 @@ func TestLookupProxy(t *testing.T) {
 
 	i, err := CoreDNSServer(corefile)
 	if err != nil {
-		t.Fatalf("could not get CoreDNS serving instance: %s", err)
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
 	}
 
 	udp, _ := CoreDNSServerPorts(i, 0)
 	if udp == "" {
-		t.Fatalf("could not get udp listening port")
+		t.Fatalf("Could not get UDP listening port")
 	}
 	defer i.Stop()
 

test/server_test.go

@@ -14,7 +14,7 @@ func TestProxyToChaosServer(t *testing.T) {
 `
 	chaos, err := CoreDNSServer(corefile)
 	if err != nil {
-		t.Fatalf("could not get CoreDNS serving instance: %s", err)
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
 	}
 
 	udpChaos, _ := CoreDNSServerPorts(chaos, 0)
@@ -26,7 +26,7 @@ func TestProxyToChaosServer(t *testing.T) {
 `
 	proxy, err := CoreDNSServer(corefileProxy)
 	if err != nil {
-		t.Fatalf("could not get CoreDNS serving instance")
+		t.Fatalf("Could not get CoreDNS serving instance")
 	}
 
 	udp, _ := CoreDNSServerPorts(proxy, 0)