diff --git a/nameserver/entry.go b/nameserver/entry.go index 0ab22008..1738b91e 100644 --- a/nameserver/entry.go +++ b/nameserver/entry.go @@ -54,8 +54,9 @@ func (e1 *Entry) merge(e2 *Entry) bool { if e2.Version > e1.Version { e1.Version = e2.Version e1.Tombstone = e2.Tombstone + return true } - return e2.Version != e1.Version + return false } func (e1 *Entry) String() string { @@ -87,8 +88,10 @@ func (es *Entries) add(hostname, containerid string, origin router.PeerName, add return !(*es)[i].less(&entry) }) if i < len(*es) && (*es)[i].equal(&entry) { - (*es)[i].Tombstone = 0 - (*es)[i].Version++ + if (*es)[i].Tombstone > 0 { + (*es)[i].Tombstone = 0 + (*es)[i].Version++ + } } else { *es = append(*es, Entry{}) copy((*es)[i+1:], (*es)[i:]) @@ -99,24 +102,23 @@ func (es *Entries) add(hostname, containerid string, origin router.PeerName, add func (es *Entries) merge(incoming Entries) Entries { defer es.checkAndPanic() - var ( - newEntries Entries - i = 0 - ) + newEntries := Entries{} + i := 0 for _, entry := range incoming { for i < len(*es) && (*es)[i].less(&entry) { i++ } if i < len(*es) && (*es)[i].equal(&entry) { - if !(*es)[i].merge(&entry) { - continue + if (*es)[i].merge(&entry) { + newEntries = append(newEntries, entry) } + } else { + *es = append(*es, Entry{}) + copy((*es)[i+1:], (*es)[i:]) + (*es)[i] = entry + newEntries = append(newEntries, entry) } - *es = append(*es, Entry{}) - copy((*es)[i+1:], (*es)[i:]) - (*es)[i] = entry - newEntries = append(newEntries, entry) } return newEntries diff --git a/nameserver/entry_test.go b/nameserver/entry_test.go index 5e8efa03..d5d6e329 100644 --- a/nameserver/entry_test.go +++ b/nameserver/entry_test.go @@ -48,7 +48,13 @@ func TestMerge(t *testing.T) { Entry{Hostname: "F"}, } - e1.merge(e2) + diff := e1.merge(e2) + expectedDiff := Entries{ + Entry{Hostname: "B"}, + Entry{Hostname: "E"}, + } + require.Equal(t, expectedDiff, diff) + expected := Entries{ Entry{Hostname: "A"}, Entry{Hostname: "B"}, @@ -57,8 +63,21 @@ func TestMerge(t *testing.T) { Entry{Hostname: "E"}, Entry{Hostname: "F"}, } + require.Equal(t, expected, e1) - require.Equal(t, e1, expected) + diff = e1.merge(e1) + require.Equal(t, Entries{}, diff) +} + +func TestOldMerge(t *testing.T) { + e1 := Entries{Entry{Hostname: "A", Version: 0}} + diff := e1.merge(Entries{Entry{Hostname: "A", Version: 1}}) + require.Equal(t, Entries{Entry{Hostname: "A", Version: 1}}, diff) + require.Equal(t, Entries{Entry{Hostname: "A", Version: 1}}, e1) + + diff = e1.merge(Entries{Entry{Hostname: "A", Version: 0}}) + require.Equal(t, Entries{}, diff) + require.Equal(t, Entries{Entry{Hostname: "A", Version: 1}}, e1) } func TestTombstone(t *testing.T) { diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index e6e713c1..5a2e7cb0 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -226,7 +226,10 @@ func (n *Nameserver) receiveGossip(msg []byte) (router.GossipData, router.Gossip } newEntries := n.entries.merge(entries) - return &newEntries, &entries, nil + if len(newEntries) > 0 { + return &newEntries, &entries, nil + } + return nil, &entries, nil } // merge received data into state and return "everything new I've diff --git a/test/210_dns_cross_hosts_test.sh b/test/210_dns_cross_hosts_test.sh index b3836bbc..1d936e3e 100755 --- a/test/210_dns_cross_hosts_test.sh +++ b/test/210_dns_cross_hosts_test.sh @@ -12,14 +12,30 @@ NAME4=seefour.weave.local start_suite "Resolve names across hosts" -weave_on $HOST1 launch --ipalloc-range $UNIVERSE -weave_on $HOST2 launch --ipalloc-range $UNIVERSE $HOST1 +for host in $HOSTS; do + weave_on $host launch --ipalloc-range $UNIVERSE $HOSTS +done +# Basic test start_container $HOST2 $C2/24 --name=c2 -h $NAME2 start_container_with_dns $HOST1 $C1/24 --name=c1 assert_dns_record $HOST1 c1 $NAME2 $C2 +# 2 containers on each host, all with the same names +FOO_IPS= +for host in $HOSTS; do + CID=$(proxy docker_on $host run -dti --name=foo $SMALL_IMAGE /bin/sh) + FOO_IPS="$FOO_IPS $(container_ip $host foo)" + + CID=$(proxy docker_on $host run -dti --name=bar $SMALL_IMAGE /bin/sh) + BAR_IPS="$BAR_IPS $(container_ip $host bar)" +done + +start_container_with_dns $HOST1 --name=baz +assert_dns_record $HOST1 baz foo.weave.local $FOO_IPS +assert_dns_record $HOST1 baz bar.weave.local $BAR_IPS + # resolution for names mapped to multiple addresses weave_on $HOST2 dns-add $C2a c2 -h $NAME2 weave_on $HOST2 dns-add $C2b c2 -h $NAME2 diff --git a/testing/gossip/mocks.go b/testing/gossip/mocks.go index 43f7ee5a..34e65059 100644 --- a/testing/gossip/mocks.go +++ b/testing/gossip/mocks.go @@ -92,9 +92,24 @@ func (grouter *TestRouter) run(sender router.PeerName, gossiper router.Gossiper, continue } for _, msg := range message.data.Encode() { - if _, err := gossiper.OnGossipBroadcast(message.sender, msg); err != nil { + // TODO: this should call OnGossipBroadcast, and we should implement + // 'trickle' gossip correctly in this mock. But no one depends on this + // difference for testing right now, so we abuse the interface here. + diff, err := gossiper.OnGossip(msg) + if err != nil { panic(fmt.Sprintf("Error doing gossip broadcast: %s", err)) } + if diff == nil { + continue + } + // Sanity check - reconsuming the diff should yield nil + for _, diffMsg := range diff.Encode() { + if nextDiff, err := gossiper.OnGossip(diffMsg); err != nil { + panic(fmt.Sprintf("Error doing gossip broadcast: %s", err)) + } else if nextDiff != nil { + panic(fmt.Sprintf("Breach of gossip interface: %v != nil", nextDiff)) + } + } } } case <-gossipTimer: