From 87a2886539af9976a2a0fe2f798e2683646f2703 Mon Sep 17 00:00:00 2001 From: Alvaro Saurin Date: Mon, 11 May 2015 14:36:26 +0200 Subject: [PATCH] Refactored the mocked clock. Refactored some common code in the Zone lookups --- nameserver/addrs.go | 2 + nameserver/mocks_test.go | 17 ++++ nameserver/server_cache_test.go | 15 +--- nameserver/zone.go | 28 +++--- nameserver/zone_lookup.go | 151 +++++++++++++------------------- nameserver/zone_lookup_test.go | 22 ++--- 6 files changed, 105 insertions(+), 130 deletions(-) diff --git a/nameserver/addrs.go b/nameserver/addrs.go index 6d6e3abd..bb4ac27c 100644 --- a/nameserver/addrs.go +++ b/nameserver/addrs.go @@ -25,6 +25,8 @@ type ZoneLookup interface { LookupInaddr(inaddr string) ([]ZoneRecord, error) } +type ZoneLookupFunc func(target string) ([]ZoneRecord, error) + type ZoneObservable interface { // Observe anything that affects a particular name in the zone ObserveName(name string, observer ZoneRecordObserver) error diff --git a/nameserver/mocks_test.go b/nameserver/mocks_test.go index 55fabdcb..ca3c941d 100644 --- a/nameserver/mocks_test.go +++ b/nameserver/mocks_test.go @@ -2,6 +2,7 @@ package nameserver import ( "fmt" + "github.com/benbjohnson/clock" "github.com/miekg/dns" . "github.com/weaveworks/weave/common" wt "github.com/weaveworks/weave/testing" @@ -423,6 +424,22 @@ func runLocalTCPServer(laddr string, handler dns.HandlerFunc) (*dns.Server, stri ////////////////////////////////////////////////////////////////// +type mockedClock struct { + *clock.Mock +} + +func newMockedClock() *mockedClock { + return &mockedClock{clock.NewMock()} +} + +func (clk *mockedClock) Forward(secs int) { + Debug.Printf(">>>>>>> Moving clock forward %d seconds - Time traveling >>>>>>>", secs) + clk.Add(time.Duration(secs) * time.Second) + Debug.Printf("<<<<<<< Time travel finished! We are at %s <<<<<<<", clk.Now()) +} + +////////////////////////////////////////////////////////////////// + // Perform a DNS query and assert the reply code, number or answers, etc func assertExchange(t *testing.T, z string, ty uint16, port int, minAnswers int, maxAnswers int, expErr int) (*dns.Msg, *dns.Msg) { wt.AssertNotEqualInt(t, port, 0, "invalid DNS server port") diff --git a/nameserver/server_cache_test.go b/nameserver/server_cache_test.go index fe920a0e..c06eae1c 100644 --- a/nameserver/server_cache_test.go +++ b/nameserver/server_cache_test.go @@ -2,7 +2,6 @@ package nameserver import ( "fmt" - "github.com/benbjohnson/clock" "github.com/miekg/dns" . "github.com/weaveworks/weave/common" wt "github.com/weaveworks/weave/testing" @@ -22,13 +21,7 @@ func TestServerCacheRefresh(t *testing.T) { InitDefaultLogging(testing.Verbose()) Info.Println("TestServerCacheRefresh starting") - - clk := clock.NewMock() - forwardClock := func(secs int) { - Debug.Printf(">>>>>>> Moving clock forward %d seconds - Time traveling >>>>>>>", secs) - clk.Add(time.Duration(secs) * time.Second) - Debug.Printf("<<<<<<< Time travel finished! We are at %s <<<<<<<", clk.Now()) - } + clk := newMockedClock() Debug.Printf("Creating 2 zone databases") zoneConfig := ZoneConfig{ @@ -71,7 +64,7 @@ func TestServerCacheRefresh(t *testing.T) { assertInCache(t, cache, qName1, "after asking for first name") assertNotLocalInCache(t, cache, qName2, "after asking for second name") - forwardClock(refreshInterval / 2) + clk.Forward(refreshInterval / 2) Debug.Printf("Adding an IP to %s and to %s", testName1, testName2) dbs[1].Zone.AddRecord(containerID, testName1, net.ParseIP("10.2.2.2")) @@ -80,7 +73,7 @@ func TestServerCacheRefresh(t *testing.T) { // Zone database #2 at this point: // first.weave.local = 10.2.2.1 10.2.2.2 // second.weave.local = 10.9.9.2 - forwardClock(refreshInterval/2 + 2) + clk.Forward(refreshInterval/2 + 2) // at this point, the testName1 should have been refreshed // so it should have two IPs, and the cache entry should have been invalidated @@ -95,7 +88,7 @@ func TestServerCacheRefresh(t *testing.T) { // delete the IPs, and some time passes by so the cache should be purged... dbs[1].Zone.DeleteRecord(containerID, net.ParseIP("10.2.2.1")) dbs[1].Zone.DeleteRecord(containerID, net.ParseIP("10.2.2.2")) - forwardClock(refreshInterval + 1) + clk.Forward(refreshInterval + 1) qName1, _ = assertExchange(t, testName1, dns.TypeA, testPort, 0, 0, dns.RcodeNameError) qName2, _ = assertExchange(t, testName2, dns.TypeA, testPort, 0, 0, dns.RcodeNameError) diff --git a/nameserver/zone.go b/nameserver/zone.go index 7021bb47..c797779c 100644 --- a/nameserver/zone.go +++ b/nameserver/zone.go @@ -122,7 +122,7 @@ func (re *recordEntry) notifyIPObservers() { } } -func (re recordEntry) TTL() int { +func (re *recordEntry) TTL() int { // if we never set the TTL (eg, when using AddRecord()), return the standard value... if re.ttl == 0 { return int(localTTL) @@ -131,11 +131,11 @@ func (re recordEntry) TTL() int { } // Check if the info in this record is still valid according to the TTL -func (re recordEntry) hasExpired(now time.Time) bool { +func (re *recordEntry) hasExpired(now time.Time) bool { return re.ttl > 0 && now.Sub(re.insTime).Seconds() > float64(re.ttl) } -func (re recordEntry) String() string { +func (re *recordEntry) String() string { var buf bytes.Buffer fmt.Fprintf(&buf, "%s", re.Record) lobs := len(re.observers) @@ -163,10 +163,10 @@ func newName(n string) *name { observers: make([]ZoneRecordObserver, 0), } } -func (n name) len() int { return len(n.ipv4) } -func (n name) empty() bool { return n.len() == 0 } -func (n name) hasIP(ip net.IP) bool { return n.hasIPv4(ipToIPv4(ip)) } -func (n name) hasIPv4(ip IPv4) bool { _, found := n.ipv4[ip]; return found } +func (n *name) len() int { return len(n.ipv4) } +func (n *name) empty() bool { return n.len() == 0 } +func (n *name) hasIP(ip net.IP) bool { return n.hasIPv4(ipToIPv4(ip)) } +func (n *name) hasIPv4(ip IPv4) bool { _, found := n.ipv4[ip]; return found } // Get all the entries for this name func (n *name) getAllEntries() (res []*recordEntry) { @@ -257,7 +257,7 @@ func (n *name) notifyNameObservers() { } // Convert a set of IP records to a comma-separated string -func (n name) String() string { +func (n *name) String() string { var buf bytes.Buffer for _, ip := range n.ipv4 { fmt.Fprintf(&buf, "%s, ", ip) @@ -276,10 +276,10 @@ type namesSet struct { names map[string]*name } -func newNamesSet() *namesSet { return &namesSet{names: make(map[string]*name)} } -func (ns namesSet) len() int { return len(ns.names) } -func (ns namesSet) empty() bool { return ns.len() == 0 } -func (ns namesSet) hasName(name string) bool { +func newNamesSet() *namesSet { return &namesSet{names: make(map[string]*name)} } +func (ns *namesSet) len() int { return len(ns.names) } +func (ns *namesSet) empty() bool { return ns.len() == 0 } +func (ns *namesSet) hasName(name string) bool { if n, found := ns.names[name]; !found { return false } else { @@ -365,7 +365,7 @@ func (ns *namesSet) deleteIP(ip IPv4) error { } // Get the name query time -func (ns namesSet) getNameLastAccess(n string) time.Time { +func (ns *namesSet) getNameLastAccess(n string) time.Time { if name, found := ns.names[n]; found { return name.lastAccessTime } @@ -383,7 +383,7 @@ func (ns *namesSet) touchName(name string, now time.Time) { } } -func (ns namesSet) String() string { +func (ns *namesSet) String() string { var buf bytes.Buffer for _, name := range ns.names { fmt.Fprintf(&buf, "%s, ", name) diff --git a/nameserver/zone_lookup.go b/nameserver/zone_lookup.go index 247df6bd..6eac008e 100644 --- a/nameserver/zone_lookup.go +++ b/nameserver/zone_lookup.go @@ -6,6 +6,20 @@ import ( "time" ) +func (zone *zoneDb) lookup(target string, lfun func(ns *namesSet) []*recordEntry) (res []ZoneRecord, err error) { + for identName, nameset := range zone.idents { + if identName != defaultRemoteIdent { + for _, ze := range lfun(nameset) { + res = append(res, ze) + } + } + } + if len(res) == 0 { + err = LookupError(target) + } + return +} + // Perform a lookup for a name in the zone // The name can be resolved locally with the local database func (zone *zoneDb) LookupName(name string) (res []ZoneRecord, err error) { @@ -15,17 +29,49 @@ func (zone *zoneDb) LookupName(name string) (res []ZoneRecord, err error) { // note: LookupName() is usually called from the mDNS server, so we do not touch the name name = dns.Fqdn(name) Debug.Printf("[zonedb] Looking for name '%s' in local database", name) - for identName, nameset := range zone.idents { - if identName != defaultRemoteIdent { - for _, ze := range nameset.getEntriesForName(name) { - res = append(res, ze) - } - } + return zone.lookup(name, func(ns *namesSet) []*recordEntry { return ns.getEntriesForName(name) }) +} + +// Perform a lookup for a IP address in the zone +// The address can be resolved locally with the local database +func (zone *zoneDb) LookupInaddr(inaddr string) (res []ZoneRecord, err error) { + zone.mx.RLock() + defer zone.mx.RUnlock() + + // note: LookupInaddr() is usually called from the mDNS server, so we do not touch the name + revIPv4, err := raddrToIPv4(inaddr) + if err != nil { + return nil, newParseError("lookup address", inaddr) + } + Debug.Printf("[zonedb] Looking for address in local database: '%s' (%s)", revIPv4, inaddr) + return zone.lookup(inaddr, func(ns *namesSet) []*recordEntry { return ns.getEntriesForIP(revIPv4) }) +} + +// Perform a domain lookup with mDNS +func (zone *zoneDb) domainLookup(target string, lfun ZoneLookupFunc) (res []ZoneRecord, err error) { + // no local results have been obtained in the local database: try with a mDNS query + Debug.Printf("[zonedb] '%s' not in local database... trying with mDNS", target) + lanswers, err := lfun(target) + if err != nil { + Debug.Printf("[zonedb] mDNS lookup error for '%s': %s", target, err) + return nil, err } - if len(res) == 0 { - err = LookupError(name) + // if the request has been successful, save the IP in the local database and return the corresponding ZoneRecord + // (we do not get the remote ident in the mDNS reply, so we save it in a "remote" ident) + Debug.Printf("[zonedb] adding '%s' (obtained with mDNS) to '%s'", lanswers, target) + res = make([]ZoneRecord, len(lanswers)) + zone.mx.Lock() + now := zone.clock.Now() + for i, answer := range lanswers { + res[i], err = zone.getNamesSet(defaultRemoteIdent).addIPToName(answer, now) + if err != nil { + zone.mx.Unlock() + Warning.Printf("[zonedb] '%s' insertion for %s failed: %s", answer, target, err) + return nil, err + } } + zone.mx.Unlock() return } @@ -40,7 +86,7 @@ func (zone *zoneDb) DomainLookupName(name string) (res []ZoneRecord, err error) for identName, nameset := range zone.idents { for _, ze := range nameset.getEntriesForName(name) { // filter the entries with expired TTL - // locally introduced entries are nver expired as the always have TTL=0 + // locally introduced entries are never expired: they always have TTL=0 if ze.hasExpired(now) { Debug.Printf("[zonedb] '%s': expired entry '%s' ignored: removing", name, ze) nameset.deleteName(name) @@ -57,69 +103,16 @@ func (zone *zoneDb) DomainLookupName(name string) (res []ZoneRecord, err error) if len(res) > 0 { Debug.Printf("[zonedb] '%s' resolved in local database", name) } else { - // no local results have been obtained in the local database: try with a mDNS query - // (this request is not a background query, so we cannot delay the response) - Debug.Printf("[zonedb] name '%s' not in local database: trying with mDNS", name) - ips, err := zone.mdnsCli.LookupName(name) - if err != nil { - Debug.Printf("[zonedb] mDNS lookup error for '%s': %s", name, err) - return nil, err - } - - // if the request has been successful, save the IP in the local database and return the corresponding ZoneRecord - // (we do not get the remote ident in the mDNS reply, so we save it in a "remote" ident) - Debug.Printf("[zonedb] adding '%s' (obtained with mDNS) to '%s'", ips, name) - res = make([]ZoneRecord, len(ips)) - zone.mx.Lock() - now = zone.clock.Now() - for i, zr := range ips { - res[i], err = zone.getNamesSet(defaultRemoteIdent).addIPToName(zr, now) - if err != nil { - zone.mx.Unlock() - Warning.Printf("[zonedb] IP [%s] insertion for '%s' failed: %s", ips, name, err) - return nil, err - } - } - zone.mx.Unlock() + res, err = zone.domainLookup(name, zone.mdnsCli.LookupName) } if len(res) > 0 { - // start (if we need to) a background refresh for the name - // note: we do not spend time trying to update names that did not return an initial response... zone.startUpdatingName(name) - return res, nil } - return nil, LookupError(name) } -// Perform a lookup for a IP address in the zone -// The address can be resolved locally with the local database -func (zone *zoneDb) LookupInaddr(inaddr string) (res []ZoneRecord, err error) { - zone.mx.RLock() - defer zone.mx.RUnlock() - - // note: LookupInaddr() is usually called from the mDNS server, so we do not touch the name - - revIPv4, err := raddrToIPv4(inaddr) - if err != nil { - return nil, newParseError("lookup address", inaddr) - } - Debug.Printf("[zonedb] Looking for address in local database: '%s' (%s)", revIPv4, inaddr) - for identName, nameset := range zone.idents { - if identName != defaultRemoteIdent { - for _, ze := range nameset.getEntriesForIP(revIPv4) { - res = append(res, ZoneRecord(ze)) - } - } - } - if len(res) == 0 { - err = LookupError(inaddr) - } - return -} - // Perform a lookup for a IP address in the zone // The address can be resolved either with the local database or // with some other resolution method (eg, a mDNS query) @@ -136,7 +129,7 @@ func (zone *zoneDb) DomainLookupInaddr(inaddr string) (res []ZoneRecord, err err for identName, nameset := range zone.idents { for _, ze := range nameset.getEntriesForIP(revIPv4) { // filter the entries with expired TTL - // locally introduced entries are nver expired as the always have TTL=0 + // locally introduced entries are never expired: they always have TTL=0 if ze.hasExpired(now) { Debug.Printf("[zonedb] '%s': expired entry '%s' ignored: removing", revIPv4, ze) nameset.deleteIP(revIPv4) @@ -153,44 +146,20 @@ func (zone *zoneDb) DomainLookupInaddr(inaddr string) (res []ZoneRecord, err err if len(res) > 0 { Debug.Printf("[zonedb] '%s' resolved in local database", inaddr) } else { - // no local results have been obtained in the local database: try with a mDNS query - Debug.Printf("[zonedb] '%s'(%+v) not in local database... trying with mDNS", inaddr, revIPv4) - names, err := zone.mdnsCli.LookupInaddr(inaddr) - if err != nil { - Debug.Printf("[zonedb] mDNS lookup error for '%s': %s", inaddr, err) - return nil, err - } - - // if the request has been successful, save the IP in the local database and return the corresponding ZoneRecord - // (we do not get the remote ident in the mDNS reply, so we save it in a "remote" ident) - Debug.Printf("[zonedb] adding '%s' (obtained with mDNS) to '%s'", names, revIPv4) - res = make([]ZoneRecord, len(names)) - zone.mx.Lock() - now = zone.clock.Now() - for i, name := range names { - res[i], err = zone.getNamesSet(defaultRemoteIdent).addIPToName(name, now) - if err != nil { - zone.mx.Unlock() - Warning.Printf("[zonedb] Name '%s' insertion for %s failed: %s", name.Name(), revIPv4, err) - return nil, err - } - } - zone.mx.Unlock() + res, err = zone.domainLookup(inaddr, zone.mdnsCli.LookupInaddr) } if len(res) > 0 { // note: even for reverse addresses, we perform the background updates in the name, not in the IP // this simplifies the process and produces basically the same results... // note: we do not spend time trying to update names that did not return an initial response... - for _, name := range res { - zone.startUpdatingName(name.Name()) + for _, r := range res { + zone.startUpdatingName(r.Name()) } return res, nil } - return nil, LookupError(inaddr) - } ////////////////////////////////////////////////////////////////////////////// diff --git a/nameserver/zone_lookup_test.go b/nameserver/zone_lookup_test.go index 8593086b..6c09ab6b 100644 --- a/nameserver/zone_lookup_test.go +++ b/nameserver/zone_lookup_test.go @@ -1,7 +1,6 @@ package nameserver import ( - "github.com/benbjohnson/clock" . "github.com/weaveworks/weave/common" wt "github.com/weaveworks/weave/testing" "net" @@ -26,12 +25,7 @@ func TestZoneRefresh(t *testing.T) { InitDefaultLogging(testing.Verbose()) Info.Println("TestZoneRefresh starting") - clk := clock.NewMock() - forwardClock := func(secs int) { - Debug.Printf(">>>>>>> Moving clock forward %d seconds - Time traveling >>>>>>>", secs) - clk.Add(time.Duration(secs) * time.Second) - Debug.Printf("<<<<<<< Time travel finished! We are at %s <<<<<<<", clk.Now()) - } + clk := newMockedClock() // Create multiple zone databases, linked through mocked mDNS connections zc := ZoneConfig{ @@ -60,14 +54,14 @@ func TestZoneRefresh(t *testing.T) { t.Logf("Db #1 after the lookup:\n%s", dbs[0].Zone) wt.AssertEqualInt(t, len(res), 1, "lookup result") - forwardClock(refreshInterval / 2) + clk.Forward(refreshInterval / 2) Debug.Printf("A couple of seconds later, we should still have one IP for that name") res, err = dbs[0].Zone.DomainLookupName(name) wt.AssertNoErr(t, err) wt.AssertEqualInt(t, len(res), 1, "lookup result") Debug.Printf("And then we add 2 IPs for that name at ZoneDb 2") - forwardClock(1) + clk.Forward(1) Debug.Printf("Adding 2 IPs to '%s' in Db #2", name) dbs[1].Zone.AddRecord("someident", name, net.ParseIP(addr2)) dbs[1].Zone.AddRecord("someident", name, net.ParseIP(addr3)) @@ -76,7 +70,7 @@ func TestZoneRefresh(t *testing.T) { dbs[0].Zone.DomainLookupName(name) Debug.Printf("Wait for a while, until a refresh is performed...") - forwardClock(refreshInterval + 1) + clk.Forward(refreshInterval + 1) Debug.Printf("A refresh should have been scheduled now: we should have 3 IPs:") Debug.Printf("the first (local) IP and the others obtained from zone2 with a mDNS query") @@ -87,7 +81,7 @@ func TestZoneRefresh(t *testing.T) { wt.AssertEqualInt(t, len(res), 3, "lookup result length") Debug.Printf("We will not ask for `name` for a while, so it will become irrelevant and will be removed...") - forwardClock(relevantTime + refreshInterval + 1) + clk.Forward(relevantTime + refreshInterval + 1) // the name should be irrelevant now, and all remote info should have been // removed from the zone database @@ -97,17 +91,17 @@ func TestZoneRefresh(t *testing.T) { wt.AssertFalse(t, dbs[0].Zone.HasNameRemoteInfo(name), "remote name info") Debug.Printf("There is no remote info about this name at zone 1: a new IP appears remotely meanwhile...") - forwardClock(1) + clk.Forward(1) Debug.Printf("Adding '%s' to Db #2", name) dbs[1].Zone.AddRecord("someident", name, net.ParseIP(addr4)) Debug.Printf("When we ask about this name again, we get 4 IPs (1 local, 3 remote)") - forwardClock(1) + clk.Forward(1) Debug.Printf("Asking for '%s' again... the first lookup will return only the local results", name) res, err = dbs[0].Zone.DomainLookupName(name) wt.AssertEqualInt(t, len(res), 1, "lookup result length") Debug.Printf("... but a second lookup should return all the results in the network") - forwardClock(1) + clk.Forward(1) res, err = dbs[0].Zone.DomainLookupName(name) Debug.Printf("Got: %s", res) wt.AssertEqualInt(t, len(res), 4, "lookup result length") // TODO: this fails 1% of the runs... !!??