From 5ffd50d3d3a4d59c03103a022f9a2556385d9bc6 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Thu, 15 Aug 2019 14:54:53 -0400 Subject: [PATCH] Filter out left/leaving serf members when determining if new ACLs can be used. --- agent/consul/util.go | 8 +++ agent/consul/util_test.go | 127 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) diff --git a/agent/consul/util.go b/agent/consul/util.go index 534cc1136551..e3d8be1b3af7 100644 --- a/agent/consul/util.go +++ b/agent/consul/util.go @@ -322,6 +322,14 @@ func ServersGetACLMode(members []serf.Member, leader string, datacenter string) continue } + if parts.Status != serf.StatusAlive && parts.Status != serf.StatusFailed { + // ignore any server that isn't alive or failed. We are considering failed + // because in this state there is a reasonable expectation that they could + // become stable again. Also autopilot should remove dead servers if they + // are truly gone. + continue + } + numServers += 1 if memberAddr := (&net.TCPAddr{IP: member.Addr, Port: parts.Port}).String(); memberAddr == leader { diff --git a/agent/consul/util_test.go b/agent/consul/util_test.go index 83969089e487..a0002c162471 100644 --- a/agent/consul/util_test.go +++ b/agent/consul/util_test.go @@ -7,6 +7,7 @@ import ( "regexp" "testing" + "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/go-version" "github.com/hashicorp/serf/serf" "github.com/stretchr/testify/require" @@ -622,3 +623,129 @@ func TestInterpolateHIL(t *testing.T) { }) } } + +func TestServersGetACLMode(t *testing.T) { + t.Parallel() + makeMember := func(role string, datacenter string, acls structs.ACLMode, status serf.MemberStatus, addr net.IP) serf.Member { + return serf.Member{ + Name: "foo", + Addr: addr, + Tags: map[string]string{ + "role": role, + "id": "asdf", + "dc": datacenter, + "port": "10000", + "build": "1.6.0", + "wan_join_port": "1234", + "vsn": "1", + "expect": "3", + "raft_vsn": "3", + "acls": string(acls), + }, + Status: status, + } + } + + type tcase struct { + members []serf.Member + leaderAddr string + datacenter string + numServers int + minMode structs.ACLMode + leaderMode structs.ACLMode + } + + cases := map[string]tcase{ + "filter-members": tcase{ + members: []serf.Member{ + makeMember("consul", "primary", structs.ACLModeLegacy, serf.StatusAlive, net.IP([]byte{127, 0, 0, 1})), + makeMember("consul", "primary", structs.ACLModeLegacy, serf.StatusFailed, net.IP([]byte{127, 0, 0, 2})), + // filter non-server + makeMember("client", "primary", structs.ACLModeUnknown, serf.StatusAlive, net.IP([]byte{127, 0, 0, 3})), + // filtered datacenter + makeMember("consul", "secondary", structs.ACLModeUnknown, serf.StatusAlive, net.IP([]byte{127, 0, 0, 4})), + // filtered status + makeMember("consul", "primary", structs.ACLModeUnknown, serf.StatusLeaving, net.IP([]byte{127, 0, 0, 5})), + // filtered status + makeMember("consul", "primary", structs.ACLModeUnknown, serf.StatusLeft, net.IP([]byte{127, 0, 0, 6})), + // filtered status + makeMember("consul", "primary", structs.ACLModeUnknown, serf.StatusNone, net.IP([]byte{127, 0, 0, 7})), + }, + numServers: 2, + leaderAddr: "127.0.0.1:10000", + datacenter: "primary", + minMode: structs.ACLModeLegacy, + leaderMode: structs.ACLModeLegacy, + }, + "disabled": tcase{ + members: []serf.Member{ + makeMember("consul", "primary", structs.ACLModeLegacy, serf.StatusAlive, net.IP([]byte{127, 0, 0, 1})), + makeMember("consul", "primary", structs.ACLModeUnknown, serf.StatusAlive, net.IP([]byte{127, 0, 0, 2})), + makeMember("consul", "primary", structs.ACLModeDisabled, serf.StatusAlive, net.IP([]byte{127, 0, 0, 3})), + }, + numServers: 3, + leaderAddr: "127.0.0.1:10000", + datacenter: "", + minMode: structs.ACLModeDisabled, + leaderMode: structs.ACLModeLegacy, + }, + "unknown": tcase{ + members: []serf.Member{ + makeMember("consul", "primary", structs.ACLModeLegacy, serf.StatusAlive, net.IP([]byte{127, 0, 0, 1})), + makeMember("consul", "primary", structs.ACLModeUnknown, serf.StatusAlive, net.IP([]byte{127, 0, 0, 2})), + }, + numServers: 2, + leaderAddr: "127.0.0.1:10000", + datacenter: "", + minMode: structs.ACLModeUnknown, + leaderMode: structs.ACLModeLegacy, + }, + "legacy": tcase{ + members: []serf.Member{ + makeMember("consul", "primary", structs.ACLModeEnabled, serf.StatusAlive, net.IP([]byte{127, 0, 0, 1})), + makeMember("consul", "primary", structs.ACLModeLegacy, serf.StatusAlive, net.IP([]byte{127, 0, 0, 2})), + }, + numServers: 2, + leaderAddr: "127.0.0.1:10000", + datacenter: "", + minMode: structs.ACLModeLegacy, + leaderMode: structs.ACLModeEnabled, + }, + "enabled": tcase{ + members: []serf.Member{ + makeMember("consul", "primary", structs.ACLModeEnabled, serf.StatusAlive, net.IP([]byte{127, 0, 0, 1})), + makeMember("consul", "primary", structs.ACLModeEnabled, serf.StatusAlive, net.IP([]byte{127, 0, 0, 2})), + makeMember("consul", "primary", structs.ACLModeEnabled, serf.StatusAlive, net.IP([]byte{127, 0, 0, 3})), + }, + numServers: 3, + leaderAddr: "127.0.0.1:10000", + datacenter: "", + minMode: structs.ACLModeEnabled, + leaderMode: structs.ACLModeEnabled, + }, + "failed-members": tcase{ + members: []serf.Member{ + makeMember("consul", "primary", structs.ACLModeLegacy, serf.StatusAlive, net.IP([]byte{127, 0, 0, 1})), + makeMember("consul", "primary", structs.ACLModeUnknown, serf.StatusFailed, net.IP([]byte{127, 0, 0, 2})), + makeMember("consul", "primary", structs.ACLModeLegacy, serf.StatusFailed, net.IP([]byte{127, 0, 0, 3})), + }, + numServers: 3, + leaderAddr: "127.0.0.1:10000", + datacenter: "", + minMode: structs.ACLModeUnknown, + leaderMode: structs.ACLModeLegacy, + }, + } + + for name, tc := range cases { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + actualServers, actualMinMode, actualLeaderMode := ServersGetACLMode(tc.members, tc.leaderAddr, tc.datacenter) + + require.Equal(t, tc.numServers, actualServers) + require.Equal(t, tc.minMode, actualMinMode) + require.Equal(t, tc.leaderMode, actualLeaderMode) + }) + } +}