diff --git a/GNUmakefile b/GNUmakefile index 72351df5a7..d5d7ed3cf9 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -62,11 +62,11 @@ cov: gocov test $(GOFILES) | gocov-html > /tmp/coverage.html open /tmp/coverage.html -test: other-consul porter dev-build vet +test: other-consul dev-build vet @echo "--> Running go test" @rm -f test.log exit-code go test -tags '$(GOTAGS)' -i ./... - porter go test $(GOTEST_FLAGS) -tags '$(GOTAGS)' -timeout 5m -v ./... &>test.log ; echo $$? > exit-code + go test $(GOTEST_FLAGS) -tags '$(GOTAGS)' -timeout 5m -v ./... &>test.log ; echo $$? > exit-code @echo "Exit code: $$(cat exit-code)" >> test.log @grep -A5 'DATA RACE' test.log || true @grep -A10 'panic: test timed out' test.log || true @@ -86,10 +86,6 @@ other-consul: exit 1 ; \ fi -porter: - @echo "--> Building port number service..." - go install github.com/hashicorp/consul/test/porter/cmd/porter - cover: go test $(GOFILES) --cover diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index be347fe56c..9090b6e847 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -13,7 +13,7 @@ import ( "github.com/hashicorp/consul/agent/metadata" "github.com/hashicorp/consul/agent/token" - "github.com/hashicorp/consul/test/porter" + "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/testutil/retry" @@ -41,10 +41,7 @@ func testServerConfig(t *testing.T) (string, *Config) { dir := testutil.TempDir(t, "consul") config := DefaultConfig() - ports, err := porter.RandomPorts(3) - if err != nil { - t.Fatal("RandomPorts:", err) - } + ports := freeport.Get(3) config.NodeName = uniqueNodeName(t.Name()) config.Bootstrap = true config.Datacenter = "dc1" diff --git a/agent/testagent.go b/agent/testagent.go index 4998587fc8..9b99cd6471 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -20,8 +20,8 @@ import ( "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/consul/logger" - "github.com/hashicorp/consul/test/porter" "github.com/hashicorp/consul/testutil/retry" uuid "github.com/hashicorp/go-uuid" ) @@ -297,10 +297,7 @@ func UniqueID() string { // Instead of relying on one set of ports to be sufficient we retry // starting the agent with different ports on port conflict. func randomPortsSource() config.Source { - ports, err := porter.RandomPorts(5) - if err != nil { - panic(err) - } + ports := freeport.Get(5) return config.Source{ Name: "ports", Format: "hcl", diff --git a/lib/freeport/freeport.go b/lib/freeport/freeport.go new file mode 100644 index 0000000000..08fdf62348 --- /dev/null +++ b/lib/freeport/freeport.go @@ -0,0 +1,107 @@ +// Package freeport provides a helper for allocating free ports across multiple +// processes on the same machine. +package freeport + +import ( + "math/rand" + "net" + "sync" + "time" +) + +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 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 Get(n int) (ports []int) { + mu.Lock() + defer mu.Unlock() + + if n > blockSize-1 { + panic("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 +} diff --git a/test/porter/client.go b/test/porter/client.go deleted file mode 100644 index 1c746e9be0..0000000000 --- a/test/porter/client.go +++ /dev/null @@ -1,61 +0,0 @@ -package porter - -import ( - "encoding/json" - "fmt" - "io/ioutil" - "net/http" - "os" - "strings" -) - -var ( - // DefaultAddr is the the default bind address of a Porter server. This acts - // as the fallback address if the Porter server is not specified. - DefaultAddr = "127.0.0.1:7965" -) - -const ( - // porterErrPrefix is the string returned when displaying a porter error - porterErrPrefix = `Are you running porter? -Install with 'go install github.com/hashicorp/consul/test/porter/cmd/porter' -Then run 'porter go test ...'` -) - -// PorterExistErr is used to wrap an error that is likely from Porter not being -// run. -type PorterExistErr struct { - Wrapped error -} - -func (p *PorterExistErr) Error() string { - return fmt.Sprintf("%s:\n%s", porterErrPrefix, p.Wrapped) -} - -func RandomPorts(n int) ([]int, error) { - addr := os.Getenv("PORTER_ADDR") - if addr == "" { - b, err := ioutil.ReadFile("/tmp/porter.addr") - if err == nil { - addr = string(b) - } - } - if addr == "" { - addr = DefaultAddr - } - resp, err := http.Get(fmt.Sprintf("http://%s/%d", addr, n)) - if err != nil { - if strings.Contains(err.Error(), "connection refused") { - return nil, &PorterExistErr{Wrapped: err} - } - return nil, err - } - defer resp.Body.Close() - data, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, err - } - var p []int - err = json.Unmarshal(data, &p) - return p, err -} diff --git a/test/porter/cmd/porter/main.go b/test/porter/cmd/porter/main.go deleted file mode 100644 index ea1bdb29d6..0000000000 --- a/test/porter/cmd/porter/main.go +++ /dev/null @@ -1,154 +0,0 @@ -package main - -import ( - "encoding/json" - "flag" - "fmt" - "io/ioutil" - "log" - "net" - "net/http" - "os" - "os/exec" - "os/signal" - "strconv" - "sync" - - "github.com/hashicorp/consul/test/porter" -) - -var addrFile = "/tmp/porter.addr" - -var ( - addr string - firstPort int - lastPort int - verbose bool - - mu sync.Mutex - port int -) - -func main() { - log.SetFlags(0) - - flag.StringVar(&addr, "addr", porter.DefaultAddr, "host:port") - flag.IntVar(&firstPort, "first-port", 10000, "first port to allocate") - flag.IntVar(&lastPort, "last-port", 20000, "last port to allocate") - flag.BoolVar(&verbose, "verbose", false, "log port allocations") - flag.Parse() - - // check if there is an instance running - startServer := true - b, err := ioutil.ReadFile(addrFile) - if err == nil { - addr = string(b) - conn, err := net.Dial("tcp", addr) - if err == nil { - log.Println("found running porter instance at", addr) - startServer = false - conn.Close() - } else { - log.Printf("found dead porter instance at %s, will take over", addr) - } - } - - args := flag.Args() - if startServer { - if err := ioutil.WriteFile(addrFile, []byte(addr), 0644); err != nil { - log.Fatalf("Cannot write %s: %s", addrFile, err) - } - defer os.Remove(addrFile) - go func() { - http.HandleFunc("/", servePort) - if err := http.ListenAndServe(addr, nil); err != nil { - log.Fatal(err) - } - }() - } else { - if len(args) == 0 { - log.Println("no command and existing porter instance found, exiting") - os.Exit(0) - } - } - - // no command to run: wait for CTRL-C - if len(args) == 0 { - log.Print("PORTER_ADDR=" + addr) - c := make(chan os.Signal, 1) - signal.Notify(c, os.Interrupt) - s := <-c - log.Println("Got signal:", s) - return - } - - // run command and exit with 1 in case of error - if err := run(args); err != nil { - log.Fatal(err) - } -} - -func run(args []string) error { - path, err := exec.LookPath(args[0]) - if err != nil { - return fmt.Errorf("Cannot find %q in path", args[0]) - } - - cmd := exec.Command(path, args[1:]...) - if os.Getenv("PORTER_ADDR") == "" { - cmd.Env = append(os.Environ(), "PORTER_ADDR="+addr) - } - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - return cmd.Run() -} - -func servePort(w http.ResponseWriter, r *http.Request) { - var count int - n, err := strconv.Atoi(r.RequestURI[1:]) - if err == nil { - count = n - } - if count <= 0 { - count = 1 - } - - // getPort assumes the lock is already held and tries to return a port - // that's not in use. It will panic if it has to try too many times. - getPort := func() int { - for i := 0; i < 10; i++ { - port++ - if port < firstPort { - port = firstPort - } - if port >= lastPort { - port = firstPort - } - - conn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port)) - if err != nil { - return port - } - conn.Close() - if verbose { - log.Printf("porter: skipping port %d, already in use", port) - } - } - panic(fmt.Errorf("could not find a free port")) - } - - p := make([]int, count) - mu.Lock() - for i := 0; i < count; i++ { - p[i] = getPort() - } - mu.Unlock() - - if err := json.NewEncoder(w).Encode(p); err != nil { - // this shouldn't happen so we panic since we can't recover - panic(err) - } - if verbose { - log.Printf("porter: allocated ports %v", p) - } -} diff --git a/testutil/porter_test.go b/testutil/porter_test.go deleted file mode 100644 index 8595b74b69..0000000000 --- a/testutil/porter_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package testutil - -import ( - "testing" - - "github.com/hashicorp/consul/test/porter" -) - -func Test_PorterIsRunning(t *testing.T) { - if _, err := porter.RandomPorts(1); err != nil { - t.Fatalf("Porter must be running for Consul's unit tests") - } -} diff --git a/testutil/server.go b/testutil/server.go index 93b734a671..4993f13cb9 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -27,7 +27,7 @@ import ( "testing" "time" - "github.com/hashicorp/consul/test/porter" + "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/consul/testutil/retry" "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/go-uuid" @@ -111,17 +111,7 @@ func defaultServerConfig() *TestServerConfig { panic(err) } - ports, err := porter.RandomPorts(6) - if err != nil { - if _, ok := err.(*porter.PorterExistErr); ok { - // Fall back in the case that the testutil server is being used - // without porter. This should NEVER be used for Consul's own - // unit tests. See comments for getRandomPorts() for more details. - ports = getRandomPorts(6) - } else { - panic(err) - } - } + ports := freeport.Get(6) return &TestServerConfig{ NodeName: "node-" + nodeID, NodeID: nodeID, @@ -390,22 +380,3 @@ func (s *TestServer) waitForLeader() error { } return nil } - -// getRandomPorts returns a set of random port or panics on error. This -// is here to support external uses of testutil which may not have porter, -// but this has been shown not to work well with parallel tests (such as -// Consul's unit tests). This fallback should NEVER be used for Consul's -// own tests. -func getRandomPorts(n int) []int { - ports := make([]int, n) - for i := 0; i < n; i++ { - l, err := net.Listen("tcp", ":0") - if err != nil { - panic(err) - } - l.Close() - ports[i] = l.Addr().(*net.TCPAddr).Port - } - - return ports -}