Avoid to taking into account wrong versions of protocols in Vsn. (#178)

* Avoid to take into account wrong versions of protocols in Vsn.

On Consul, sometimes, nodes do send a pMin = pMan = 0 in Vsn
This causes a corruption of the acceptable versions of protocol
and thus requiring version = [0, 1].

After this corruption occurs, all new nodes cannot join anymore, it
then force the restart of all Consul servers to resume normal
operations.

While not fixing the root cause, this patch discards alive nodes
claiming version 0,0,0 and will avoid this breakage.

See hashicorp/consul#3217

* Always set the Vsn when creating state, so race condition cannot happen

* Do not move m.encodeBroadcastNotify(a.Node, aliveMsg, a, notify) since not needed

* Test the bare minimum for size of Vsn

Co-Authored-By: pierresouchay <[email protected]>

* Fixed test TestMemberList_ProbeNode_Awareness_OldProtocol

* Avoid to crash when len(Vsn) is incorrect and ignore the message when there is an Alive delegate

Author: pierresouchay

memberlist.go

@@ -72,6 +72,15 @@ type Memberlist struct {
 	logger *log.Logger
 }
 
+// BuildVsnArray creates the array of Vsn
+func (conf *Config) BuildVsnArray() []uint8 {
+	return []uint8{
+		ProtocolVersionMin, ProtocolVersionMax, conf.ProtocolVersion,
+		conf.DelegateProtocolMin, conf.DelegateProtocolMax,
+		conf.DelegateProtocolVersion,
+	}
+}
+
 // newMemberlist creates the network listeners.
 // Does not schedule execution of background maintenance.
 func newMemberlist(conf *Config) (*Memberlist, error) {
@@ -402,11 +411,7 @@ func (m *Memberlist) setAlive() error {
 		Addr:        addr,
 		Port:        uint16(port),
 		Meta:        meta,
-		Vsn: []uint8{
-			ProtocolVersionMin, ProtocolVersionMax, m.config.ProtocolVersion,
-			m.config.DelegateProtocolMin, m.config.DelegateProtocolMax,
-			m.config.DelegateProtocolVersion,
-		},
+		Vsn:         m.config.BuildVsnArray(),
 	}
 	m.aliveNode(&a, nil, true)
 	return nil
@@ -447,11 +452,7 @@ func (m *Memberlist) UpdateNode(timeout time.Duration) error {
 		Addr:        state.Addr,
 		Port:        state.Port,
 		Meta:        meta,
-		Vsn: []uint8{
-			ProtocolVersionMin, ProtocolVersionMax, m.config.ProtocolVersion,
-			m.config.DelegateProtocolMin, m.config.DelegateProtocolMax,
-			m.config.DelegateProtocolVersion,
-		},
+		Vsn:         m.config.BuildVsnArray(),
 	}
 	notifyCh := make(chan struct{})
 	m.aliveNode(&a, notifyCh, true)

net_test.go

@@ -569,6 +569,10 @@ func TestSendMsg_Piggyback(t *testing.T) {
 		Node:        "rand",
 		Addr:        []byte{127, 0, 0, 255},
 		Meta:        nil,
+		Vsn: []uint8{
+			ProtocolVersionMin, ProtocolVersionMax, ProtocolVersionMin,
+			1, 1, 1,
+		},
 	}
 	m.encodeAndBroadcast("rand", aliveMsg, &a)
 

state.go

@@ -850,11 +850,26 @@ func (m *Memberlist) aliveNode(a *alive, notify chan struct{}, bootstrap bool) {
 		return
 	}
 
+	if len(a.Vsn) >= 3 {
+		pMin := a.Vsn[0]
+		pMax := a.Vsn[1]
+		pCur := a.Vsn[2]
+		if pMin == 0 || pMax == 0 || pMin > pMax {
+			m.logger.Printf("[WARN] memberlist: Ignoring an alive message for '%s' (%v:%d) because protocol version(s) are wrong: %d <= %d <= %d should be >0", a.Node, net.IP(a.Addr), a.Port, pMin, pCur, pMax)
+			return
+		}
+	}
+
 	// Invoke the Alive delegate if any. This can be used to filter out
 	// alive messages based on custom logic. For example, using a cluster name.
 	// Using a merge delegate is not enough, as it is possible for passive
 	// cluster merging to still occur.
 	if m.config.Alive != nil {
+		if len(a.Vsn) < 6 {
+			m.logger.Printf("[WARN] memberlist: ignoring alive message for '%s' (%v:%d) because Vsn is not present",
+				a.Node, net.IP(a.Addr), a.Port)
+			return
+		}
 		node := &Node{
 			Name: a.Node,
 			Addr: a.Addr,
@@ -886,6 +901,14 @@ func (m *Memberlist) aliveNode(a *alive, notify chan struct{}, bootstrap bool) {
 			},
 			State: stateDead,
 		}
+		if len(a.Vsn) > 5 {
+			state.PMin = a.Vsn[0]
+			state.PMax = a.Vsn[1]
+			state.PCur = a.Vsn[2]
+			state.DMin = a.Vsn[3]
+			state.DMax = a.Vsn[4]
+			state.DCur = a.Vsn[5]
+		}
 
 		// Add to map
 		m.nodeMap[a.Node] = state
@@ -965,9 +988,8 @@ func (m *Memberlist) aliveNode(a *alive, notify chan struct{}, bootstrap bool) {
 			bytes.Equal(a.Vsn, versions) {
 			return
 		}
-
 		m.refute(state, a.Incarnation)
-		m.logger.Printf("[WARN] memberlist: Refuting an alive message")
+		m.logger.Printf("[WARN] memberlist: Refuting an alive message for '%s' (%v:%d) meta:(%v VS %v), vsn:(%v VS %v)", a.Node, net.IP(a.Addr), a.Port, a.Meta, state.Meta, a.Vsn, versions)
 	} else {
 		m.encodeBroadcastNotify(a.Node, aliveMsg, a, notify)