From 14d6fec05a3173717347a1b140f7dc169d688b77 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 7 Apr 2020 09:27:48 -0400 Subject: [PATCH] tests: deflake some SetServer related tests Some tests assert on numbers on numbers of servers, e.g. TestHTTP_AgentSetServers and TestHTTP_AgentListServers_ACL . Though, in dev and test modes, the agent starts with servers having duplicate entries for advertised and normalized RPC values, then settles with one unique value after Raft/Serf re-sets servers with one single unique value. This leads to flakiness, as the test will fail if assertion runs before Serf update takes effect. Here, we update the inital dev handling so it only adds a unique value if the advertised and normalized values are the same. Sample log lines illustrating the problem: ``` === CONT TestHTTP_AgentSetServers TestHTTP_AgentSetServers: testlog.go:34: 2020-04-06T21:47:51.016Z [INFO] nomad.raft: initial configuration: index=1 servers="[{Suffrage:Voter ID:127.0.0.1:9008 Address:127.0.0.1:9008}]" TestHTTP_AgentSetServers: testlog.go:34: 2020-04-06T21:47:51.016Z [INFO] nomad: serf: EventMemberJoin: TestHTTP_AgentSetServers.global 127.0.0.1 TestHTTP_AgentSetServers: testlog.go:34: 2020-04-06T21:47:51.035Z [DEBUG] client.server_mgr: new server list: new_servers=[127.0.0.1:9008, 127.0.0.1:9008] old_servers=[] ... TestHTTP_AgentSetServers: agent_endpoint_test.go:759: Error Trace: agent_endpoint_test.go:759 http_test.go:1089 agent_endpoint_test.go:705 Error: "[127.0.0.1:9008 127.0.0.1:9008]" should have 1 item(s), but has 2 Test: TestHTTP_AgentSetServers ``` --- command/agent/agent.go | 15 ++++++++++----- command/agent/agent_endpoint_test.go | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 572d1914c..e1b4364b5 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -421,15 +421,20 @@ func (a *Agent) finalizeClientConfig(c *clientconfig.Config) error { // configured explicitly. This handles both running server and client on one // host and -dev mode. if a.server != nil { - if a.config.AdvertiseAddrs == nil || a.config.AdvertiseAddrs.RPC == "" { + advertised := a.config.AdvertiseAddrs + normalized := a.config.normalizedAddrs + + if advertised == nil || advertised.RPC == "" { return fmt.Errorf("AdvertiseAddrs is nil or empty") - } else if a.config.normalizedAddrs == nil || a.config.normalizedAddrs.RPC == "" { + } else if normalized == nil || normalized.RPC == "" { return fmt.Errorf("normalizedAddrs is nil or empty") } - c.Servers = append(c.Servers, - a.config.normalizedAddrs.RPC, - a.config.AdvertiseAddrs.RPC) + if normalized.RPC == advertised.RPC { + c.Servers = append(c.Servers, normalized.RPC) + } else { + c.Servers = append(c.Servers, normalized.RPC, advertised.RPC) + } } // Setup the plugin loaders diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 8e78705aa..795dffd38 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -711,7 +711,7 @@ func TestHTTP_AgentSetServers(t *testing.T) { } defer conn.Close() - // Write the Consul RPC byte to set the mode + // Write the Nomad RPC byte to set the mode if _, err := conn.Write([]byte{byte(pool.RpcNomad)}); err != nil { return false, err }