From 001322751fbabe49f276ba355fbcc858c423a2b8 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 21 Mar 2016 11:52:13 +0000 Subject: [PATCH 1/4] Don't hold Nameserver lock over call out via isKnownPeer --- nameserver/nameserver.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index d15ad3a3..806db03b 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -204,12 +204,15 @@ func (n *Nameserver) receiveGossip(msg []byte) (mesh.GossipData, mesh.GossipData } n.Lock() - defer n.Unlock() - + isKnownPeer := n.isKnownPeer + n.Unlock() // Don't hold the lock over call out to code elsewhere gossip.Entries.filter(func(e *Entry) bool { - return n.isKnownPeer(e.Origin) + return isKnownPeer(e.Origin) }) + n.Lock() + defer n.Unlock() + newEntries := n.entries.merge(gossip.Entries) if len(newEntries) > 0 { return &GossipData{Entries: newEntries, Timestamp: now()}, &gossip, nil From 010766fe9e3037502f351619ffc7c5f82c5c6487 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 21 Mar 2016 15:22:36 +0000 Subject: [PATCH 2/4] Check entries claiming to originate from us against our current data, and override if necessary --- nameserver/entry.go | 20 ++++++++++++++++++-- nameserver/nameserver.go | 25 +++++++++++++++++++++---- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/nameserver/entry.go b/nameserver/entry.go index cb0830fa..89a48ca7 100644 --- a/nameserver/entry.go +++ b/nameserver/entry.go @@ -105,6 +105,11 @@ func (e1 *Entry) addLowercase() { e1.lHostname = strings.ToLower(e1.Hostname) } +func (e *Entry) tombstone() { + e.Version++ + e.Tombstone = now() +} + func check(es SortableEntries) error { if !sort.IsSorted(es) { return fmt.Errorf("Not sorted!") @@ -182,8 +187,7 @@ func (es *Entries) tombstone(ourname mesh.PeerName, f func(*Entry) bool) Entries tombstoned := Entries{} for i, e := range *es { if f(&e) && e.Origin == ourname { - e.Version++ - e.Tombstone = now() + e.tombstone() (*es)[i] = e tombstoned = append(tombstoned, e) } @@ -191,6 +195,7 @@ func (es *Entries) tombstone(ourname mesh.PeerName, f func(*Entry) bool) Entries return tombstoned } +// note f() may only modify entries such that they remain in order defined by less() func (es *Entries) filter(f func(*Entry) bool) { defer es.checkAndPanic().checkAndPanic() @@ -205,6 +210,17 @@ func (es *Entries) filter(f func(*Entry) bool) { *es = (*es)[:i] } +func (es Entries) findEqual(e *Entry) (*Entry, bool) { + i := sort.Search(len(es), func(i int) bool { + return !es[i].insensitiveLess(e) + }) + if i < len(es) && es[i].equal(*e) { + return &es[i], true + } else { + return nil, false + } +} + func (es Entries) lookup(hostname string) Entries { es.checkAndPanic() diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index 806db03b..002c7f29 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -203,16 +203,33 @@ func (n *Nameserver) receiveGossip(msg []byte) (mesh.GossipData, mesh.GossipData return nil, nil, fmt.Errorf("clock skew of %d detected", delta) } - n.Lock() - isKnownPeer := n.isKnownPeer - n.Unlock() // Don't hold the lock over call out to code elsewhere + // Filter to remove entries from unknown peers, done before we take + // the nameserver lock, so we don't have to worry what isKnownPeer locks. gossip.Entries.filter(func(e *Entry) bool { - return isKnownPeer(e.Origin) + return n.isKnownPeer(e.Origin) }) n.Lock() defer n.Unlock() + // Check entries claiming to originate from us against our current data + gossip.Entries.filter(func(e *Entry) bool { + if e.Origin == n.ourName { + if ourEntry, ok := n.entries.findEqual(e); ok { + if ourEntry.Version < e.Version || + (ourEntry.Version == e.Version && ourEntry.Tombstone != e.Tombstone) { + // Take our version of the data, but make the version higher than the incoming + nextVersion := e.Version + 1 + *e = *ourEntry + e.Version = nextVersion + } + } else { // We have no entry matching the one that came in with us as Origin + e.tombstone() + } + } + return true + }) + newEntries := n.entries.merge(gossip.Entries) if len(newEntries) > 0 { return &GossipData{Entries: newEntries, Timestamp: now()}, &gossip, nil From 38a79b7f593255ab15588baafe4c705d2bd9c7aa Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 21 Mar 2016 15:37:46 +0000 Subject: [PATCH 3/4] Pick entry with newer tombstone to merge --- nameserver/entry.go | 3 +++ nameserver/entry_test.go | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/nameserver/entry.go b/nameserver/entry.go index 89a48ca7..9781e178 100644 --- a/nameserver/entry.go +++ b/nameserver/entry.go @@ -93,6 +93,9 @@ func (e1 *Entry) merge(e2 *Entry) bool { e1.Version = e2.Version e1.Tombstone = e2.Tombstone return true + } else if e2.Version == e1.Version && e2.Tombstone > e1.Tombstone { + e1.Tombstone = e2.Tombstone + return true } return false } diff --git a/nameserver/entry_test.go b/nameserver/entry_test.go index 57dfde02..582dab4b 100644 --- a/nameserver/entry_test.go +++ b/nameserver/entry_test.go @@ -86,6 +86,21 @@ func TestTombstone(t *testing.T) { Entry{Hostname: "B", Version: 1, Tombstone: 1234}, }) require.Equal(t, expected, es) + + // Now try a merge including two entries which differ only in tombstone + e2 := make(Entries, len(es)) + copy(e2, es) + e2[1].Tombstone = 5555 + + diff := es.merge(e2) + + expected2 := l(Entries{ + Entry{Hostname: "A"}, + Entry{Hostname: "B", Version: 1, Tombstone: 5555}, + }) + require.Equal(t, expected2, es) + expectedDiff := l(Entries{Entry{Hostname: "B", Version: 1, Tombstone: 5555}}) + require.Equal(t, expectedDiff, diff) } func TestDelete(t *testing.T) { From c2cf825cdf5aa1aae3ad35008dbe2a407e8c2ea0 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 21 Mar 2016 17:58:12 +0000 Subject: [PATCH 4/4] Smoke test for stale DNS entries --- test/215_stale_dns_3_test.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100755 test/215_stale_dns_3_test.sh diff --git a/test/215_stale_dns_3_test.sh b/test/215_stale_dns_3_test.sh new file mode 100755 index 00000000..b75fcdd0 --- /dev/null +++ b/test/215_stale_dns_3_test.sh @@ -0,0 +1,31 @@ +#! /bin/bash + +. ./config.sh + +start_suite "Check propagation of DNS entries when gossip is delayed" + +weave_on $HOST1 launch-router --no-discovery +weave_on $HOST2 launch-router --no-discovery $HOST1 +weave_on $HOST3 launch-router --no-discovery $HOST2 + +start_container $HOST1 --name=c1 +C1=$(container_ip $HOST1 c1) +start_container_with_dns $HOST3 --name=c3 + +assert_dns_record $HOST3 c3 c1.weave.local $C1 + +# Stall the propagation of gossip, stop weave and kill the container +run_on $HOST3 "sudo iptables -A INPUT -s $HOST2 -j DROP" + +weave_on $HOST1 stop-router +docker_on $HOST1 rm -f c1 + +weave_on $HOST1 launch-router --no-discovery $HOST3 + +# Now re-enable gossip +run_on $HOST3 "sudo iptables -D INPUT -s $HOST2 -j DROP" + +# Currently failing due to Mesh #28 +# assert_no_dns_record $HOST3 c3 c1.weave.local + +end_suite