From 7e0ae6112c3a3392a453d463fc239f7f90c5d01a Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 2 Oct 2015 16:33:28 +0100 Subject: [PATCH 1/3] detect when IPAM has been seeded by different peers This is an early detector for a multitude of other symptoms, some of which cause panics. When creating the ring we record the paxos-supplied normalised list of peers used for seeding. This subsequently gets gossipped around as part of the Ring data structure, and checked in ring.Merge that it is the same for the recipient. There are some subtleties due to the desire to maintain backward compatibility and deal with empty rings - The gossip gob decoder will ignore the Ring.Seeds field when an old weave receives a Ring from a new weave. So everything on the old weave will just continue to work as before. - The gossip gob decoder will set the Ring.Seeds field to nil when a new weave receives a Ring from an old weave. We therefore omit the seed equivalence check when the received Seeds slice is empty. - Our ring may not have any seeds - either because it is empty or was created from a ring received from an old weave. We therefore a) omit the seed equivalence check in that situation, and b) set our seeds to those received. Note that this allows us to learn about the seeds well after ring creation - a situation which may arise when receiving gossip from a mixture of old and new weaves. Fixes #1463. --- ipam/ring/ring.go | 24 +++++++++++++++++++++--- test/180_ipalloc_seed_mismatch_3.sh | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) create mode 100755 test/180_ipalloc_seed_mismatch_3.sh diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index 6021efcc..b82bad1d 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -19,9 +19,10 @@ import ( // Ring represents the ring itself type Ring struct { - Start, End address.Address // [min, max) tokens in this ring. Due to wrapping, min == max (effectively) - Peer router.PeerName // name of peer owning this ring instance - Entries entries // list of entries sorted by token + Start, End address.Address // [min, max) tokens in this ring. Due to wrapping, min == max (effectively) + Peer router.PeerName // name of peer owning this ring instance + Entries entries // list of entries sorted by token + Seeds []router.PeerName // peers with which the ring was seeded } func (r *Ring) assertInvariants() { @@ -36,6 +37,7 @@ var ( ErrNotSorted = errors.New("Ring not sorted") ErrTokenRepeated = errors.New("Token appears twice in ring") ErrTokenOutOfRange = errors.New("Token is out of range") + ErrDifferentSeeds = errors.New("Received ring was seeded differently from ours!") ErrDifferentRange = errors.New("Received range differs from ours!") ErrNewerVersion = errors.New("Received new version for entry I own!") ErrInvalidEntry = errors.New("Received invalid state update!") @@ -188,6 +190,17 @@ func (r *Ring) Merge(gossip Ring) error { return err } + if len(gossip.Seeds) > 0 && len(r.Seeds) > 0 { + if len(gossip.Seeds) != len(r.Seeds) { + return ErrDifferentSeeds + } + for i, seed := range gossip.Seeds { + if seed != r.Seeds[i] { + return ErrDifferentSeeds + } + } + } + if r.Start != gossip.Start || r.End != gossip.End { return ErrDifferentRange } @@ -254,6 +267,9 @@ func (r *Ring) Merge(gossip Ring) error { previousOwner = nil } + if len(r.Seeds) == 0 { + r.Seeds = gossip.Seeds + } r.Entries = result return nil } @@ -330,6 +346,8 @@ func (r *Ring) ClaimForPeers(peers []router.PeerName) { } common.Assert(pos == r.End) + + r.Seeds = peers } func (r *Ring) FprintWithNicknames(w io.Writer, m map[router.PeerName]string) { diff --git a/test/180_ipalloc_seed_mismatch_3.sh b/test/180_ipalloc_seed_mismatch_3.sh new file mode 100755 index 00000000..fd7c412f --- /dev/null +++ b/test/180_ipalloc_seed_mismatch_3.sh @@ -0,0 +1,28 @@ +#! /bin/bash + +. ./config.sh + +start_suite "IPAM ring seed mismatch" + +weave_on $HOST1 launch-router --no-discovery +weave_on $HOST2 launch-router --no-discovery +weave_on $HOST3 launch-router --no-discovery + +# Do some allocations to get the ring initialized on two hosts +start_container $HOST1 +start_container $HOST2 + +# Connect 2 to 3 and allocate so they will each have half the ring +weave_on $HOST2 connect $HOST3 +start_container $HOST3 + +# More allocations on 1 to increase the version number +start_container $HOST1 +start_container $HOST1 + +weave_on $HOST1 connect $HOST3 + +# Check that weave on 3 is still running +assert_raises "weave_on $HOST3 status" + +end_suite From 5a45471c6855fcfb331c2d4986552b7c82fbef8f Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 6 Oct 2015 14:20:42 +0100 Subject: [PATCH 2/3] more readable errors on ring.Merge There are three significant changes here: - rewrite ring.ErrDifferentSeeds errors - remove the pre-checking of ranges and instead rewrite ring.ErrDifferentRange errors - don't pruneNicknames() and signal ringUpdated() when there was an error We don't rewrite other errors since our current belief is that only ErrDifferentSeeds and ErrDifferentRange can arise from user error. --- ipam/allocator.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index d3c0e0e2..a6d17eaf 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -618,16 +618,20 @@ func (alloc *Allocator) update(sender router.PeerName, msg []byte) error { // shouldn't get updates for a empty Ring. But tolerate // them just in case. if data.Ring != nil { - if data.Ring.Range() != alloc.universe { - return fmt.Errorf("Incompatible IP allocation range %s; ours is %s", - data.Ring.Range().AsCIDRString(), alloc.universe.AsCIDRString()) + switch err = alloc.ring.Merge(*data.Ring); err { + case ring.ErrDifferentSeeds: + return fmt.Errorf("IP allocation was seeded by different peers (received: %v, ours: %v)", + data.Ring.Seeds, alloc.ring.Seeds) + case ring.ErrDifferentRange: + return fmt.Errorf("Incompatible IP allocation ranges (received: %s, ours: %s)", + data.Ring.Range().AsCIDRString(), alloc.ring.Range().AsCIDRString()) + default: + if err == nil && !alloc.ring.Empty() { + alloc.pruneNicknames() + alloc.ringUpdated() + } + return err } - err = alloc.ring.Merge(*data.Ring) - if !alloc.ring.Empty() { - alloc.pruneNicknames() - alloc.ringUpdated() - } - return err } if data.Paxos != nil { From 8aaf4cfaf00ef70f233b411720b607a89d4bcc1f Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 6 Oct 2015 14:41:41 +0100 Subject: [PATCH 3/3] include nicknames in error message, if we have them --- ipam/allocator.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index a6d17eaf..b93fb4e4 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -354,6 +354,18 @@ func (alloc *Allocator) pruneNicknames() { } } +func (alloc *Allocator) annotatePeernames(names []router.PeerName) []string { + var res []string + for _, name := range names { + if nickname, found := alloc.nicknames[name]; found { + res = append(res, fmt.Sprint(name, "(", nickname, ")")) + } else { + res = append(res, name.String()) + } + } + return res +} + func decodeRange(msg []byte) (r address.Range, err error) { decoder := gob.NewDecoder(bytes.NewReader(msg)) return r, decoder.Decode(&r) @@ -621,7 +633,7 @@ func (alloc *Allocator) update(sender router.PeerName, msg []byte) error { switch err = alloc.ring.Merge(*data.Ring); err { case ring.ErrDifferentSeeds: return fmt.Errorf("IP allocation was seeded by different peers (received: %v, ours: %v)", - data.Ring.Seeds, alloc.ring.Seeds) + alloc.annotatePeernames(data.Ring.Seeds), alloc.annotatePeernames(alloc.ring.Seeds)) case ring.ErrDifferentRange: return fmt.Errorf("Incompatible IP allocation ranges (received: %s, ours: %s)", data.Ring.Range().AsCIDRString(), alloc.ring.Range().AsCIDRString())