diff --git a/agent/agent_test.go b/agent/agent_test.go index 60c80b8833..7fbe9559f3 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1737,11 +1737,7 @@ func TestAgent_RestoreServiceWithAliasCheck(t *testing.T) { testCtx, testCancel := context.WithCancel(context.Background()) defer testCancel() - testHTTPServer, returnPort := launchHTTPCheckServer(t, testCtx) - defer func() { - testHTTPServer.Close() - returnPort() - }() + testHTTPServer := launchHTTPCheckServer(t, testCtx) registerServicesAndChecks := func(t *testing.T, a *TestAgent) { // add one persistent service with a simple check @@ -1846,11 +1842,8 @@ node_name = "` + a.Config.NodeName + `" } } -func launchHTTPCheckServer(t *testing.T, ctx context.Context) (srv *httptest.Server, returnPortsFn func()) { - ports := freeport.MustTake(1) - port := ports[0] - - addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(port)) +func launchHTTPCheckServer(t *testing.T, ctx context.Context) *httptest.Server { + addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(freeport.Port(t))) var lc net.ListenConfig listener, err := lc.Listen(ctx, "tcp", addr) @@ -1861,12 +1854,13 @@ func launchHTTPCheckServer(t *testing.T, ctx context.Context) (srv *httptest.Ser _, _ = w.Write([]byte("OK\n")) }) - srv = &httptest.Server{ + srv := &httptest.Server{ Listener: listener, Config: &http.Server{Handler: handler}, } srv.Start() - return srv, func() { freeport.Return(ports) } + t.Cleanup(srv.Close) + return srv } func TestAgent_AddCheck_Alias(t *testing.T) { @@ -4704,14 +4698,12 @@ func TestAgent_JoinWAN_viaMeshGateway(t *testing.T) { t.Parallel() - gwPort := freeport.MustTake(1) - defer freeport.Return(gwPort) - gwAddr := ipaddr.FormatAddressPort("127.0.0.1", gwPort[0]) + port := freeport.Port(t) + gwAddr := ipaddr.FormatAddressPort("127.0.0.1", port) // Due to some ordering, we'll have to manually configure these ports in // advance. - secondaryRPCPorts := freeport.MustTake(2) - defer freeport.Return(secondaryRPCPorts) + secondaryRPCPorts := freeport.GetN(t, 2) a1 := StartTestAgent(t, TestAgent{Name: "bob", HCL: ` domain = "consul" @@ -4765,7 +4757,7 @@ func TestAgent_JoinWAN_viaMeshGateway(t *testing.T) { ID: "mesh-gateway", Name: "mesh-gateway", Meta: map[string]string{structs.MetaWANFederationKey: "1"}, - Port: gwPort[0], + Port: port, } req, err := http.NewRequest("PUT", "/v1/agent/service/register", jsonReader(args)) require.NoError(t, err) @@ -4879,7 +4871,7 @@ func TestAgent_JoinWAN_viaMeshGateway(t *testing.T) { ID: "mesh-gateway", Name: "mesh-gateway", Meta: map[string]string{structs.MetaWANFederationKey: "1"}, - Port: gwPort[0], + Port: port, } req, err := http.NewRequest("PUT", "/v1/agent/service/register", jsonReader(args)) require.NoError(t, err) @@ -4894,7 +4886,7 @@ func TestAgent_JoinWAN_viaMeshGateway(t *testing.T) { ID: "mesh-gateway", Name: "mesh-gateway", Meta: map[string]string{structs.MetaWANFederationKey: "1"}, - Port: gwPort[0], + Port: port, } req, err := http.NewRequest("PUT", "/v1/agent/service/register", jsonReader(args)) require.NoError(t, err) diff --git a/agent/connect/ca/testing.go b/agent/connect/ca/testing.go index 3b470063f0..6ba9df791a 100644 --- a/agent/connect/ca/testing.go +++ b/agent/connect/ca/testing.go @@ -120,11 +120,7 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { return nil, fmt.Errorf("%q not found on $PATH", vaultBinaryName) } - ports := freeport.MustTake(2) - returnPortsFn := func() { - freeport.Return(ports) - } - + ports := freeport.GetN(t, 2) var ( clientAddr = fmt.Sprintf("127.0.0.1:%d", ports[0]) clusterAddr = fmt.Sprintf("127.0.0.1:%d", ports[1]) @@ -136,7 +132,6 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { Address: "http://" + clientAddr, }) if err != nil { - returnPortsFn() return nil, err } client.SetToken(token) @@ -156,16 +151,14 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { cmd.Stdout = ioutil.Discard cmd.Stderr = ioutil.Discard if err := cmd.Start(); err != nil { - returnPortsFn() return nil, err } testVault := &TestVaultServer{ - RootToken: token, - Addr: "http://" + clientAddr, - cmd: cmd, - client: client, - returnPortsFn: returnPortsFn, + RootToken: token, + Addr: "http://" + clientAddr, + cmd: cmd, + client: client, } t.Cleanup(func() { if err := testVault.Stop(); err != nil { @@ -181,9 +174,6 @@ type TestVaultServer struct { Addr string cmd *exec.Cmd client *vaultapi.Client - - // returnPortsFn will put the ports claimed for the test back into the - returnPortsFn func() } var printedVaultVersion sync.Once @@ -229,11 +219,6 @@ func (v *TestVaultServer) Stop() error { if err := v.cmd.Wait(); err != nil { return err } - - if v.returnPortsFn != nil { - v.returnPortsFn() - } - return nil } diff --git a/agent/consul/authmethod/kubeauth/testing.go b/agent/consul/authmethod/kubeauth/testing.go index 7a61804fd7..84e9c6881f 100644 --- a/agent/consul/authmethod/kubeauth/testing.go +++ b/agent/consul/authmethod/kubeauth/testing.go @@ -33,9 +33,8 @@ import ( // - GET /api/v1/namespaces//serviceaccounts/ // type TestAPIServer struct { - srv *httptest.Server - caCert string - returnFunc func() + srv *httptest.Server + caCert string mu sync.Mutex authorizedJWT string // token review and sa read @@ -48,12 +47,7 @@ type TestAPIServer struct { // random free port. func StartTestAPIServer(t testing.T) *TestAPIServer { s := &TestAPIServer{} - - ports := freeport.MustTake(1) - s.returnFunc = func() { - freeport.Return(ports) - } - s.srv = httptestNewUnstartedServerWithPort(s, ports[0]) + s.srv = httptestNewUnstartedServerWithPort(s, freeport.Port(t)) s.srv.Config.ErrorLog = log.New(ioutil.Discard, "", 0) s.srv.StartTLS() @@ -101,10 +95,6 @@ func (s *TestAPIServer) SetAllowedServiceAccount( // Stop stops the running TestAPIServer. func (s *TestAPIServer) Stop() { s.srv.Close() - if s.returnFunc != nil { - s.returnFunc() - s.returnFunc = nil - } } // Addr returns the current base URL for the running webserver. diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go index d6ae206bca..79c3f1a7b5 100644 --- a/agent/consul/client_test.go +++ b/agent/consul/client_test.go @@ -33,11 +33,7 @@ func testClientConfig(t *testing.T) (string, *Config) { dir := testutil.TempDir(t, "consul") config := DefaultConfig() - ports := freeport.MustTake(2) - t.Cleanup(func() { - freeport.Return(ports) - }) - + ports := freeport.GetN(t, 2) config.Datacenter = "dc1" config.DataDir = dir config.NodeName = uniqueNodeName(t.Name()) diff --git a/agent/consul/operator_raft_endpoint_test.go b/agent/consul/operator_raft_endpoint_test.go index da5bcc1214..63db469cf5 100644 --- a/agent/consul/operator_raft_endpoint_test.go +++ b/agent/consul/operator_raft_endpoint_test.go @@ -140,13 +140,10 @@ func TestOperator_RaftRemovePeerByAddress(t *testing.T) { testrpc.WaitForLeader(t, s1.RPC, "dc1") - ports := freeport.MustTake(1) - defer freeport.Return(ports) - // Try to remove a peer that's not there. arg := structs.RaftRemovePeerRequest{ Datacenter: "dc1", - Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", ports[0])), + Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", freeport.Port(t))), } var reply struct{} err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByAddress", &arg, &reply) @@ -263,10 +260,7 @@ func TestOperator_RaftRemovePeerByID(t *testing.T) { // Add it manually to Raft. { - ports := freeport.MustTake(1) - defer freeport.Return(ports) - - future := s1.raft.AddVoter(arg.ID, raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", ports[0])), 0, 0) + future := s1.raft.AddVoter(arg.ID, raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", freeport.Port(t))), 0, 0) if err := future.Error(); err != nil { t.Fatalf("err: %v", err) } diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 978d2453cd..3cca5b75fb 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -116,11 +116,7 @@ func testServerConfig(t *testing.T) (string, *Config) { dir := testutil.TempDir(t, "consul") config := DefaultConfig() - ports := freeport.MustTake(3) - t.Cleanup(func() { - freeport.Return(ports) - }) - + ports := freeport.GetN(t, 3) config.NodeName = uniqueNodeName(t.Name()) config.Bootstrap = true config.Datacenter = "dc1" @@ -516,10 +512,7 @@ func TestServer_JoinWAN_SerfAllowedCIDRs(t *testing.T) { } func skipIfCannotBindToIP(t *testing.T, ip string) { - ports := freeport.MustTake(1) - defer freeport.Return(ports) - - addr := ipaddr.FormatAddressPort(ip, ports[0]) + addr := ipaddr.FormatAddressPort(ip, freeport.Port(t)) l, err := net.Listen("tcp", addr) l.Close() if err != nil { @@ -725,9 +718,8 @@ func TestServer_JoinWAN_viaMeshGateway(t *testing.T) { t.Parallel() - gwPort := freeport.MustTake(1) - defer freeport.Return(gwPort) - gwAddr := ipaddr.FormatAddressPort("127.0.0.1", gwPort[0]) + port := freeport.Port(t) + gwAddr := ipaddr.FormatAddressPort("127.0.0.1", port) dir1, s1 := testServerWithConfig(t, func(c *Config) { c.TLSConfig.Domain = "consul" @@ -813,7 +805,7 @@ func TestServer_JoinWAN_viaMeshGateway(t *testing.T) { ID: "mesh-gateway", Service: "mesh-gateway", Meta: map[string]string{structs.MetaWANFederationKey: "1"}, - Port: gwPort[0], + Port: port, }, } @@ -868,7 +860,7 @@ func TestServer_JoinWAN_viaMeshGateway(t *testing.T) { ID: "mesh-gateway", Service: "mesh-gateway", Meta: map[string]string{structs.MetaWANFederationKey: "1"}, - Port: gwPort[0], + Port: port, }, } @@ -885,7 +877,7 @@ func TestServer_JoinWAN_viaMeshGateway(t *testing.T) { ID: "mesh-gateway", Service: "mesh-gateway", Meta: map[string]string{structs.MetaWANFederationKey: "1"}, - Port: gwPort[0], + Port: port, }, } diff --git a/agent/grpc/client_test.go b/agent/grpc/client_test.go index 1c11ee4ebe..1b8cfa1ab6 100644 --- a/agent/grpc/client_test.go +++ b/agent/grpc/client_test.go @@ -29,10 +29,7 @@ func useTLSForDcAlwaysTrue(_ string) bool { } func TestNewDialer_WithTLSWrapper(t *testing.T) { - ports := freeport.MustTake(1) - defer freeport.Return(ports) - - lis, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(ports[0]))) + lis, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(freeport.Port(t)))) require.NoError(t, err) t.Cleanup(logError(t, lis.Close)) @@ -68,8 +65,7 @@ func TestNewDialer_WithTLSWrapper(t *testing.T) { } func TestNewDialer_WithALPNWrapper(t *testing.T) { - ports := freeport.MustTake(3) - defer freeport.Return(ports) + ports := freeport.GetN(t, 3) var ( s1addr = ipaddr.FormatAddressPort("127.0.0.1", ports[0]) @@ -193,10 +189,7 @@ func TestNewDialer_IntegrationWithTLSEnabledHandler(t *testing.T) { func TestNewDialer_IntegrationWithTLSEnabledHandler_viaMeshGateway(t *testing.T) { // if this test is failing because of expired certificates // use the procedure in test/CA-GENERATION.md - ports := freeport.MustTake(1) - defer freeport.Return(ports) - - gwAddr := ipaddr.FormatAddressPort("127.0.0.1", ports[0]) + gwAddr := ipaddr.FormatAddressPort("127.0.0.1", freeport.Port(t)) res := resolver.NewServerResolverBuilder(newConfig(t)) registerWithGRPC(t, res) diff --git a/agent/grpc/server_test.go b/agent/grpc/server_test.go index 9945f1e6cb..07af7299fc 100644 --- a/agent/grpc/server_test.go +++ b/agent/grpc/server_test.go @@ -47,12 +47,7 @@ func newTestServer(t *testing.T, name string, dc string, tlsConf *tlsutil.Config testservice.RegisterSimpleServer(server, &simple{name: name, dc: dc}) }) - ports := freeport.MustTake(1) - t.Cleanup(func() { - freeport.Return(ports) - }) - - lis, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(ports[0]))) + lis, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(freeport.Port(t)))) require.NoError(t, err) rpc := &fakeRPCListener{t: t, handler: handler, tlsConf: tlsConf} diff --git a/agent/testagent.go b/agent/testagent.go index b421f3cec6..80a4e0cf06 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -165,8 +165,7 @@ func (a *TestAgent) Start(t *testing.T) error { Name: name, }) - portsConfig, returnPortsFn := randomPortsSource(a.UseTLS) - t.Cleanup(returnPortsFn) + portsConfig := randomPortsSource(t, a.UseTLS) // Create NodeID outside the closure, so that it does not change testHCLConfig := TestConfigHCL(NodeID()) @@ -378,8 +377,8 @@ func (a *TestAgent) consulConfig() *consul.Config { // chance of port conflicts for concurrently executed test binaries. // Instead of relying on one set of ports to be sufficient we retry // starting the agent with different ports on port conflict. -func randomPortsSource(tls bool) (data string, returnPortsFn func()) { - ports := freeport.MustTake(7) +func randomPortsSource(t *testing.T, tls bool) string { + ports := freeport.GetN(t, 7) var http, https int if tls { @@ -400,7 +399,7 @@ func randomPortsSource(tls bool) (data string, returnPortsFn func()) { server = ` + strconv.Itoa(ports[5]) + ` grpc = ` + strconv.Itoa(ports[6]) + ` } - `, func() { freeport.Return(ports) } + ` } func NodeID() string { diff --git a/connect/proxy/listener_test.go b/connect/proxy/listener_test.go index 83cb399877..8e5a07f329 100644 --- a/connect/proxy/listener_test.go +++ b/connect/proxy/listener_test.go @@ -112,15 +112,13 @@ func TestPublicListener(t *testing.T) { // Can't enable t.Parallel since we rely on the global metrics instance. ca := agConnect.TestCA(t, nil) - ports := freeport.MustTake(1) - defer freeport.Return(ports) - testApp := NewTestTCPServer(t) defer testApp.Close() + port := freeport.Port(t) cfg := PublicListenerConfig{ BindAddress: "127.0.0.1", - BindPort: ports[0], + BindPort: port, LocalServiceAddress: testApp.Addr().String(), HandshakeTimeoutMs: 100, LocalConnectTimeoutMs: 100, @@ -144,7 +142,7 @@ func TestPublicListener(t *testing.T) { // Proxy and backend are running, play the part of a TLS client using same // cert for now. conn, err := svc.Dial(context.Background(), &connect.StaticResolver{ - Addr: TestLocalAddr(ports[0]), + Addr: TestLocalAddr(port), CertURI: agConnect.TestSpiffeIDService(t, "db"), }) require.NoError(t, err) @@ -166,9 +164,6 @@ func TestUpstreamListener(t *testing.T) { // Can't enable t.Parallel since we rely on the global metrics instance. ca := agConnect.TestCA(t, nil) - ports := freeport.MustTake(1) - defer freeport.Return(ports) - // Run a test server that we can dial. testSvr := connect.NewTestServer(t, "db", ca) go func() { @@ -184,7 +179,7 @@ func TestUpstreamListener(t *testing.T) { DestinationName: "db", Config: map[string]interface{}{"connect_timeout_ms": 100}, LocalBindAddress: "localhost", - LocalBindPort: ports[0], + LocalBindPort: freeport.Port(t), } // Setup metrics to test they are recorded diff --git a/connect/proxy/proxy_test.go b/connect/proxy/proxy_test.go index 46274de9b2..904e9c7c44 100644 --- a/connect/proxy/proxy_test.go +++ b/connect/proxy/proxy_test.go @@ -27,8 +27,7 @@ func TestProxy_public(t *testing.T) { t.Skip("too slow for testing.Short") } - ports := freeport.MustTake(2) - defer freeport.Return(ports) + ports := freeport.GetN(t, 2) a := agent.NewTestAgent(t, "") defer a.Shutdown() diff --git a/connect/proxy/testing.go b/connect/proxy/testing.go index cad243478b..ebaf0d1cc5 100644 --- a/connect/proxy/testing.go +++ b/connect/proxy/testing.go @@ -24,24 +24,19 @@ type TestTCPServer struct { l net.Listener stopped int32 accepted, closed, active int32 - returnPortsFn func() } // NewTestTCPServer opens as a listening socket on the given address and returns // a TestTCPServer serving requests to it. The server is already started and can // be stopped by calling Close(). func NewTestTCPServer(t testing.T) *TestTCPServer { - ports := freeport.MustTake(1) - addr := TestLocalAddr(ports[0]) + addr := TestLocalAddr(freeport.Port(t)) l, err := net.Listen("tcp", addr) require.NoError(t, err) log.Printf("test tcp server listening on %s", addr) - s := &TestTCPServer{ - l: l, - returnPortsFn: func() { freeport.Return(ports) }, - } + s := &TestTCPServer{l: l} go s.accept() return s @@ -53,10 +48,6 @@ func (s *TestTCPServer) Close() { if s.l != nil { s.l.Close() } - if s.returnPortsFn != nil { - s.returnPortsFn() - s.returnPortsFn = nil - } } // Addr returns the address that this server is listening on. diff --git a/connect/testing.go b/connect/testing.go index 30a517b61f..180b15d6c5 100644 --- a/connect/testing.go +++ b/connect/testing.go @@ -96,24 +96,21 @@ type TestServer struct { // Listening is closed when the listener is run. Listening chan struct{} - l net.Listener - returnPortsFn func() - stopFlag int32 - stopChan chan struct{} + l net.Listener + stopFlag int32 + stopChan chan struct{} } // NewTestServer returns a TestServer. It should be closed when test is // complete. func NewTestServer(t testing.T, service string, ca *structs.CARoot) *TestServer { - ports := freeport.MustTake(1) return &TestServer{ - Service: service, - CA: ca, - stopChan: make(chan struct{}), - TLSCfg: TestTLSConfig(t, service, ca), - Addr: fmt.Sprintf("127.0.0.1:%d", ports[0]), - Listening: make(chan struct{}), - returnPortsFn: func() { freeport.Return(ports) }, + Service: service, + CA: ca, + stopChan: make(chan struct{}), + TLSCfg: TestTLSConfig(t, service, ca), + Addr: fmt.Sprintf("127.0.0.1:%d", freeport.Port(t)), + Listening: make(chan struct{}), } } @@ -190,10 +187,6 @@ func (s *TestServer) Close() error { if s.l != nil { s.l.Close() } - if s.returnPortsFn != nil { - s.returnPortsFn() - s.returnPortsFn = nil - } close(s.stopChan) } return nil diff --git a/lib/testing_httpserver.go b/lib/testing_httpserver.go index df5e1f4141..86832ed209 100644 --- a/lib/testing_httpserver.go +++ b/lib/testing_httpserver.go @@ -18,12 +18,7 @@ import ( // _know_ nothing is listening. If you simply assumed unbound ports were free // you'd end up with test cross-talk and weirdness. func StartTestServer(t testing.T, handler http.Handler) string { - ports := freeport.MustTake(1) - t.Cleanup(func() { - freeport.Return(ports) - }) - - addr := ipaddr.FormatAddressPort("127.0.0.1", ports[0]) + addr := ipaddr.FormatAddressPort("127.0.0.1", freeport.Port(t)) server := &http.Server{Addr: addr, Handler: handler} t.Cleanup(func() {