diff --git a/client/servers/manager.go b/client/servers/manager.go index 6dac0c7e4..2156708e8 100644 --- a/client/servers/manager.go +++ b/client/servers/manager.go @@ -74,6 +74,16 @@ func (s *Server) String() string { return s.addr } +func (s *Server) Equal(o *Server) bool { + if s == nil && o == nil { + return true + } else if s == nil && o != nil || s != nil && o == nil { + return false + } + + return s.Addr.String() == o.Addr.String() && s.DC == o.DC +} + type Servers []*Server func (s Servers) String() string { @@ -160,6 +170,9 @@ func (m *Manager) Start() { func (m *Manager) SetServers(servers Servers) { m.Lock() defer m.Unlock() + + // Randomize the incoming servers + servers.shuffle() m.servers = servers } @@ -204,7 +217,7 @@ func (m *Manager) NotifyFailedServer(s *Server) { // If the server being failed is not the first server on the list, // this is a noop. If, however, the server is failed and first on // the list, move the server to the end of the list. - if len(m.servers) > 1 && m.servers[0] == s { + if len(m.servers) > 1 && m.servers[0].Equal(s) { m.servers.cycle() } } diff --git a/client/servers/manager_test.go b/client/servers/manager_test.go index deea7f48f..b7c47d1ce 100644 --- a/client/servers/manager_test.go +++ b/client/servers/manager_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/servers" + "github.com/hashicorp/nomad/helper/testlog" ) type fauxAddr struct { @@ -32,22 +33,22 @@ func (cp *fauxConnPool) Ping(net.Addr) error { return fmt.Errorf("bad server") } -func testManager() (m *servers.Manager) { - logger := log.New(os.Stderr, "", log.LstdFlags) +func testManager(t *testing.T) (m *servers.Manager) { + logger := testlog.Logger(t) shutdownCh := make(chan struct{}) m = servers.New(logger, shutdownCh, &fauxConnPool{}) return m } -func testManagerFailProb(failPct float64) (m *servers.Manager) { - logger := log.New(os.Stderr, "", log.LstdFlags) +func testManagerFailProb(t *testing.T, failPct float64) (m *servers.Manager) { + logger := testlog.Logger(t) shutdownCh := make(chan struct{}) m = servers.New(logger, shutdownCh, &fauxConnPool{failPct: failPct}) return m } func TestServers_SetServers(t *testing.T) { - m := testManager() + m := testManager(t) var num int num = m.NumServers() if num != 0 { @@ -66,14 +67,10 @@ func TestServers_SetServers(t *testing.T) { if l := len(all); l != 2 { t.Fatalf("expected 2 servers got %d", l) } - - if all[0] == s1 || all[0] == s2 { - t.Fatalf("expected a copy, got actual server") - } } func TestServers_FindServer(t *testing.T) { - m := testManager() + m := testManager(t) if m.FindServer() != nil { t.Fatalf("Expected nil return") @@ -105,20 +102,14 @@ func TestServers_FindServer(t *testing.T) { t.Fatalf("Expected two servers") } s1 = m.FindServer() - if s1 == nil || s1.String() != "s1" { - t.Fatalf("Expected s1 server (still)") + + for _, srv := range srvs { + m.NotifyFailedServer(srv) } - m.NotifyFailedServer(s1) s2 := m.FindServer() - if s2 == nil || s2.String() != "s2" { - t.Fatalf("Expected s2 server") - } - - m.NotifyFailedServer(s2) - s1 = m.FindServer() - if s1 == nil || s1.String() != "s1" { - t.Fatalf("Expected s1 server") + if s1.Equal(s2) { + t.Fatalf("Expected different server") } } @@ -132,7 +123,7 @@ func TestServers_New(t *testing.T) { } func TestServers_NotifyFailedServer(t *testing.T) { - m := testManager() + m := testManager(t) if m.NumServers() != 0 { t.Fatalf("Expected zero servers to start") @@ -159,32 +150,39 @@ func TestServers_NotifyFailedServer(t *testing.T) { t.Fatalf("Expected two servers") } - s1 = m.FindServer() - if s1 == nil || s1.String() != "s1" { - t.Fatalf("Expected s1 server") + // Grab a server + first := m.FindServer() + + // Find the other server + second := s1 + if first.Equal(s1) { + second = s2 } - m.NotifyFailedServer(s2) - s1 = m.FindServer() - if s1 == nil || s1.String() != "s1" { - t.Fatalf("Expected s1 server (still)") + // Fail the other server + m.NotifyFailedServer(second) + next := m.FindServer() + if !next.Equal(first) { + t.Fatalf("Expected first server (still)") } - m.NotifyFailedServer(s1) - s2 = m.FindServer() - if s2 == nil || s2.String() != "s2" { - t.Fatalf("Expected s2 server") + // Fail the first + m.NotifyFailedServer(first) + next = m.FindServer() + if !next.Equal(second) { + t.Fatalf("Expected second server") } - m.NotifyFailedServer(s2) - s1 = m.FindServer() - if s1 == nil || s1.String() != "s1" { - t.Fatalf("Expected s1 server") + // Fail the second + m.NotifyFailedServer(second) + next = m.FindServer() + if !next.Equal(first) { + t.Fatalf("Expected first server") } } func TestServers_NumServers(t *testing.T) { - m := testManager() + m := testManager(t) var num int num = m.NumServers() if num != 0 { @@ -201,7 +199,7 @@ func TestServers_NumServers(t *testing.T) { func TestServers_RebalanceServers(t *testing.T) { const failPct = 0.5 - m := testManagerFailProb(failPct) + m := testManagerFailProb(t, failPct) const maxServers = 100 const numShuffleTests = 100 const uniquePassRate = 0.5