diff --git a/agent/consul/autopilot_test.go b/agent/consul/autopilot_test.go index 922d72a28d..4a2e737319 100644 --- a/agent/consul/autopilot_test.go +++ b/agent/consul/autopilot_test.go @@ -401,7 +401,6 @@ func TestAutopilot_PromoteNonVoter(t *testing.T) { func TestAutopilot_MinQuorum(t *testing.T) { dc := "dc1" - closeMap := make(map[string]chan struct{}) conf := func(c *Config) { c.Datacenter = dc c.Bootstrap = false @@ -409,13 +408,6 @@ func TestAutopilot_MinQuorum(t *testing.T) { c.AutopilotConfig.MinQuorum = 3 c.RaftConfig.ProtocolVersion = raft.ProtocolVersion(2) c.AutopilotInterval = 100 * time.Millisecond - //Let us know when a server is actually gone - ch := make(chan struct{}) - c.NotifyShutdown = func() { - t.Logf("%v is shutdown", c.NodeName) - close(ch) - } - closeMap[c.NodeName] = ch } dir1, s1 := testServerWithConfig(t, conf) defer os.RemoveAll(dir1) @@ -463,8 +455,7 @@ func TestAutopilot_MinQuorum(t *testing.T) { if dead == nil { t.Fatalf("no members set") } - dead.Shutdown() - <-closeMap[dead.config.NodeName] + require.NoError(t, dead.Shutdown()) retry.Run(t, func(r *retry.R) { leader := findStatus(true) if leader == nil { @@ -480,10 +471,7 @@ func TestAutopilot_MinQuorum(t *testing.T) { delete(servers, dead.config.NodeName) //Autopilot should not take this one into left dead = findStatus(false) - if err := dead.Shutdown(); err != nil { - t.Fatalf("could not shut down %s, error %v", dead.config.NodeName, err) - } - <-closeMap[dead.config.NodeName] + require.NoError(t, dead.Shutdown()) retry.Run(t, func(r *retry.R) { leader := findStatus(true) @@ -496,5 +484,4 @@ func TestAutopilot_MinQuorum(t *testing.T) { } } }) - } diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go index 4ec005ecd3..8addc2e087 100644 --- a/agent/consul/client_test.go +++ b/agent/consul/client_test.go @@ -23,18 +23,15 @@ import ( func testClientConfig(t *testing.T) (string, *Config) { dir := testutil.TempDir(t, "consul") + t.Cleanup(func() { + os.RemoveAll(dir) + }) config := DefaultConfig() ports := freeport.MustTake(2) - - returnPortsFn := func() { - // The method of plumbing this into the client shutdown hook doesn't - // cover all exit points, so we insulate this against multiple - // invocations and then it's safe to call it a bunch of times. + t.Cleanup(func() { freeport.Return(ports) - config.NotifyShutdown = nil // self-erasing - } - config.NotifyShutdown = returnPortsFn + }) config.Datacenter = "dc1" config.DataDir = dir @@ -82,9 +79,6 @@ func testClientWithConfigWithErr(t *testing.T, cb func(c *Config)) (string, *Cli } client, err := NewClient(config, WithLogger(logger), WithTLSConfigurator(tlsConf)) - if err != nil { - config.NotifyShutdown() - } return dir, client, err } @@ -446,7 +440,6 @@ func TestClient_RPC_TLS(t *testing.T) { defer s1.Shutdown() dir2, conf2 := testClientConfig(t) - defer conf2.NotifyShutdown() conf2.VerifyOutgoing = true configureTLS(conf2) c1, err := newClient(t, conf2) @@ -503,7 +496,6 @@ func TestClient_RPC_RateLimit(t *testing.T) { testrpc.WaitForLeader(t, s1.RPC, "dc1") dir2, conf2 := testClientConfig(t) - defer conf2.NotifyShutdown() conf2.RPCRate = 2 conf2.RPCMaxBurst = 2 c1, err := newClient(t, conf2) @@ -571,7 +563,6 @@ func TestClient_SnapshotRPC_RateLimit(t *testing.T) { testrpc.WaitForLeader(t, s1.RPC, "dc1") dir2, conf1 := testClientConfig(t) - defer conf1.NotifyShutdown() conf1.RPCRate = 2 conf1.RPCMaxBurst = 2 c1, err := newClient(t, conf1) @@ -614,7 +605,6 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) { defer s1.Shutdown() dir2, conf2 := testClientConfig(t) - defer conf2.NotifyShutdown() conf2.VerifyOutgoing = true configureTLS(conf2) c1, err := newClient(t, conf2) diff --git a/agent/consul/config.go b/agent/consul/config.go index ebfca98249..a48effe441 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -119,9 +119,6 @@ type Config struct { // configured at this point. NotifyListen func() - // NotifyShutdown is called after Server is completely Shutdown. - NotifyShutdown func() - // RPCAddr is the RPC address used by Consul. This should be reachable // by the WAN and LAN RPCAddr *net.TCPAddr diff --git a/agent/consul/server.go b/agent/consul/server.go index 5ba45d71ce..56d2a557ac 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -933,10 +933,6 @@ func (s *Server) Shutdown() error { s.acls.Close() } - if s.config.NotifyShutdown != nil { - s.config.NotifyShutdown() - } - if s.fsm != nil { s.fsm.State().Abandon() } diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 18d61618f0..037730b70b 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -129,18 +129,15 @@ func waitForLeaderEstablishment(t *testing.T, servers ...*Server) { func testServerConfig(t *testing.T) (string, *Config) { dir := testutil.TempDir(t, "consul") + t.Cleanup(func() { + os.RemoveAll(dir) + }) config := DefaultConfig() ports := freeport.MustTake(3) - - returnPortsFn := func() { - // The method of plumbing this into the server shutdown hook doesn't - // cover all exit points, so we insulate this against multiple - // invocations and then it's safe to call it a bunch of times. + t.Cleanup(func() { freeport.Return(ports) - config.NotifyShutdown = nil // self-erasing - } - config.NotifyShutdown = returnPortsFn + }) config.NodeName = uniqueNodeName(t.Name()) config.Bootstrap = true @@ -154,7 +151,6 @@ func testServerConfig(t *testing.T) (string, *Config) { nodeID, err := uuid.GenerateUUID() if err != nil { - returnPortsFn() t.Fatal(err) } config.NodeID = types.NodeID(nodeID) @@ -211,9 +207,6 @@ func testServerConfig(t *testing.T) (string, *Config) { "IntermediateCertTTL": "288h", }, } - - config.NotifyShutdown = returnPortsFn - return dir, config } @@ -270,8 +263,6 @@ func testServerWithConfig(t *testing.T, cb func(*Config)) (string, *Server) { var err error srv, err = newServer(t, config) if err != nil { - config.NotifyShutdown() - os.RemoveAll(dir) r.Fatalf("err: %v", err) } })