Fix port allocation race condition during tests. (#215)

Makefile

@@ -570,8 +570,8 @@ build:
 test:
 	$(GO) test ./... -race
 
-test-10:
-	$(GO) test ./... -race -test.count 10 -cover
+test-in-ci:
+	$(GO) test ./... -race -test.count 25 -cover
 
 fmt:
 	$(GO) fmt ./...
@@ -736,4 +736,4 @@ sync-deps:
 sleep-10:
 	sleep 10
 
-.PHONY: deploy-redirect-site sync-deps sleep-10 proxy-dashboard proxy-prometheus proxy-grafana clean clean-toolchain clean-binaries clean-protos presubmit test test-10 vet
+.PHONY: deploy-redirect-site sync-deps sleep-10 proxy-dashboard proxy-prometheus proxy-grafana clean clean-toolchain clean-binaries clean-protos presubmit test test-in-ci vet

cloudbuild.yaml

@@ -104,7 +104,7 @@ steps:
   waitFor: ['Build: Protocol Buffers']
 - id: 'Build: Test'
   name: open-match-build
-  args: ['make', 'GOPROXY=off', 'test-10']
+  args: ['make', 'GOPROXY=off', 'test-in-ci']
   volumes:
   - name: 'go-vol'
     path: '/go'

go.mod

@@ -25,6 +25,7 @@ require (
 	github.com/modern-go/reflect2 v1.0.1 // indirect
 	github.com/opencensus-integrations/redigo v2.0.1+incompatible
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
+	github.com/pkg/errors v0.8.0
 	github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90 // indirect
 	github.com/prometheus/procfs v0.0.0-20190403104016-ea9eea638872 // indirect
 	github.com/rafaeljusto/redigomock v0.0.0-20190202135759-257e089e14a1

go.sum

@@ -117,6 +117,7 @@ github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/9
 github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI=
 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
 github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
+github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

internal/app/mmforc/mmforc.go

@@ -29,6 +29,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/GoogleCloudPlatform/open-match/internal/util/netlistener"
+
 	"github.com/GoogleCloudPlatform/open-match/config"
 	"github.com/GoogleCloudPlatform/open-match/internal/logging"
 	"github.com/GoogleCloudPlatform/open-match/internal/metrics"
@@ -106,7 +108,15 @@ func initializeApplication() (config.View, error) {
 	ocMmforcViews := DefaultMmforcViews                                              // mmforc OpenCensus views.
 	ocMmforcViews = append(ocMmforcViews, redigometrics.ObservabilityMetricViews...) // redis OpenCensus views.
 	mmforcLog.WithFields(log.Fields{"viewscount": len(ocMmforcViews)}).Info("Loaded OpenCensus views")
-	metrics.ConfigureOpenCensusPrometheusExporter(cfg, ocMmforcViews)
+
+	lh, err := netlistener.NewFromPortNumber(cfg.GetInt("metrics.port"))
+	if err != nil {
+		mmforcLog.WithFields(log.Fields{
+			"error": err.Error(),
+		}).Error("Unable to create metrics TCP listener")
+		return nil, err
+	}
+	metrics.ConfigureOpenCensusPrometheusExporter(lh, cfg, ocMmforcViews)
 	return cfg, nil
 }
 

internal/metrics/helper.go

@@ -21,10 +21,10 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"strconv"
 	"time"
 
 	"github.com/GoogleCloudPlatform/open-match/config"
+	"github.com/GoogleCloudPlatform/open-match/internal/util/netlistener"
 	log "github.com/sirupsen/logrus"
 
 	"go.opencensus.io/exporter/prometheus"
@@ -46,10 +46,9 @@ var (
 // by Promethus for  metrics gathering. The calling code can select any views
 // it wants to  register, from any number of libraries, and pass them in as an
 // array.
-func ConfigureOpenCensusPrometheusExporter(cfg config.View, views []*view.View) {
-
+func ConfigureOpenCensusPrometheusExporter(lh *netlistener.ListenerHolder, cfg config.View, views []*view.View) {
 	//var infoCtx, err = tag.New(context.Background(), tag.Insert(KeySeverity, "info"))
-	metricsPort := cfg.GetInt("metrics.port")
+	metricsPort := lh.Number()
 	metricsEP := cfg.GetString("metrics.endpoint")
 	metricsRP := cfg.GetInt("metrics.reportingPeriod")
 
@@ -85,7 +84,15 @@ func ConfigureOpenCensusPrometheusExporter(cfg config.View, views []*view.View)
 			"port":     metricsPort,
 			"endpoint": metricsEP,
 		}).Info("Attempting to start http server for OpenCensus metrics on localhost")
-		err := http.ListenAndServe(":"+strconv.Itoa(metricsPort), mux)
+		listener, err := lh.Obtain()
+		if err != nil {
+			mhLog.WithFields(log.Fields{
+				"error":    err,
+				"port":     metricsPort,
+				"endpoint": metricsEP,
+			}).Fatal("Failed to run Prometheus endpoint")
+		}
+		err = http.Serve(listener, mux)
 		if err != nil {
 			mhLog.WithFields(log.Fields{
 				"error":    err,

internal/serving/construction.go

@@ -5,6 +5,7 @@ import (
 	"github.com/GoogleCloudPlatform/open-match/internal/logging"
 	"github.com/GoogleCloudPlatform/open-match/internal/metrics"
 	redishelpers "github.com/GoogleCloudPlatform/open-match/internal/statestorage/redis"
+	"github.com/GoogleCloudPlatform/open-match/internal/util/netlistener"
 	"github.com/opencensus-integrations/redigo/redis"
 	log "github.com/sirupsen/logrus"
 	"go.opencensus.io/plugin/ocgrpc"
@@ -67,7 +68,14 @@ func NewMulti(paramsList []*ServerParams) (*OpenMatchServer, error) {
 	ocServerViews = append(ocServerViews, config.CfgVarCountView)            // config loader view.
 	ocServerViews = append(ocServerViews, redis.ObservabilityMetricViews...) // redis OpenCensus views.
 	logger.WithFields(log.Fields{"viewscount": len(ocServerViews)}).Info("Loaded OpenCensus views")
-	metrics.ConfigureOpenCensusPrometheusExporter(cfg, ocServerViews)
+	promLh, err := netlistener.NewFromPortNumber(cfg.GetInt("metrics.port"))
+	if err != nil {
+		logger.WithFields(log.Fields{
+			"error": err.Error(),
+		}).Error("Unable to create metrics TCP listener")
+		return nil, err
+	}
+	metrics.ConfigureOpenCensusPrometheusExporter(promLh, cfg, ocServerViews)
 
 	// Connect to redis
 	pool, err := redishelpers.ConnectionPool(cfg)
@@ -77,7 +85,13 @@ func NewMulti(paramsList []*ServerParams) (*OpenMatchServer, error) {
 	}
 
 	// Instantiate the gRPC server with the bindings we've made.
-	grpcServer := NewGrpcServer(cfg.GetInt(paramsList[0].PortConfigName), logger)
+	grpcLh, err := netlistener.NewFromPortNumber(cfg.GetInt(paramsList[0].PortConfigName))
+	if err != nil {
+		logger.Fatal(err)
+		return nil, err
+	}
+
+	grpcServer := NewGrpcServer(grpcLh, logger)
 
 	omServer := &OpenMatchServer{
 		GrpcServer: grpcServer,

internal/serving/grpcserver.go

@@ -1,9 +1,9 @@
 package serving
 
 import (
-	"fmt"
 	"net"
 
+	"github.com/GoogleCloudPlatform/open-match/internal/util/netlistener"
 	log "github.com/sirupsen/logrus"
 	"go.opencensus.io/plugin/ocgrpc"
 
@@ -12,7 +12,7 @@ import (
 
 // GrpcWrapper is a decoration around the standard GRPC server that sets up a bunch of things common to Open Match servers.
 type GrpcWrapper struct {
-	port         int
+	lh           *netlistener.ListenerHolder
 	handlerFuncs []func(*grpc.Server)
 	server       *grpc.Server
 	ln           net.Listener
@@ -21,9 +21,9 @@ type GrpcWrapper struct {
 }
 
 // NewGrpcServer creates a new GrpcWrapper.
-func NewGrpcServer(port int, logger *log.Entry) *GrpcWrapper {
+func NewGrpcServer(lh *netlistener.ListenerHolder, logger *log.Entry) *GrpcWrapper {
 	return &GrpcWrapper{
-		port:         port,
+		lh:           lh,
 		logger:       logger,
 		handlerFuncs: []func(*grpc.Server){},
 	}
@@ -39,17 +39,17 @@ func (gw *GrpcWrapper) Start() error {
 	if gw.ln != nil {
 		return nil
 	}
-	ln, err := net.Listen("tcp", fmt.Sprintf(":%d", gw.port))
+	ln, err := gw.lh.Obtain()
 	if err != nil {
 		gw.logger.WithFields(log.Fields{
 			"error": err.Error(),
-			"port":  gw.port,
+			"port":  gw.lh.Number(),
 		}).Error("net.Listen() error")
 		return err
 	}
 	gw.ln = ln
 
-	gw.logger.WithFields(log.Fields{"port": gw.port}).Info("TCP net listener initialized")
+	gw.logger.WithFields(log.Fields{"port": gw.lh.Number()}).Info("TCP net listener initialized")
 
 	server := grpc.NewServer(grpc.StatsHandler(&ocgrpc.ServerHandler{}))
 	for _, handlerFunc := range gw.handlerFuncs {
@@ -59,7 +59,7 @@ func (gw *GrpcWrapper) Start() error {
 	gw.grpcAwaiter = make(chan error)
 
 	go func() {
-		gw.logger.Infof("Serving gRPC on :%d", gw.port)
+		gw.logger.Infof("Serving gRPC on :%d", gw.lh.Number())
 		err := gw.server.Serve(ln)
 		gw.grpcAwaiter <- err
 		if err != nil {

internal/serving/testing/minimatch.go

@@ -10,6 +10,7 @@ import (
 	"github.com/GoogleCloudPlatform/open-match/internal/pb"
 	"github.com/GoogleCloudPlatform/open-match/internal/serving"
 	redishelpers "github.com/GoogleCloudPlatform/open-match/internal/statestorage/redis"
+	netlistenerTesting "github.com/GoogleCloudPlatform/open-match/internal/util/netlistener/testing"
 	"github.com/alicebob/miniredis"
 	"github.com/opencensus-integrations/redigo/redis"
 	log "github.com/sirupsen/logrus"
@@ -90,12 +91,9 @@ func createOpenMatchServer(paramsList []*serving.ServerParams) (*MiniMatchServer
 	cfg.Set("logging.format", "text")
 	cfg.Set("logging.source", true)
 
-	promPort, err := NextPort()
-	if err != nil {
-		return nil, fmt.Errorf("cannot create port for prometheus, %s", err)
-	}
+	promListener := netlistenerTesting.MustListen()
 
-	cfg.Set("metrics.port", promPort)
+	cfg.Set("metrics.port", promListener.Number())
 	cfg.Set("metrics.endpoint", "/metrics")
 	cfg.Set("metrics.reportingPeriod", "5s")
 
@@ -116,7 +114,7 @@ func createOpenMatchServer(paramsList []*serving.ServerParams) (*MiniMatchServer
 	ocServerViews = append(ocServerViews, config.CfgVarCountView)            // config loader view.
 	ocServerViews = append(ocServerViews, redis.ObservabilityMetricViews...) // redis OpenCensus views.
 	logger.WithFields(log.Fields{"viewscount": len(ocServerViews)}).Info("Loaded OpenCensus views")
-	metrics.ConfigureOpenCensusPrometheusExporter(cfg, ocServerViews)
+	metrics.ConfigureOpenCensusPrometheusExporter(promListener, cfg, ocServerViews)
 
 	// Connect to redis
 	mredis, err := miniredis.Run()
@@ -152,18 +150,14 @@ func createOpenMatchServer(paramsList []*serving.ServerParams) (*MiniMatchServer
 		return nil, err
 	}
 
-	grpcPort, err := NextPort()
-	if err != nil {
-		closeOnFailure()
-		return nil, fmt.Errorf("cannot provision a port for gRPC, %s", err)
-	}
+	grpcLh := netlistenerTesting.MustListen()
 	for _, params := range paramsList {
-		cfg.Set(params.PortConfigName, grpcPort)
+		cfg.Set(params.PortConfigName, grpcLh.Number())
 	}
 
 	// Instantiate the gRPC server with the connections we've made
 	logger.Info("Attempting to start gRPC server")
-	grpcServer := serving.NewGrpcServer(grpcPort, logger)
+	grpcServer := serving.NewGrpcServer(grpcLh, logger)
 
 	mmServer := &MiniMatchServer{
 		OpenMatchServer: &serving.OpenMatchServer{