Filter out left/leaving serf members when determining if new AC… (#6332)

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 {

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)
+		})
+	}
+}