agent/grpc/resolver: namespace the server ID with the DC name

So that if two datacenters end up with overlapping serverIDs we don't send requests to the wrong server
pull/8961/head
Daniel Nephin 2020-10-13 18:45:17 -04:00
parent bea3d0fd96
commit d8299670cc
2 changed files with 16 additions and 5 deletions

View File

@ -8,11 +8,12 @@ import (
"testing" "testing"
"time" "time"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/agent/grpc/internal/testservice" "github.com/hashicorp/consul/agent/grpc/internal/testservice"
"github.com/hashicorp/consul/agent/grpc/resolver" "github.com/hashicorp/consul/agent/grpc/resolver"
"github.com/hashicorp/consul/agent/metadata" "github.com/hashicorp/consul/agent/metadata"
"github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/stretchr/testify/require"
) )
func TestNewDialer_WithTLSWrapper(t *testing.T) { func TestNewDialer_WithTLSWrapper(t *testing.T) {

View File

@ -7,8 +7,9 @@ import (
"sync" "sync"
"time" "time"
"github.com/hashicorp/consul/agent/metadata"
"google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver"
"github.com/hashicorp/consul/agent/metadata"
) )
var registerLock sync.Mutex var registerLock sync.Mutex
@ -31,7 +32,7 @@ type ServerResolverBuilder struct {
// parallel testing because gRPC registers resolvers globally. // parallel testing because gRPC registers resolvers globally.
scheme string scheme string
// servers is an index of Servers by Server.ID. The map contains server IDs // servers is an index of Servers by Server.ID. The map contains server IDs
// for all datacenters, so it assumes the ID is globally unique. // for all datacenters.
servers map[string]*metadata.Server servers map[string]*metadata.Server
// resolvers is an index of connections to the serverResolver which manages // resolvers is an index of connections to the serverResolver which manages
// addresses of servers for that connection. // addresses of servers for that connection.
@ -131,7 +132,7 @@ func (s *ServerResolverBuilder) AddServer(server *metadata.Server) {
s.lock.Lock() s.lock.Lock()
defer s.lock.Unlock() defer s.lock.Unlock()
s.servers[server.ID] = server s.servers[uniqueID(server)] = server
addrs := s.getDCAddrs(server.Datacenter) addrs := s.getDCAddrs(server.Datacenter)
for _, resolver := range s.resolvers { for _, resolver := range s.resolvers {
@ -141,12 +142,21 @@ func (s *ServerResolverBuilder) AddServer(server *metadata.Server) {
} }
} }
// uniqueID returns a unique identifier for the server which includes the
// Datacenter and the ID.
//
// In practice it is expected that the server.ID is already a globally unique
// UUID. This function is an extra safeguard in case that ever changes.
func uniqueID(server *metadata.Server) string {
return server.Datacenter + "-" + server.ID
}
// RemoveServer updates the resolvers' states with the given server removed. // RemoveServer updates the resolvers' states with the given server removed.
func (s *ServerResolverBuilder) RemoveServer(server *metadata.Server) { func (s *ServerResolverBuilder) RemoveServer(server *metadata.Server) {
s.lock.Lock() s.lock.Lock()
defer s.lock.Unlock() defer s.lock.Unlock()
delete(s.servers, server.ID) delete(s.servers, uniqueID(server))
addrs := s.getDCAddrs(server.Datacenter) addrs := s.getDCAddrs(server.Datacenter)
for _, resolver := range s.resolvers { for _, resolver := range s.resolvers {