Do not overwrite negLocal entries in the cache

Unit tests

nameserver/cache.go

@@ -5,13 +5,14 @@ import (
 	"container/heap"
 	"errors"
 	"fmt"
-	"github.com/benbjohnson/clock"
-	"github.com/miekg/dns"
-	. "github.com/weaveworks/weave/common"
 	"math"
 	"math/rand"
 	"sync"
 	"time"
+
+	"github.com/benbjohnson/clock"
+	"github.com/miekg/dns"
+	. "github.com/weaveworks/weave/common"
 )
 
 var (
@@ -145,9 +146,12 @@ func (e *cacheEntry) setReply(reply *dns.Msg, ttl int, flags uint8, now time.Tim
 		prevValidUntil = e.validUntil
 	}
 
-	e.Status = stResolved
-	e.Flags = flags
-	e.putTime = now
+	// make sure we do not overwrite noLocalReplies entries
+	if flags&CacheNoLocalReplies != 0 {
+		if e.Flags&CacheNoLocalReplies != 0 {
+			return false
+		}
+	}
 
 	if ttl != nullTTL {
 		e.validUntil = now.Add(time.Second * time.Duration(ttl))
@@ -161,8 +165,14 @@ func (e *cacheEntry) setReply(reply *dns.Msg, ttl int, flags uint8, now time.Tim
 			}
 		}
 		e.validUntil = now.Add(time.Second * time.Duration(minTTL))
+	} else {
+		Warning.Printf("[cache] no valid TTL could be calculated")
 	}
 
+	e.Status = stResolved
+	e.Flags = flags
+	e.putTime = now
+
 	if reply != nil {
 		e.reply = *reply
 		e.ReplyLen = reply.Len()

nameserver/cache_test.go

@@ -2,13 +2,14 @@ package nameserver
 
 import (
 	"fmt"
+	"net"
+	"testing"
+	"time"
+
 	"github.com/benbjohnson/clock"
 	"github.com/miekg/dns"
 	. "github.com/weaveworks/weave/common"
 	wt "github.com/weaveworks/weave/testing"
-	"net"
-	"testing"
-	"time"
 )
 
 // Check that the cache keeps its intended capacity constant
@@ -195,12 +196,23 @@ func TestCacheEntries(t *testing.T) {
 	question3 := &questionMsg3.Question[0]
 
 	t.Logf("Checking that a entry with CacheNoLocalReplies return an error")
-	l.Put(questionMsg3, nil, nullTTL, CacheNoLocalReplies)
+	l.Put(questionMsg3, nil, negLocalTTL, CacheNoLocalReplies)
 	resp, err = l.Get(questionMsg3, minUDPSize)
 	wt.AssertNil(t, resp, "Get() response with CacheNoLocalReplies")
-	wt.AssertNotNil(t, err, "Get() error with CacheNoLocalReplies")
+	wt.AssertTrue(t, err == errNoLocalReplies, "Get() error with CacheNoLocalReplies")
 
-	clk.Add(time.Second * time.Duration(negLocalTTL+1))
+	t.Logf("Checking that more Put's do not make the negative TTL longer")
+	clk.Add(time.Duration(negLocalTTL/2) * time.Second)
+	l.Put(questionMsg3, nil, negLocalTTL, CacheNoLocalReplies)
+	clk.Add(time.Duration(1) * time.Second)
+	l.Put(questionMsg3, nil, negLocalTTL, CacheNoLocalReplies)
+	clk.Add(time.Duration(1) * time.Second)
+	l.Put(questionMsg3, nil, negLocalTTL, CacheNoLocalReplies)
+	resp, err = l.Get(questionMsg3, minUDPSize)
+	wt.AssertNil(t, resp, "Get() response with CacheNoLocalReplies")
+	wt.AssertTrue(t, err == errNoLocalReplies, "Get() error with CacheNoLocalReplies")
+
+	clk.Add(time.Duration(negLocalTTL/2) * time.Second)
 	t.Logf("Checking that we get an expired response after %f seconds", negLocalTTL)
 	resp, err = l.Get(questionMsg3, minUDPSize)
 	wt.AssertNil(t, resp, "expired Get() response with CacheNoLocalReplies")