Add release v1.4.6

Change invariant fail on Merge() from panic to error

Fixes #2083.

CHANGELOG.md

@@ -1,3 +1,19 @@
+## Release 1.4.6
+
+Fixes some issues encountered by our users.
+
+* Restarting a peer could leave stale entries in WeaveDNS
+  #1867/#2023/#2081
+* Weave proxy occasionally failing to attach any new containers
+  #2016/#2049
+
+Other fixes:
+
+* Resolved a crash when a restarting peer re-connected to a peer that
+  had not received the latest IPAM data #2083/#2092
+* Make IP address space available immediately after a dead peer is
+  removed #2068
+
 ## Release 1.4.5
 
 Higher performance for multicast and broadcast traffic when using

common/docker/client.go

@@ -4,12 +4,18 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"time"
 
 	docker "github.com/fsouza/go-dockerclient"
 
 	. "github.com/weaveworks/weave/common"
 )
 
+const (
+	InitialInterval = 1 * time.Second
+	MaxInterval     = 20 * time.Second
+)
+
 // An observer for container events
 type ContainerObserver interface {
 	ContainerStarted(ident string)
@@ -72,21 +78,33 @@ func (c *Client) Info() string {
 
 // AddObserver adds an observer for docker events
 func (c *Client) AddObserver(ob ContainerObserver) error {
-	events := make(chan *docker.APIEvents)
-	if err := c.AddEventListener(events); err != nil {
-		Log.Errorf("[docker] Unable to add listener to Docker API: %s", err)
-		return err
-	}
-
 	go func() {
-		for event := range events {
+		retryInterval := InitialInterval
-			switch event.Status {
+		for {
-			case "start":
+			events := make(chan *docker.APIEvents)
-				id := event.ID
+			if err := c.AddEventListener(events); err != nil {
-				ob.ContainerStarted(id)
+				Log.Errorf("[docker] Unable to add listener to Docker API: %s - retrying in %ds", err, retryInterval/time.Second)
-			case "die":
+			} else {
-				id := event.ID
+				start := time.Now()
-				ob.ContainerDied(id)
+				for event := range events {
+					switch event.Status {
+					case "start":
+						id := event.ID
+						ob.ContainerStarted(id)
+					case "die":
+						id := event.ID
+						ob.ContainerDied(id)
+					}
+				}
+				if time.Since(start) > retryInterval {
+					retryInterval = InitialInterval
+				}
+				Log.Errorf("[docker] Event listener channel closed - retrying subscription in %ds", retryInterval/time.Second)
+			}
+			time.Sleep(retryInterval)
+			retryInterval = retryInterval * 3 / 2
+			if retryInterval > MaxInterval {
+				retryInterval = MaxInterval
 			}
 		}
 	}()

ipam/allocator.go

@@ -334,7 +334,10 @@ func (alloc *Allocator) AdminTakeoverRanges(peerNameOrNickname string) error {
 		}
 
 		newRanges, err := alloc.ring.Transfer(peername, alloc.ourName)
-		alloc.space.AddRanges(newRanges)
+		if err == nil && len(newRanges) > 0 {
+			alloc.space.AddRanges(newRanges)
+			alloc.gossip.GossipBroadcast(alloc.Gossip())
+		}
 		resultChan <- err
 	}
 	return <-resultChan

ipam/ring/ring.go

@@ -46,33 +46,37 @@ var (
 )
 
 func (r *Ring) checkInvariants() error {
-	if !sort.IsSorted(r.Entries) {
+	return r.checkEntries(r.Entries)
+}
+
+func (r *Ring) checkEntries(entries entries) error {
+	if !sort.IsSorted(entries) {
 		return ErrNotSorted
 	}
 
 	// Check no token appears twice
 	// We know it's sorted...
-	for i := 1; i < len(r.Entries); i++ {
+	for i := 1; i < len(entries); i++ {
-		if r.Entries[i-1].Token == r.Entries[i].Token {
+		if entries[i-1].Token == entries[i].Token {
 			return ErrTokenRepeated
 		}
 	}
 
-	if len(r.Entries) == 0 {
+	if len(entries) == 0 {
 		return nil
 	}
 
 	// Check tokens are in range
-	if r.Entries.entry(0).Token < r.Start {
+	if entries.entry(0).Token < r.Start {
 		return ErrTokenOutOfRange
 	}
-	if r.Entries.entry(-1).Token >= r.End {
+	if entries.entry(-1).Token >= r.End {
 		return ErrTokenOutOfRange
 	}
 
 	// Check all the freespaces are in range
-	for i, entry := range r.Entries {
+	for i, entry := range entries {
-		next := r.Entries.entry(i + 1)
+		next := entries.entry(i + 1)
 		distance := r.distance(entry.Token, next.Token)
 
 		if entry.Free > distance {
@@ -181,7 +185,6 @@ func (r *Ring) GrantRangeToHost(start, end address.Address, peer mesh.PeerName)
 // Merge the given ring into this ring and return any new ranges added
 func (r *Ring) Merge(gossip Ring) error {
 	r.assertInvariants()
-	defer r.assertInvariants()
 	defer r.updateExportedVariables()
 
 	// Don't panic when checking the gossiped in ring.
@@ -267,6 +270,10 @@ func (r *Ring) Merge(gossip Ring) error {
 		previousOwner = nil
 	}
 
+	if err := r.checkEntries(result); err != nil {
+		return fmt.Errorf("Merge of incoming data causes: %s", err)
+	}
+
 	if len(r.Seeds) == 0 {
 		r.Seeds = gossip.Seeds
 	}

mesh/connection_maker.go

@@ -32,6 +32,7 @@ const (
 	TargetWaiting    TargetState = iota // we are waiting to connect there
 	TargetAttempting                    // we are attempting to connect there
 	TargetConnected                     // we are connected to there
+	TargetSuspended                     // not currently attempting to connect here
 )
 
 // Information about an address where we may find a peer
@@ -266,16 +267,23 @@ func (cm *ConnectionMaker) connectToTargets(validTarget map[string]struct{}, dir
 	now := time.Now() // make sure we catch items just added
 	after := MaxDuration
 	for address, target := range cm.targets {
-		if target.state != TargetWaiting {
+		if target.state != TargetWaiting && target.state != TargetSuspended {
 			continue
 		}
 		if _, valid := validTarget[address]; !valid {
-			delete(cm.targets, address)
+			// Not valid: suspend reconnects if direct peer,
+			// otherwise forget this target entirely
+			if _, direct := directTarget[address]; direct {
+				target.state = TargetSuspended
+			} else {
+				delete(cm.targets, address)
+			}
 			continue
 		}
 		if target.tryAfter.IsZero() {
 			continue
 		}
+		target.state = TargetWaiting
 		switch duration := target.tryAfter.Sub(now); {
 		case duration <= 0:
 			target.state = TargetAttempting

mesh/status.go

@@ -181,6 +181,7 @@ func NewLocalConnectionStatusSlice(cm *ConnectionMaker) []LocalConnectionStatus
 					add("retrying", target.lastError.Error())
 				}
 			case TargetConnected:
+			case TargetSuspended:
 			}
 		}
 		resultChan <- slice

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
 }
@@ -105,6 +108,11 @@ func (e1 *Entry) addLowercase() {
 	e1.lHostname = strings.ToLower(e1.Hostname)
 }
 
+func (e1 *Entry) tombstone() {
+	e1.Version++
+	e1.Tombstone = now()
+}
+
 func check(es SortableEntries) error {
 	if !sort.IsSorted(es) {
 		return fmt.Errorf("Not sorted!")
@@ -182,8 +190,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()
-			e.Tombstone = now()
 			(*es)[i] = e
 			tombstoned = append(tombstoned, e)
 		}
@@ -191,6 +198,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 +213,16 @@ 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
+	}
+	return nil, false
+}
+
 func (es Entries) lookup(hostname string) Entries {
 	es.checkAndPanic()
 

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) {

nameserver/nameserver.go

@@ -203,11 +203,31 @@ func (n *Nameserver) receiveGossip(msg []byte) (mesh.GossipData, mesh.GossipData
 		return nil, nil, fmt.Errorf("clock skew of %d detected", delta)
 	}
 
+	// 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 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 {
-		return n.isKnownPeer(e.Origin)
+		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)

site/ipam.md

@@ -48,17 +48,6 @@ time, you can give the number of peers like this:
 
     host3$ weave launch --init-peer-count 3 $HOST2
 
-The consensus mechanism used to determine a majority transitions
-through three states: 'deferred', 'waiting' and 'achieved':
-
-* 'deferred' - no allocation requests or claims have been made yet;
-  consensus is deferred until then
-* 'waiting' - an attempt to achieve consensus is ongoing, triggered by
-  an allocation or claim request; allocations will block. This state
-  persists until a quorum of peers are able to communicate amongst
-  themselves successfully
-* 'achieved' - consensus achieved; allocations proceed normally
-
 ### More on `--init-peer-count`
 
 TL;DR: it isn't a problem to over-estimate by a bit, but if you supply
@@ -214,7 +203,7 @@ reports on the current status of the weave router and IP allocator:
 ...
 
        Service: ipam
-     Consensus: waiting(quorum: 2, known: 0)
+        Status: awaiting consensus (quorum: 2, known: 0)
          Range: 10.32.0.0-10.47.255.255
  DefaultSubnet: 10.32.0.0/12
 
@@ -225,4 +214,21 @@ The first section covers the router; see the [troubleshooting
 guide](troubleshooting.html#weave-status) for full details.
 
 The 'Service: ipam' section displays the consensus state as well as
-the total allocation range and default subnet.
+the total allocation range and default subnet. Columns are as follows:
+
+* 'Status' - allocator state
+    * 'idle' - no allocation requests or claims have been made yet;
+      consensus is deferred until then
+    * 'awaiting consensus' - an attempt to achieve consensus is
+      ongoing, triggered by an allocation or claim request;
+      allocations will block.  This state persists until a quorum of
+      peers are able to communicate amongst themselves successfully
+    * 'ready' - consensus achieved; allocations proceed normally
+    * 'waiting for IP range grant from peers' - peer has exhausted its
+      agreed portion of the range and is waiting to be granted some
+      more
+    * 'all IP ranges owned by unreachable peers' - peer has exhausted
+      its agreed portion of the range but cannot reach anyone to ask
+      for more
+* 'Range' - total allocation range set by `--ipalloc-range`
+* 'DefaultSubnet' - default subnet set by `--ipalloc-default-subnet`

site/troubleshooting.md

@@ -62,7 +62,7 @@ $ weave status
  TrustedSubnets: none
 
         Service: ipam
-      Consensus: achieved
+         Status: ready
           Range: 10.32.0.0-10.47.255.255
   DefaultSubnet: 10.32.0.0/12
 

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

test/690_proxy_config_test.sh

@@ -34,8 +34,8 @@ assert_raises "proxy docker_on $HOST1 ps"
 weave_on $HOST1 stop-proxy
 
 # Booting it over tls errors
-assert_raises "DOCKER_CLIENT_ARGS='--tls' weave_on $HOST1 launch-proxy" 1
+assert_raises "! DOCKER_CLIENT_ARGS='--tls' weave_on $HOST1 launch-proxy"
-assert_raises "DOCKER_CERT_PATH='./tls' DOCKER_TLS_VERIFY=1 weave_on $HOST1 launch-proxy" 1
+assert_raises "! DOCKER_CERT_PATH='./tls' DOCKER_TLS_VERIFY=1 weave_on $HOST1 launch-proxy"
 
 # Booting it with a specific -H overrides defaults
 weave_on $HOST1 launch-proxy -H tcp://0.0.0.0:12345

test/695_proxy_failure_modes_test.sh

@@ -6,7 +6,7 @@ start_suite "Proxy failure modes"
 
 # docker run should fail when weave router is not running
 weave_on $HOST1 launch-proxy
-assert_raises "proxy docker_on $HOST1 run --rm $SMALL_IMAGE true" 1
+assert_raises "! proxy docker_on $HOST1 run --rm $SMALL_IMAGE true"
 assert_raises "proxy docker_on $HOST1 run --rm $SMALL_IMAGE true 2>&1 1>/dev/null | grep 'Error response from daemon: weave container is not present. Have you launched it?'"
 
 end_suite

test/gce.sh

@@ -11,7 +11,7 @@ set -e
 : ${SSH_KEY_FILE:=$HOME/.ssh/gce_ssh_key}
 : ${PROJECT:=positive-cocoa-90213}
 : ${IMAGE:=ubuntu-14-04}
-: ${TEMPLATE_NAME:=test-template-9}
+: ${TEMPLATE_NAME:=test-template-10}
 : ${ZONE:=us-central1-a}
 : ${NUM_HOSTS:=5}
 SUFFIX=""