grpc: Split call of net.Listen out from Serve

This is going to be useful for the next commit that fixes up
some flakiness in this package's tests.

Author: mmlb

grpc/grpc.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net"
 	"strconv"
+	"sync"
 
 	grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
 	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
@@ -22,13 +23,17 @@ import (
 // Server is used to hold configured info and ultimately the grpc server
 type Server struct {
 	err       error
-	port      int
 	server    *grpc.Server
 	cert      tls.Certificate
 	options   []grpc.ServerOption
 	streamers []grpc.StreamServerInterceptor
 	unariers  []grpc.UnaryServerInterceptor
 	registry  []func(*grpc.Server)
+
+	mu       sync.RWMutex
+	port     int
+	once     sync.Once
+	listener net.Listener
 }
 
 // The ServiceRegister type is used as a callback once the underlying grpc server is setup to register the main service.
@@ -99,6 +104,8 @@ func NewServer(l log.Logger, reg ServiceRegister, options ...Option) (*Server, e
 
 // Port returns the port the server is listening on
 func (s *Server) Port() int {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
 	return s.port
 }
 
@@ -107,15 +114,49 @@ func (s *Server) Server() *grpc.Server {
 	return s.server
 }
 
+func (s *Server) listen() error {
+	var err error
+	s.once.Do(func() {
+		s.mu.RLock()
+		port := s.port
+		s.mu.RUnlock()
+
+		s.listener, err = net.Listen("tcp", fmt.Sprintf(":%d", port)) //nolint:ineffassign // we do in fact want to assign to err in the outer scope
+		if err != nil {
+			err = errors.Wrap(err, "listen")
+			return
+		}
+
+		var p string
+		_, p, err = net.SplitHostPort(s.listener.Addr().String()) //nolint:ineffassign // we do in fact want to assign to err in the outer scope
+		if err != nil {
+			err = errors.Wrap(err, "extract server port")
+			return
+		}
+
+		port, err = strconv.Atoi(p)
+		if err != nil {
+			err = errors.Wrap(err, "parse server port")
+			return
+		}
+
+		s.mu.Lock()
+		s.port = port
+		s.mu.Unlock()
+	})
+
+	return err
+}
+
 // Serve starts the grpc server
 func (s *Server) Serve() error {
-	lis, err := net.Listen("tcp", fmt.Sprintf(":%d", s.port))
+	err := s.listen()
 	if err != nil {
-		return errors.Wrap(err, "listen")
+		return err
 	}
-	defer lis.Close()
 
-	return errors.Wrap(s.server.Serve(lis), "serve")
+	defer s.listener.Close()
+	return errors.Wrap(s.server.Serve(s.listener), "serve")
 }
 
 // ServerOption will add the opt param to the underlying grpc.NewServer() call.

grpc/grpc_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/packethost/pkg/internal/testenv"
 	"github.com/packethost/pkg/log"
 	"github.com/stretchr/testify/require"
+	assert "github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 	pb "google.golang.org/grpc/examples/helloworld/helloworld"
@@ -65,7 +66,7 @@ func TestDefaultsAndOrdering(t *testing.T) {
 	s, err := NewServer(l, srv, pre1, Register(reg1), pre2, Register(reg2))
 	assert.NoError(err)
 	assert.NotNil(s)
-	assert.Equal(s.port, 8080)
+	assert.Equal(s.Port(), 8080)
 
 	// ensure order
 	assert.True(pre1I > 0)
@@ -84,7 +85,7 @@ func TestPort(t *testing.T) {
 	s, err := NewServer(l, defSrv)
 	assert.NoError(err)
 	assert.NotNil(s)
-	assert.Equal(s.port, 8080)
+	assert.Equal(s.Port(), 8080)
 	assert.Equal(s.Port(), 8080)
 
 	os.Setenv("GRPC_PORT", "4242")
@@ -93,13 +94,13 @@ func TestPort(t *testing.T) {
 	s, err = NewServer(l, defSrv)
 	assert.NoError(err)
 	assert.NotNil(s)
-	assert.Equal(s.port, 4242)
+	assert.Equal(s.Port(), 4242)
 	assert.Equal(s.Port(), 4242)
 
 	s, err = NewServer(l, defSrv, Port(2424))
 	assert.NoError(err)
 	assert.NotNil(s)
-	assert.Equal(s.port, 2424)
+	assert.Equal(s.Port(), 2424)
 	assert.Equal(s.Port(), 2424)
 
 	os.Setenv("GRPC_PORT", "0")
@@ -245,7 +246,7 @@ func TestX509(t *testing.T) {
 		assert.NoError(err)
 		assert.NotNil(s)
 		serve(t, s, func() {
-			assert.NoError(connectGRPC(t, s.port, ""))
+			assert.NoError(connectGRPC(t, s.Port(), ""))
 		})
 	})
 
@@ -262,8 +263,8 @@ func TestX509(t *testing.T) {
 		assert.NoError(err)
 		assert.NotNil(s)
 		serve(t, s, func() {
-			assert.Error(connectGRPC(t, s.port, ""))
-			assert.NoError(connectGRPC(t, s.port, certE))
+			assert.Error(connectGRPC(t, s.Port(), ""))
+			assert.NoError(connectGRPC(t, s.Port(), certE))
 		})
 	})
 
@@ -276,9 +277,9 @@ func TestX509(t *testing.T) {
 		assert.NoError(err)
 		assert.NotNil(s)
 		serve(t, s, func() {
-			assert.Error(connectGRPC(t, s.port, ""))
-			assert.Error(connectGRPC(t, s.port, certE))
-			assert.NoError(connectGRPC(t, s.port, certKP))
+			assert.Error(connectGRPC(t, s.Port(), ""))
+			assert.Error(connectGRPC(t, s.Port(), certE))
+			assert.NoError(connectGRPC(t, s.Port(), certKP))
 		})
 	})
 
@@ -292,10 +293,10 @@ func TestX509(t *testing.T) {
 		assert.NoError(err)
 		assert.NotNil(s)
 		serve(t, s, func() {
-			assert.Error(connectGRPC(t, s.port, ""))
-			assert.Error(connectGRPC(t, s.port, certE))
-			assert.Error(connectGRPC(t, s.port, certKP))
-			assert.NoError(connectGRPC(t, s.port, certLKP))
+			assert.Error(connectGRPC(t, s.Port(), ""))
+			assert.Error(connectGRPC(t, s.Port(), certE))
+			assert.Error(connectGRPC(t, s.Port(), certKP))
+			assert.NoError(connectGRPC(t, s.Port(), certLKP))
 		})
 	})
 
@@ -308,3 +309,52 @@ func TestX509(t *testing.T) {
 		assert.Nil(s)
 	})
 }
+
+func TestListen(t *testing.T) {
+	t.Run("invalid port fails", func(t *testing.T) {
+		s := Server{port: -1}
+		assert.Error(t, s.listen())
+		assert.Nil(t, s.listener)
+	})
+	t.Run("port 0 updates .port", func(t *testing.T) {
+		s := Server{}
+		assert.NoError(t, s.listen())
+		assert.NotNil(t, s.listener)
+		defer s.listener.Close()
+
+		assert.NotZero(t, s.Port())
+	})
+	t.Run("can run 2 servers by using port 0", func(t *testing.T) {
+		s1 := Server{}
+		assert.NoError(t, s1.listen())
+		assert.NotNil(t, s1.listener)
+		defer s1.listener.Close()
+		assert.NotZero(t, s1.port)
+
+		s2 := Server{}
+		assert.NoError(t, s2.listen())
+		assert.NotNil(t, s2.listener)
+		defer s2.listener.Close()
+		assert.NotZero(t, s2.port)
+
+		assert.NotEqual(t, s1.port, s2.port)
+	})
+	t.Run("Serve calls s.listen if necessary", func(t *testing.T) {
+		l := log.Test(t, t.Name())
+
+		s, err := NewServer(l, defSrv)
+		assert.NoError(t, err)
+		assert.NotNil(t, s)
+
+		go func() {
+			if err := s.Serve(); err != nil {
+				panic(err)
+			}
+		}()
+
+		for port := s.Port(); port == 0; port = s.Port() {
+			time.Sleep(100 * time.Microsecond)
+		}
+		assert.NoError(t, connectGRPC(t, s.Port(), ""))
+	})
+}