Admin/Query: Log the real port instead of the provided one to enable the use of port 0

cmd/flags/admin.go

@@ -26,6 +26,7 @@ import (
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
+	"github.com/jaegertracing/jaeger/pkg/netutils"
 	"github.com/jaegertracing/jaeger/pkg/recoveryhandler"
 	"github.com/jaegertracing/jaeger/pkg/version"
 )
@@ -97,9 +98,15 @@ func (s *AdminServer) Serve() error {
 		return err
 	}
 	s.serveWithListener(l)
+
+	tcpPort := s.adminPort
+	if port, err := netutils.GetPort(l.Addr()); err == nil {
+		tcpPort = port
+	}
+
 	s.logger.Info(
 		"Admin server started",
-		zap.Int("http-port", s.adminPort),
+		zap.Int("http-port", tcpPort),
 		zap.Stringer("health-status", s.hc.Get()))
 	return nil
 }

cmd/flags/admin_test.go

@@ -0,0 +1,46 @@
+// Copyright (c) 2020 The Jaeger Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package flags
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest/observer"
+
+	"github.com/jaegertracing/jaeger/pkg/config"
+)
+
+func TestAdminServerHandlesPortZero(t *testing.T) {
+	adminServer := NewAdminServer(0)
+
+	v, _ := config.Viperize(adminServer.AddFlags)
+
+	zapCore, logs := observer.New(zap.InfoLevel)
+	logger := zap.New(zapCore)
+
+	adminServer.initFromViper(v, logger)
+
+	assert.NoError(t, adminServer.Serve())
+	defer adminServer.Close()
+
+	message := logs.FilterMessage("Admin server started")
+	assert.Equal(t, 1, message.Len(), "Expected Admin server started log message.")
+
+	onlyEntry := message.All()[0]
+	port := onlyEntry.ContextMap()["http-port"].(int64)
+	assert.Greater(t, port, int64(0))
+}

cmd/query/app/server.go

@@ -29,6 +29,7 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/flags"
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
+	"github.com/jaegertracing/jaeger/pkg/netutils"
 	"github.com/jaegertracing/jaeger/pkg/recoveryhandler"
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2"
 )
@@ -99,6 +100,15 @@ func (s *Server) Start() error {
 	}
 	s.conn = conn
 
+	tcpPort := s.queryOptions.Port
+	if port, err := netutils.GetPort(s.conn.Addr()); err == nil {
+		tcpPort = port
+	}
+
+	s.svc.Logger.Info(
+		"Query server started",
+		zap.Int("port", tcpPort))
+
 	// cmux server acts as a reverse-proxy between HTTP and GRPC backends.
 	cmuxServer := cmux.New(s.conn)
 
@@ -109,7 +119,7 @@ func (s *Server) Start() error {
 	httpListener := cmuxServer.Match(cmux.Any())
 
 	go func() {
-		s.svc.Logger.Info("Starting HTTP server", zap.Int("port", s.queryOptions.Port))
+		s.svc.Logger.Info("Starting HTTP server", zap.Int("port", tcpPort))
 
 		switch err := s.httpServer.Serve(httpListener); err {
 		case nil, http.ErrServerClosed, cmux.ErrListenerClosed:
@@ -122,7 +132,7 @@ func (s *Server) Start() error {
 
 	// Start GRPC server concurrently
 	go func() {
-		s.svc.Logger.Info("Starting GRPC server", zap.Int("port", s.queryOptions.Port))
+		s.svc.Logger.Info("Starting GRPC server", zap.Int("port", tcpPort))
 
 		if err := s.grpcServer.Serve(grpcListener); err != nil {
 			s.svc.Logger.Error("Could not start GRPC server", zap.Error(err))
@@ -132,7 +142,7 @@ func (s *Server) Start() error {
 
 	// Start cmux server concurrently.
 	go func() {
-		s.svc.Logger.Info("Starting CMUX server", zap.Int("port", s.queryOptions.Port))
+		s.svc.Logger.Info("Starting CMUX server", zap.Int("port", tcpPort))
 
 		err := cmuxServer.Serve()
 		// TODO: Remove string comparison when https://github.com/soheilhy/cmux/pull/69 is merged

cmd/query/app/server_test.go

@@ -103,3 +103,22 @@ func TestServerGracefulExit(t *testing.T) {
 			"Error log found on server exit: %v", logEntry)
 	}
 }
+
+func TestServerHandlesPortZero(t *testing.T) {
+	flagsSvc := flags.NewService(ports.QueryAdminHTTP)
+	zapCore, logs := observer.New(zap.InfoLevel)
+	flagsSvc.Logger = zap.New(zapCore)
+
+	querySvc := &querysvc.QueryService{}
+	tracer := opentracing.NoopTracer{}
+	server := NewServer(flagsSvc, querySvc, &QueryOptions{Port: 0}, tracer)
+	assert.NoError(t, server.Start())
+	server.Close()
+
+	message := logs.FilterMessage("Query server started")
+	assert.Equal(t, 1, message.Len(), "Expected query started log message.")
+
+	onlyEntry := message.All()[0]
+	port := onlyEntry.ContextMap()["port"].(int64)
+	assert.Greater(t, port, int64(0))
+}

pkg/netutils/port.go

@@ -0,0 +1,35 @@
+// Copyright (c) 2020 The Jaeger Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package netutils
+
+import (
+	"net"
+	"strconv"
+)
+
+// GetPort returns the port of an endpoint address.
+func GetPort(addr net.Addr) (int, error) {
+	_, port, err := net.SplitHostPort(addr.String())
+	if err != nil {
+		return -1, err
+	}
+
+	parsedPort, err := strconv.Atoi(port)
+	if err != nil {
+		return -1, err
+	}
+
+	return parsedPort, nil
+}

pkg/netutils/port_test.go

@@ -0,0 +1,34 @@
+// Copyright (c) 2020 The Jaeger Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package netutils
+
+import (
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPortZero(t *testing.T) {
+	lis, err := net.Listen("tcp", "localhost:0")
+	require.NoError(t, err)
+	defer lis.Close()
+
+	port, err := GetPort(lis.Addr())
+	require.NoError(t, err)
+
+	assert.Greater(t, port, 0)
+}