From 8accabcd87f129e84563ae6bc5c16e667f61c7bc Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 23 Oct 2017 16:51:40 -0700 Subject: [PATCH] move to consul freeport implementation --- GNUmakefile | 3 +- client/client_test.go | 7 +- client/driver/docker_test.go | 7 +- command/agent/testagent.go | 9 +- helper/freeport/free.go | 34 ----- helper/freeport/free_test.go | 10 -- nomad/operator_endpoint_test.go | 6 +- nomad/server_test.go | 7 +- testutil/server.go | 20 ++- testutil/vault.go | 6 +- .../hashicorp/consul/lib/freeport/freeport.go | 131 ++++++++++++++++++ vendor/vendor.json | 6 + 12 files changed, 169 insertions(+), 77 deletions(-) delete mode 100644 helper/freeport/free.go delete mode 100644 helper/freeport/free_test.go create mode 100644 vendor/github.com/hashicorp/consul/lib/freeport/freeport.go diff --git a/GNUmakefile b/GNUmakefile index a05950375..6a2914006 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -221,7 +221,6 @@ test: ## Run the Nomad test suite and/or the Nomad UI test suite fi .PHONY: test-nomad -test-nomad: LOCAL_PACKAGES = $(shell go list ./... | grep -v '/vendor/') test-nomad: ## Run Nomad test suites @echo "==> Running Nomad test suites:" @NOMAD_TEST_RKT=1 \ @@ -229,7 +228,7 @@ test-nomad: ## Run Nomad test suites -cover \ -timeout=900s \ -tags="nomad_test $(if $(HAS_LXC),lxc)" \ - $(LOCAL_PACKAGES) + ./... .PHONY: clean clean: GOPATH=$(shell go env GOPATH) diff --git a/client/client_test.go b/client/client_test.go index 9753d1459..407009de2 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -11,12 +11,12 @@ import ( "testing" "time" + "github.com/hashicorp/consul/lib/freeport" memdb "github.com/hashicorp/go-memdb" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper" - "github.com/hashicorp/nomad/helper/freeport" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad" "github.com/hashicorp/nomad/nomad/mock" @@ -75,12 +75,13 @@ func testServer(t *testing.T, cb func(*nomad.Config)) (*nomad.Server, string) { } for i := 10; i >= 0; i-- { + ports := freeport.GetT(t, 2) config.RPCAddr = &net.TCPAddr{ IP: []byte{127, 0, 0, 1}, - Port: freeport.Get(t), + Port: ports[0], } config.NodeName = fmt.Sprintf("Node %d", config.RPCAddr.Port) - config.SerfConfig.MemberlistConfig.BindPort = freeport.Get(t) + config.SerfConfig.MemberlistConfig.BindPort = ports[1] // Create server server, err := nomad.NewServer(config, catalog, logger) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 1328c7fc5..8227072ac 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -16,13 +16,13 @@ import ( "time" docker "github.com/fsouza/go-dockerclient" + "github.com/hashicorp/consul/lib/freeport" sockaddr "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/env" cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/client/testutil" - "github.com/hashicorp/nomad/helper/freeport" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -46,8 +46,9 @@ func dockerIsRemote(t *testing.T) bool { // Returns a task with a reserved and dynamic port. The ports are returned // respectively. func dockerTask(t *testing.T) (*structs.Task, int, int) { - docker_reserved := freeport.Get(t) - docker_dynamic := freeport.Get(t) + ports := freeport.GetT(t, 2) + docker_reserved := ports[0] + docker_dynamic := ports[1] return &structs.Task{ Name: "redis-demo", Driver: "docker", diff --git a/command/agent/testagent.go b/command/agent/testagent.go index 6c79feae7..2b8fe6c5a 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -16,9 +16,9 @@ import ( "github.com/mitchellh/go-testing-interface" metrics "github.com/armon/go-metrics" + "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/client/fingerprint" - "github.com/hashicorp/nomad/helper/freeport" "github.com/hashicorp/nomad/nomad" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -259,9 +259,10 @@ func (a *TestAgent) Client() *api.Client { // Instead of relying on one set of ports to be sufficient we retry // starting the agent with different ports on port conflict. func (a *TestAgent) pickRandomPorts(c *Config) { - c.Ports.HTTP = freeport.Get(a.T) - c.Ports.RPC = freeport.Get(a.T) - c.Ports.Serf = freeport.Get(a.T) + ports := freeport.GetT(a.T, 3) + c.Ports.HTTP = ports[0] + c.Ports.RPC = ports[1] + c.Ports.Serf = ports[2] if err := c.normalizeAddrs(); err != nil { a.T.Fatalf("error normalizing config: %v", err) diff --git a/helper/freeport/free.go b/helper/freeport/free.go deleted file mode 100644 index b850a5c1f..000000000 --- a/helper/freeport/free.go +++ /dev/null @@ -1,34 +0,0 @@ -package freeport - -import ( - "fmt" - "net" - - "github.com/mitchellh/go-testing-interface" -) - -func Port() (int, error) { - ln, err := net.Listen("tcp", ":0") - if err != nil { - return -1, err - } - defer ln.Close() - - addr, ok := ln.Addr().(*net.TCPAddr) - if !ok { - return -1, fmt.Errorf("unexpected address type: %T", ln.Addr()) - } - - return addr.Port, nil -} - -func Get(t testing.T) int { - t.Helper() - - p, err := Port() - if err != nil { - t.Fatalf("failed to get free port: %v", err) - } - - return p -} diff --git a/helper/freeport/free_test.go b/helper/freeport/free_test.go deleted file mode 100644 index c044264df..000000000 --- a/helper/freeport/free_test.go +++ /dev/null @@ -1,10 +0,0 @@ -package freeport - -import "testing" - -func TestFreePort_GetFreePort(t *testing.T) { - p := Get(t) - if p <= 0 { - t.Fatalf("bad port: %d", p) - } -} diff --git a/nomad/operator_endpoint_test.go b/nomad/operator_endpoint_test.go index 204793df2..82d830f1c 100644 --- a/nomad/operator_endpoint_test.go +++ b/nomad/operator_endpoint_test.go @@ -6,9 +6,9 @@ import ( "strings" "testing" + "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/acl" - "github.com/hashicorp/nomad/helper/freeport" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -129,7 +129,7 @@ func TestOperator_RaftRemovePeerByAddress(t *testing.T) { // Try to remove a peer that's not there. arg := structs.RaftPeerByAddressRequest{ - Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", freeport.Get(t))), + Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", freeport.GetT(t, 1)[0])), } arg.Region = s1.config.Region var reply struct{} @@ -189,7 +189,7 @@ func TestOperator_RaftRemovePeerByAddress_ACL(t *testing.T) { invalidToken := mock.CreatePolicyAndToken(t, state, 1001, "test-invalid", mock.NodePolicy(acl.PolicyWrite)) arg := structs.RaftPeerByAddressRequest{ - Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", freeport.Get(t))), + Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", freeport.GetT(t, 1)[0])), } arg.Region = s1.config.Region diff --git a/nomad/server_test.go b/nomad/server_test.go index 4733018a6..04175a290 100644 --- a/nomad/server_test.go +++ b/nomad/server_test.go @@ -12,8 +12,8 @@ import ( "testing" "time" + "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/nomad/command/agent/consul" - "github.com/hashicorp/nomad/helper/freeport" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -96,11 +96,12 @@ func testServer(t *testing.T, cb func(*Config)) *Server { for i := 10; i >= 0; i-- { // Get random ports + ports := freeport.GetT(t, 2) config.RPCAddr = &net.TCPAddr{ IP: []byte{127, 0, 0, 1}, - Port: freeport.Get(t), + Port: ports[0], } - config.SerfConfig.MemberlistConfig.BindPort = freeport.Get(t) + config.SerfConfig.MemberlistConfig.BindPort = ports[1] // Create server server, err := NewServer(config, catalog, logger) diff --git a/testutil/server.go b/testutil/server.go index d6b45e9f1..5d8dfeac4 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -20,16 +20,13 @@ import ( "net/http" "os" "os/exec" - "sync/atomic" + "github.com/hashicorp/consul/lib/freeport" cleanhttp "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/nomad/helper/discover" testing "github.com/mitchellh/go-testing-interface" ) -// offset is used to atomically increment the port numbers. -var offset uint64 - // TestServerConfig is the main server configuration struct. type TestServerConfig struct { NodeName string `json:"name,omitempty"` @@ -88,11 +85,10 @@ type ServerConfigCallback func(c *TestServerConfig) // defaultServerConfig returns a new TestServerConfig struct // with all of the listen ports incremented by one. -func defaultServerConfig() *TestServerConfig { - idx := int(atomic.AddUint64(&offset, 1)) - +func defaultServerConfig(t testing.T) *TestServerConfig { + ports := freeport.GetT(t, 3) return &TestServerConfig{ - NodeName: fmt.Sprintf("node%d", idx), + NodeName: fmt.Sprintf("node-%d", ports[0]), DisableCheckpoint: true, LogLevel: "DEBUG", // Advertise can't be localhost @@ -102,9 +98,9 @@ func defaultServerConfig() *TestServerConfig { Serf: "169.254.42.42", }, Ports: &PortsConfig{ - HTTP: 20000 + idx, - RPC: 21000 + idx, - Serf: 22000 + idx, + HTTP: ports[0], + RPC: ports[1], + Serf: ports[2], }, Server: &ServerConfig{ Enabled: true, @@ -161,7 +157,7 @@ func NewTestServer(t testing.T, cb ServerConfigCallback) *TestServer { } defer configFile.Close() - nomadConfig := defaultServerConfig() + nomadConfig := defaultServerConfig(t) nomadConfig.DataDir = dataDir if cb != nil { diff --git a/testutil/vault.go b/testutil/vault.go index c4265621c..6ed78d587 100644 --- a/testutil/vault.go +++ b/testutil/vault.go @@ -7,7 +7,7 @@ import ( "os/exec" "time" - "github.com/hashicorp/nomad/helper/freeport" + "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/structs/config" vapi "github.com/hashicorp/vault/api" @@ -37,7 +37,7 @@ type TestVault struct { // NewTestVault returns a new TestVault instance that has yet to be started func NewTestVault(t testing.T) *TestVault { for i := 10; i >= 0; i-- { - port := freeport.Get(t) + port := freeport.GetT(t, 1)[0] token := uuid.Generate() bind := fmt.Sprintf("-dev-listen-address=127.0.0.1:%d", port) http := fmt.Sprintf("http://127.0.0.1:%d", port) @@ -118,7 +118,7 @@ func NewTestVault(t testing.T) *TestVault { // Start must be called and it is the callers responsibility to deal with any // port conflicts that may occur and retry accordingly. func NewTestVaultDelayed(t testing.T) *TestVault { - port := freeport.Get(t) + port := freeport.GetT(t, 1)[0] token := uuid.Generate() bind := fmt.Sprintf("-dev-listen-address=127.0.0.1:%d", port) http := fmt.Sprintf("http://127.0.0.1:%d", port) diff --git a/vendor/github.com/hashicorp/consul/lib/freeport/freeport.go b/vendor/github.com/hashicorp/consul/lib/freeport/freeport.go new file mode 100644 index 000000000..c09ff4cd6 --- /dev/null +++ b/vendor/github.com/hashicorp/consul/lib/freeport/freeport.go @@ -0,0 +1,131 @@ +// Package freeport provides a helper for allocating free ports across multiple +// processes on the same machine. +package freeport + +import ( + "fmt" + "math/rand" + "net" + "sync" + "time" + + "github.com/mitchellh/go-testing-interface" +) + +const ( + // blockSize is the size of the allocated port block. ports are given out + // consecutively from that block with roll-over for the lifetime of the + // application/test run. + blockSize = 500 + + // maxBlocks is the number of available port blocks. + // lowPort + maxBlocks * blockSize must be less than 65535. + maxBlocks = 30 + + // lowPort is the lowest port number that should be used. + lowPort = 10000 + + // attempts is how often we try to allocate a port block + // before giving up. + attempts = 10 +) + +var ( + // firstPort is the first port of the allocated block. + firstPort int + + // lockLn is the system-wide mutex for the port block. + lockLn net.Listener + + // mu guards nextPort + mu sync.Mutex + + // port is the last allocated port. + port int +) + +func init() { + if lowPort+maxBlocks*blockSize > 65535 { + panic("freeport: block size too big or too many blocks requested") + } + + rand.Seed(time.Now().UnixNano()) + firstPort, lockLn = alloc() +} + +// alloc reserves a port block for exclusive use for the lifetime of the +// application. lockLn serves as a system-wide mutex for the port block and is +// implemented as a TCP listener which is bound to the firstPort and which will +// be automatically released when the application terminates. +func alloc() (int, net.Listener) { + for i := 0; i < attempts; i++ { + block := int(rand.Int31n(int32(maxBlocks))) + firstPort := lowPort + block*blockSize + ln, err := net.ListenTCP("tcp", tcpAddr("127.0.0.1", firstPort)) + if err != nil { + continue + } + // log.Printf("[DEBUG] freeport: allocated port block %d (%d-%d)", block, firstPort, firstPort+blockSize-1) + return firstPort, ln + } + panic("freeport: cannot allocate port block") +} + +func tcpAddr(ip string, port int) *net.TCPAddr { + return &net.TCPAddr{IP: net.ParseIP(ip), Port: port} +} + +// Get wraps the Free function and panics on any failure retrieving ports. +func Get(n int) (ports []int) { + ports, err := Free(n) + if err != nil { + panic(err) + } + + return ports +} + +// GetT is suitable for use when retrieving unused ports in tests. If there is +// an error retrieving free ports, the test will be failed. +func GetT(t testing.T, n int) (ports []int) { + ports, err := Free(n) + if err != nil { + t.Fatalf("Failed retrieving free port: %v", err) + } + + return ports +} + +// Free returns a list of free ports from the allocated port block. It is safe +// to call this method concurrently. Ports have been tested to be available on +// 127.0.0.1 TCP but there is no guarantee that they will remain free in the +// future. +func Free(n int) (ports []int, err error) { + mu.Lock() + defer mu.Unlock() + + if n > blockSize-1 { + return nil, fmt.Errorf("freeport: block size too small") + } + + for len(ports) < n { + port++ + + // roll-over the port + if port < firstPort+1 || port >= firstPort+blockSize { + port = firstPort + 1 + } + + // if the port is in use then skip it + ln, err := net.ListenTCP("tcp", tcpAddr("127.0.0.1", port)) + if err != nil { + // log.Println("[DEBUG] freeport: port already in use: ", port) + continue + } + ln.Close() + + ports = append(ports, port) + } + // log.Println("[DEBUG] freeport: free ports:", ports) + return ports, nil +} diff --git a/vendor/vendor.json b/vendor/vendor.json index 08c91cae1..3e66bcaa5 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -760,6 +760,12 @@ "revision": "51ea240df8476e02215d53fbfad5838bf0d44d21", "revisionTime": "2017-10-16T16:22:40Z" }, + { + "checksumSHA1": "XUc/5Wg49jT0dGHRv7FhzDosj2Q=", + "path": "github.com/hashicorp/consul/lib/freeport", + "revision": "be18f97531edb0b75a91e61c7e26a66224a46468", + "revisionTime": "2017-10-23T23:34:27Z" + }, { "checksumSHA1": "5XjgqE4UIfwXvkq5VssGNc7uPhQ=", "path": "github.com/hashicorp/consul/test/porter",