diff --git a/nomad/autopilot.go b/nomad/autopilot.go index a74e3455826..1a54387c753 100644 --- a/nomad/autopilot.go +++ b/nomad/autopilot.go @@ -6,7 +6,6 @@ package nomad import ( "context" "fmt" - "strconv" metrics "github.com/hashicorp/go-metrics/compat" "github.com/hashicorp/nomad/helper" @@ -101,7 +100,7 @@ func (d *AutopilotDelegate) RemoveFailedServer(failedSrv *autopilot.Server) { // MinRaftProtocol returns the lowest supported Raft protocol among alive // servers func (s *Server) MinRaftProtocol() (int, error) { - return minRaftProtocol(s.serf.Members(), peers.IsNomadServer) + return minRaftProtocol(s.peersCache.RegionPeers(s.Region())) } // GetClusterHealth is used to get the current health of the servers, as known @@ -172,32 +171,15 @@ func stringIDs(ids []raft.ServerID) []string { return helper.ConvertSlice(ids, func(id raft.ServerID) string { return string(id) }) } -func minRaftProtocol(members []serf.Member, serverFunc func(serf.Member) (bool, *peers.Parts)) (int, error) { +func minRaftProtocol(members []*peers.Parts) (int, error) { minVersion := -1 for _, m := range members { if m.Status != serf.StatusAlive { continue } - ok, server := serverFunc(m) - if !ok { - return -1, fmt.Errorf("not a Nomad server") - } - if server == nil { - continue - } - - vsn, ok := m.Tags["raft_vsn"] - if !ok { - vsn = "1" - } - raftVsn, err := strconv.Atoi(vsn) - if err != nil { - return -1, err - } - - if minVersion == -1 || raftVsn < minVersion { - minVersion = raftVsn + if minVersion == -1 || m.RaftVersion < minVersion { + minVersion = m.RaftVersion } } @@ -209,37 +191,22 @@ func minRaftProtocol(members []serf.Member, serverFunc func(serf.Member) (bool, } func (s *Server) autopilotServers() map[raft.ServerID]*autopilot.Server { - servers := make(map[raft.ServerID]*autopilot.Server) - for _, member := range s.serf.Members() { - srv, err := s.autopilotServer(member) - if err != nil { - s.logger.Warn("Error parsing server info", "name", member.Name, "error", err) - continue - } else if srv == nil { - // this member was a client or in another region - continue - } - - servers[srv.ID] = srv - } + // Get a list of the regional peers for our local region. We cannot use the + // LocalPeers function, as we need peers in all states. + localPeers := s.peersCache.RegionPeers(s.Region()) - return servers -} + servers := make(map[raft.ServerID]*autopilot.Server, len(localPeers)) -func (s *Server) autopilotServer(m serf.Member) (*autopilot.Server, error) { - ok, srv := peers.IsNomadServer(m) - if !ok { - return nil, nil - } - if srv.Region != s.Region() { - return nil, nil + for _, srv := range localPeers { + autopilotServer := s.autopilotServerFromMetadata(srv) + servers[autopilotServer.ID] = autopilotServer } - return s.autopilotServerFromMetadata(srv) + return servers } -func (s *Server) autopilotServerFromMetadata(srv *peers.Parts) (*autopilot.Server, error) { +func (s *Server) autopilotServerFromMetadata(srv *peers.Parts) *autopilot.Server { server := &autopilot.Server{ Name: srv.Name, ID: raft.ServerID(srv.ID), @@ -247,6 +214,7 @@ func (s *Server) autopilotServerFromMetadata(srv *peers.Parts) (*autopilot.Serve Version: srv.Build.String(), RaftVersion: srv.RaftVersion, Ext: s.autopilotServerExt(srv), + Meta: srv.Tags, } switch srv.Status { @@ -262,13 +230,5 @@ func (s *Server) autopilotServerFromMetadata(srv *peers.Parts) (*autopilot.Serve server.NodeStatus = autopilot.NodeUnknown } - members := s.serf.Members() - for _, member := range members { - if member.Name == srv.Name { - server.Meta = member.Tags - break - } - } - - return server, nil + return server } diff --git a/nomad/peers/peers.go b/nomad/peers/peers.go index 50e44857bc6..0675fb3b055 100644 --- a/nomad/peers/peers.go +++ b/nomad/peers/peers.go @@ -44,6 +44,11 @@ type Parts struct { Status serf.MemberStatus NonVoter bool + // Tags are the full Serf tags for the member. This duplicates some of the + // data which is also stored in other fields for convenience, so adds a + // little size overhead. It is included as Autopilot uses them. + Tags map[string]string + // Deprecated: Functionally unused but needs to always be set by 1 for // compatibility with v1.2.x and earlier. MajorVersion int @@ -133,6 +138,7 @@ func IsNomadServer(m serf.Member) (bool, *Parts) { RaftVersion: raftVsn, Status: m.Status, NonVoter: nonVoter, + Tags: m.Tags, MajorVersion: deprecatedAPIMajorVersion, } return true, parts diff --git a/nomad/peers/peers_test.go b/nomad/peers/peers_test.go index dde40c05084..acb54dfbb83 100644 --- a/nomad/peers/peers_test.go +++ b/nomad/peers/peers_test.go @@ -94,6 +94,7 @@ func TestIsNomadServer(t *testing.T) { must.Eq(t, 7, parts.Build.Segments()[1]) must.Eq(t, 0, parts.Build.Segments()[2]) must.True(t, parts.NonVoter) + must.MapEq(t, m.Tags, parts.Tags) m.Tags["bootstrap"] = "1" valid, parts = IsNomadServer(m)