Merge pull request #3447 from hashicorp/issue-3070

Skips unique node ID check for old versions of Consul.
This commit is contained in:
James Phillips 2017-09-06 13:24:15 -07:00 committed by GitHub
commit 78ac144fff
5 changed files with 142 additions and 19 deletions

View File

@ -5,6 +5,7 @@ import (
"github.com/hashicorp/consul/agent/metadata"
"github.com/hashicorp/consul/types"
"github.com/hashicorp/go-version"
"github.com/hashicorp/serf/serf"
)
@ -18,6 +19,10 @@ type lanMergeDelegate struct {
segment string
}
// uniqueIDMinVersion is the lowest version where we insist that nodes
// have a unique ID.
var uniqueIDMinVersion = version.Must(version.NewVersion("0.8.5"))
func (md *lanMergeDelegate) NotifyMerge(members []*serf.Member) error {
nodeMap := make(map[types.NodeID]string)
for _, m := range members {
@ -37,7 +42,16 @@ func (md *lanMergeDelegate) NotifyMerge(members []*serf.Member) error {
return fmt.Errorf("Member '%s' has conflicting node ID '%s' with member '%s'",
m.Name, nodeID, other)
}
nodeMap[nodeID] = m.Name
// Only map nodes with a version that's >= than when
// we made host-based IDs opt-in, which helps prevent
// chaos when upgrading older clusters. See #3070 for
// more details.
if ver, err := metadata.Build(m); err == nil {
if ver.Compare(uniqueIDMinVersion) >= 0 {
nodeMap[nodeID] = m.Name
}
}
}
ok, dc := isConsulNode(*m)

View File

@ -8,7 +8,7 @@ import (
"github.com/hashicorp/serf/serf"
)
func makeNode(dc, name, id string, server bool) *serf.Member {
func makeNode(dc, name, id string, server bool, build string) *serf.Member {
var role string
if server {
role = "consul"
@ -23,7 +23,7 @@ func makeNode(dc, name, id string, server bool) *serf.Member {
"dc": dc,
"id": id,
"port": "8300",
"build": "0.7.5",
"build": build,
"vsn": "2",
"vsn_max": "3",
"vsn_min": "2",
@ -43,7 +43,8 @@ func TestMerge_LAN(t *testing.T) {
makeNode("dc2",
"node1",
"96430788-246f-4379-94ce-257f7429e340",
false),
false,
"0.7.5"),
},
expect: "wrong datacenter",
},
@ -53,7 +54,8 @@ func TestMerge_LAN(t *testing.T) {
makeNode("dc2",
"node1",
"96430788-246f-4379-94ce-257f7429e340",
true),
true,
"0.7.5"),
},
expect: "wrong datacenter",
},
@ -63,7 +65,8 @@ func TestMerge_LAN(t *testing.T) {
makeNode("dc1",
"node1",
"ee954a2f-80de-4b34-8780-97b942a50a99",
true),
true,
"0.7.5"),
},
expect: "with this agent's ID",
},
@ -73,11 +76,30 @@ func TestMerge_LAN(t *testing.T) {
makeNode("dc1",
"node1",
"6185913b-98d7-4441-bd8f-f7f7d854a4af",
true),
true,
"0.8.5"),
makeNode("dc1",
"node2",
"6185913b-98d7-4441-bd8f-f7f7d854a4af",
true),
true,
"0.9.0"),
},
expect: "with member",
},
// Cluster with existing conflicting node IDs, but version is
// old enough to skip the check.
{
members: []*serf.Member{
makeNode("dc1",
"node1",
"6185913b-98d7-4441-bd8f-f7f7d854a4af",
true,
"0.8.5"),
makeNode("dc1",
"node2",
"6185913b-98d7-4441-bd8f-f7f7d854a4af",
true,
"0.8.4"),
},
expect: "with member",
},
@ -87,11 +109,13 @@ func TestMerge_LAN(t *testing.T) {
makeNode("dc1",
"node1",
"6185913b-98d7-4441-bd8f-f7f7d854a4af",
true),
true,
"0.8.5"),
makeNode("dc1",
"node2",
"cda916bc-a357-4a19-b886-59419fcee50c",
true),
true,
"0.8.5"),
},
expect: "",
},
@ -128,7 +152,8 @@ func TestMerge_WAN(t *testing.T) {
makeNode("dc2",
"node1",
"96430788-246f-4379-94ce-257f7429e340",
false),
false,
"0.7.5"),
},
expect: "not a server",
},
@ -138,11 +163,13 @@ func TestMerge_WAN(t *testing.T) {
makeNode("dc2",
"node1",
"6185913b-98d7-4441-bd8f-f7f7d854a4af",
true),
true,
"0.7.5"),
makeNode("dc3",
"node2",
"cda916bc-a357-4a19-b886-59419fcee50c",
true),
true,
"0.7.5"),
},
expect: "",
},

12
agent/metadata/build.go Normal file
View File

@ -0,0 +1,12 @@
package metadata
import (
"github.com/hashicorp/go-version"
"github.com/hashicorp/serf/serf"
)
// Build extracts the Consul version info for a member.
func Build(m *serf.Member) (*version.Version, error) {
str := versionFormat.FindString(m.Tags["build"])
return version.NewVersion(str)
}

View File

@ -0,0 +1,75 @@
package metadata
import (
"testing"
"github.com/hashicorp/go-version"
"github.com/hashicorp/serf/serf"
"github.com/pascaldekloe/goe/verify"
)
func TestBuild(t *testing.T) {
tests := []struct {
desc string
m *serf.Member
ver *version.Version
err bool
}{
{
"no version",
&serf.Member{},
nil,
true,
},
{
"bad version",
&serf.Member{
Tags: map[string]string{
"build": "nope",
},
},
nil,
true,
},
{
"good version",
&serf.Member{
Tags: map[string]string{
"build": "0.8.5",
},
},
version.Must(version.NewVersion("0.8.5")),
false,
},
{
"rc version",
&serf.Member{
Tags: map[string]string{
"build": "0.9.3rc1:d62743c",
},
},
version.Must(version.NewVersion("0.9.3")),
false,
},
{
"ent version",
&serf.Member{
Tags: map[string]string{
"build": "0.9.3+ent:d62743c",
},
},
version.Must(version.NewVersion("0.9.3")),
false,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
ver, err := Build(tt.m)
gotErr := err != nil
if wantErr := tt.err; gotErr != wantErr {
t.Fatalf("got %v want %v", gotErr, wantErr)
}
verify.Values(t, "", ver, tt.ver)
})
}
}

View File

@ -1,8 +1,3 @@
// Package agent provides a logical endpoint for Consul agents in the
// network. agent data originates from Serf gossip and is primarily used to
// communicate Consul server information. Gossiped information that ends up
// in Server contains the necessary metadata required for servers.Manager to
// select which server an RPC request should be routed to.
package metadata
import (
@ -116,7 +111,7 @@ func IsConsulServer(m serf.Member) (bool, *Server) {
}
}
build_version, err := version.NewVersion(versionFormat.FindString(m.Tags["build"]))
build_version, err := Build(&m)
if err != nil {
return false, nil
}