diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index d9202562668..1bb9a4e760e 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -17,7 +17,6 @@ package app import ( "flag" - "strings" "github.com/spf13/viper" @@ -110,25 +109,12 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions { cOpts.DynQueueSizeMemory = v.GetUint(collectorDynQueueSizeMemory) * 1024 * 1024 // we receive in MiB and store in bytes cOpts.QueueSize = v.GetInt(collectorQueueSize) cOpts.NumWorkers = v.GetInt(collectorNumWorkers) - cOpts.CollectorHTTPHostPort = getAddressFromCLIOptions(v.GetInt(collectorHTTPPort), v.GetString(CollectorHTTPHostPort)) - cOpts.CollectorGRPCHostPort = getAddressFromCLIOptions(v.GetInt(collectorGRPCPort), v.GetString(CollectorGRPCHostPort)) - cOpts.CollectorZipkinHTTPHostPort = getAddressFromCLIOptions(v.GetInt(collectorZipkinHTTPPort), v.GetString(CollectorZipkinHTTPHostPort)) + cOpts.CollectorHTTPHostPort = ports.GetAddressFromCLIOptions(v.GetInt(collectorHTTPPort), v.GetString(CollectorHTTPHostPort)) + cOpts.CollectorGRPCHostPort = ports.GetAddressFromCLIOptions(v.GetInt(collectorGRPCPort), v.GetString(CollectorGRPCHostPort)) + cOpts.CollectorZipkinHTTPHostPort = ports.GetAddressFromCLIOptions(v.GetInt(collectorZipkinHTTPPort), v.GetString(CollectorZipkinHTTPHostPort)) cOpts.CollectorTags = flags.ParseJaegerTags(v.GetString(collectorTags)) cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins) cOpts.CollectorZipkinAllowedHeaders = v.GetString(collectorZipkinAllowedHeaders) cOpts.TLS = tlsFlagsConfig.InitFromViper(v) return cOpts } - -// Utility function to get listening address based on port (deprecated flags) or host:port (new flags) -func getAddressFromCLIOptions(port int, hostPort string) string { - if port != 0 { - return ports.PortToHostPort(port) - } - - if strings.Contains(hostPort, ":") { - return hostPort - } - - return ":" + hostPort -} diff --git a/cmd/collector/app/builder_flags_test.go b/cmd/collector/app/builder_flags_test.go index 4ee34b0f152..0ffa0e8ec1b 100644 --- a/cmd/collector/app/builder_flags_test.go +++ b/cmd/collector/app/builder_flags_test.go @@ -51,7 +51,3 @@ func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) { assert.Equal(t, "127.0.0.1:1234", c.CollectorGRPCHostPort) assert.Equal(t, "0.0.0.0:3456", c.CollectorZipkinHTTPHostPort) } - -func Test_getAddressFromCLIOptions(t *testing.T) { - assert.Equal(t, ":123", getAddressFromCLIOptions(123, "")) -} diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index aec4c4899c6..b34a1b70306 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -36,7 +36,9 @@ import ( ) const ( + queryHostPort = "query.host-port" queryPort = "query.port" + queryPortWarning = "(deprecated, will be removed after 2020-08-31 or in release v1.20.0, whichever is later)" queryBasePath = "query.base-path" queryStaticFiles = "query.static-files" queryUIConfig = "query.ui-config" @@ -47,8 +49,8 @@ const ( // QueryOptions holds configuration for query service type QueryOptions struct { - // Port is the port that the query service listens in on - Port int + // HostPort is the host:port address that the query service listens o n + HostPort string // BasePath is the prefix for all UI and API HTTP routes BasePath string // StaticAssets is the path for the static assets for the UI (https://github.com/uber/jaeger-ui) @@ -66,7 +68,8 @@ type QueryOptions struct { // AddFlags adds flags for QueryOptions func AddFlags(flagSet *flag.FlagSet) { flagSet.Var(&config.StringSlice{}, queryAdditionalHeaders, `Additional HTTP response headers. Can be specified multiple times. Format: "Key: Value"`) - flagSet.Int(queryPort, ports.QueryHTTP, "The port for the query service") + flagSet.String(queryHostPort, ports.PortToHostPort(ports.QueryHTTP), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the query's HTTP server") + flagSet.Int(queryPort, 0, queryPortWarning+" see --"+queryHostPort) flagSet.String(queryBasePath, "/", "The base path for all HTTP routes, e.g. /jaeger; useful when running behind a reverse proxy") flagSet.String(queryStaticFiles, "", "The directory path override for the static assets for the UI") flagSet.String(queryUIConfig, "", "The path to the UI configuration file in JSON format") @@ -76,7 +79,7 @@ func AddFlags(flagSet *flag.FlagSet) { // InitFromViper initializes QueryOptions with properties from viper func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *QueryOptions { - qOpts.Port = v.GetInt(queryPort) + qOpts.HostPort = ports.GetAddressFromCLIOptions(v.GetInt(queryPort), v.GetString(queryHostPort)) qOpts.BasePath = v.GetString(queryBasePath) qOpts.StaticAssets = v.GetString(queryStaticFiles) qOpts.UIConfig = v.GetString(queryUIConfig) diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go index 3f07c56e1fc..22170ff8b24 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -28,13 +28,22 @@ import ( spanstore_mocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) +func TestQueryBuilderFlagsDeprecation(t *testing.T) { + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{ + "--query.port=80", + }) + qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) + assert.Equal(t, ":80", qOpts.HostPort) +} + func TestQueryBuilderFlags(t *testing.T) { v, command := config.Viperize(AddFlags) command.ParseFlags([]string{ "--query.static-files=/dev/null", "--query.ui-config=some.json", "--query.base-path=/jaeger", - "--query.port=80", + "--query.host-port=127.0.0.1:8080", "--query.additional-headers=access-control-allow-origin:blerg", "--query.additional-headers=whatever:thing", "--query.max-clock-skew-adjustment=10s", @@ -43,7 +52,7 @@ func TestQueryBuilderFlags(t *testing.T) { assert.Equal(t, "/dev/null", qOpts.StaticAssets) assert.Equal(t, "some.json", qOpts.UIConfig) assert.Equal(t, "/jaeger", qOpts.BasePath) - assert.Equal(t, 80, qOpts.Port) + assert.Equal(t, "127.0.0.1:8080", qOpts.HostPort) assert.Equal(t, http.Header{ "Access-Control-Allow-Origin": []string{"blerg"}, "Whatever": []string{"thing"}, diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index a8c40300145..4ce161aca87 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -1,4 +1,4 @@ -// Copyright (c) 2019 The Jaeger Authors. +// Copyright (c) 2019,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. @@ -15,7 +15,6 @@ package app import ( - "fmt" "net" "net/http" "strings" @@ -95,20 +94,21 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, // Start http, GRPC and cmux servers concurrently func (s *Server) Start() error { - conn, err := net.Listen("tcp", fmt.Sprintf(":%d", s.queryOptions.Port)) + conn, err := net.Listen("tcp", s.queryOptions.HostPort) if err != nil { return err } s.conn = conn - tcpPort := s.queryOptions.Port + var tcpPort int if port, err := netutils.GetPort(s.conn.Addr()); err == nil { tcpPort = port } s.svc.Logger.Info( "Query server started", - zap.Int("port", tcpPort)) + zap.Int("port", tcpPort), + zap.String("addr", s.queryOptions.HostPort)) // cmux server acts as a reverse-proxy between HTTP and GRPC backends. cmuxServer := cmux.New(s.conn) @@ -120,7 +120,7 @@ func (s *Server) Start() error { httpListener := cmuxServer.Match(cmux.Any()) go func() { - s.svc.Logger.Info("Starting HTTP server", zap.Int("port", tcpPort)) + s.svc.Logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) switch err := s.httpServer.Serve(httpListener); err { case nil, http.ErrServerClosed, cmux.ErrListenerClosed: @@ -133,7 +133,7 @@ func (s *Server) Start() error { // Start GRPC server concurrently go func() { - s.svc.Logger.Info("Starting GRPC server", zap.Int("port", tcpPort)) + s.svc.Logger.Info("Starting GRPC server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) if err := s.grpcServer.Serve(grpcListener); err != nil { s.svc.Logger.Error("Could not start GRPC server", zap.Error(err)) @@ -143,7 +143,7 @@ func (s *Server) Start() error { // Start cmux server concurrently. go func() { - s.svc.Logger.Info("Starting CMUX server", zap.Int("port", tcpPort)) + s.svc.Logger.Info("Starting CMUX server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) err := cmuxServer.Serve() // TODO: Remove string comparison when https://github.com/soheilhy/cmux/pull/69 is merged diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 2afa1460d52..7c85c417299 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2019 The Jaeger Authors. +// Copyright (c) 2019,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. @@ -16,7 +16,6 @@ package app import ( "context" - "fmt" "testing" "time" @@ -38,7 +37,7 @@ import ( func TestServerError(t *testing.T) { srv := &Server{ queryOptions: &QueryOptions{ - Port: -1, + HostPort: ":-1", }, } assert.Error(t, srv.Start()) @@ -47,6 +46,7 @@ func TestServerError(t *testing.T) { func TestServer(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() + hostPort := ports.GetAddressFromCLIOptions(ports.QueryHTTP, "") spanReader := &spanstoremocks.Reader{} dependencyReader := &depsmocks.Reader{} @@ -56,11 +56,11 @@ func TestServer(t *testing.T) { querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{}) server := NewServer(flagsSvc, querySvc, - &QueryOptions{Port: ports.QueryHTTP, BearerTokenPropagation: true}, + &QueryOptions{HostPort: hostPort, BearerTokenPropagation: true}, opentracing.NoopTracer{}) assert.NoError(t, server.Start()) - client := newGRPCClient(t, fmt.Sprintf(":%d", ports.QueryHTTP)) + client := newGRPCClient(t, hostPort) defer client.conn.Close() ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) @@ -90,7 +90,7 @@ func TestServerGracefulExit(t *testing.T) { querySvc := &querysvc.QueryService{} tracer := opentracing.NoopTracer{} - server := NewServer(flagsSvc, querySvc, &QueryOptions{Port: ports.QueryAdminHTTP}, tracer) + server := NewServer(flagsSvc, querySvc, &QueryOptions{HostPort: ports.PortToHostPort(ports.QueryAdminHTTP)}, tracer) assert.NoError(t, server.Start()) // Wait for servers to come up before we can call .Close() @@ -111,7 +111,7 @@ func TestServerHandlesPortZero(t *testing.T) { querySvc := &querysvc.QueryService{} tracer := opentracing.NoopTracer{} - server := NewServer(flagsSvc, querySvc, &QueryOptions{Port: 0}, tracer) + server := NewServer(flagsSvc, querySvc, &QueryOptions{HostPort: ":0"}, tracer) assert.NoError(t, server.Start()) server.Close() @@ -119,6 +119,6 @@ func TestServerHandlesPortZero(t *testing.T) { assert.Equal(t, 1, message.Len(), "Expected query started log message.") onlyEntry := message.All()[0] - port := onlyEntry.ContextMap()["port"].(int64) + port := onlyEntry.ContextMap()["port"] assert.Greater(t, port, int64(0)) } diff --git a/ports/ports.go b/ports/ports.go index cba71cbbb11..79048b35a83 100644 --- a/ports/ports.go +++ b/ports/ports.go @@ -16,6 +16,7 @@ package ports import ( "strconv" + "strings" ) const ( @@ -50,3 +51,16 @@ const ( func PortToHostPort(port int) string { return ":" + strconv.Itoa(port) } + +// GetAddressFromCLIOptions gets listening address based on port (deprecated flags) or host:port (new flags) +func GetAddressFromCLIOptions(port int, hostPort string) string { + if port != 0 { + return PortToHostPort(port) + } + + if strings.Contains(hostPort, ":") { + return hostPort + } + + return ":" + hostPort +} diff --git a/ports/ports_test.go b/ports/ports_test.go index 29ff987d8a7..51385afec03 100644 --- a/ports/ports_test.go +++ b/ports/ports_test.go @@ -23,3 +23,19 @@ import ( func TestPortToHostPort(t *testing.T) { assert.Equal(t, ":42", PortToHostPort(42)) } + +func TestGetAddressFromCLIOptionsLegacy(t *testing.T) { + assert.Equal(t, ":123", GetAddressFromCLIOptions(123, "")) +} + +func TestGetAddressFromCLIOptionHostPort(t *testing.T) { + assert.Equal(t, "127.0.0.1:123", GetAddressFromCLIOptions(0, "127.0.0.1:123")) +} + +func TestGetAddressFromCLIOptionOnlyPort(t *testing.T) { + assert.Equal(t, ":123", GetAddressFromCLIOptions(0, ":123")) +} + +func TestGetAddressFromCLIOptionOnlyPortWithoutColon(t *testing.T) { + assert.Equal(t, ":123", GetAddressFromCLIOptions(0, "123")) +}