From 99718193119382e8fd1ce5bb60968eabd5e37a47 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Tue, 6 Sep 2022 15:44:39 +0200 Subject: [PATCH] cli,server: fix `--sql-advertise-addr` when `--sql-addr` is not specified Release justification: bug fix Release note (bug fix): The flag `--sql-advertise-addr` now properly works even when the SQL and RPC ports are shared (because `--sql-addr` was not specified). Note that this port sharing is a deprecated feature in v22.2. --- pkg/cli/flags.go | 10 ++++++---- pkg/cli/flags_test.go | 10 ++++++++++ pkg/server/start_listen.go | 12 ++++++++---- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index 55819e1ac9ff..5e228bb40221 100644 --- a/pkg/cli/flags.go +++ b/pkg/cli/flags.go @@ -1049,17 +1049,19 @@ func extraServerFlagInit(cmd *cobra.Command) error { serverCfg.SplitListenSQL = changed(fs, cliflags.ListenSQLAddr.Name) // Fill in the defaults for --advertise-sql-addr, if the flag exists on `cmd`. - advSpecified := changed(fs, cliflags.AdvertiseAddr.Name) || + advHostSpecified := changed(fs, cliflags.AdvertiseAddr.Name) || changed(fs, cliflags.AdvertiseHost.Name) + advPortSpecified := changed(fs, cliflags.AdvertiseAddr.Name) || + changed(fs, cliflags.AdvertisePort.Name) if serverSQLAdvertiseAddr == "" { - if advSpecified { + if advHostSpecified { serverSQLAdvertiseAddr = serverAdvertiseAddr } else { serverSQLAdvertiseAddr = serverSQLAddr } } if serverSQLAdvertisePort == "" { - if advSpecified && !serverCfg.SplitListenSQL { + if advPortSpecified && !serverCfg.SplitListenSQL { serverSQLAdvertisePort = serverAdvertisePort } else { serverSQLAdvertisePort = serverSQLPort @@ -1097,7 +1099,7 @@ func extraServerFlagInit(cmd *cobra.Command) error { serverCfg.HTTPAddr = net.JoinHostPort(serverHTTPAddr, serverHTTPPort) if serverHTTPAdvertiseAddr == "" { - if advSpecified { + if advHostSpecified || advPortSpecified { serverHTTPAdvertiseAddr = serverAdvertiseAddr } else { serverHTTPAdvertiseAddr = serverHTTPAddr diff --git a/pkg/cli/flags_test.go b/pkg/cli/flags_test.go index 72a8a0c43cf2..ee7891cabf6b 100644 --- a/pkg/cli/flags_test.go +++ b/pkg/cli/flags_test.go @@ -714,6 +714,16 @@ func TestServerConnSettings(t *testing.T) { ":54321", "192.168.0.111:12345", ":54321", "192.168.0.111:12345", }, + {[]string{"start", "--listen-addr", "127.0.0.1", "--advertise-sql-addr", "192.168.0.111", "--port", "54321", "--advertise-port", "12345"}, + "127.0.0.1:54321", "127.0.0.1:12345", + "127.0.0.1:54321", "192.168.0.111:12345", + "127.0.0.1:54321", "192.168.0.111:12345", + }, + {[]string{"start", "--listen-addr", "127.0.0.1", "--advertise-sql-addr", "192.168.0.111:12345", "--port", "54321"}, + "127.0.0.1:54321", "127.0.0.1:54321", + "127.0.0.1:54321", "192.168.0.111:12345", + "127.0.0.1:54321", "192.168.0.111:12345", + }, } for i, td := range testData { diff --git a/pkg/server/start_listen.go b/pkg/server/start_listen.go index 6e0a21d7cdb2..a6072668b86a 100644 --- a/pkg/server/start_listen.go +++ b/pkg/server/start_listen.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/netutil" "github.com/cockroachdb/cockroach/pkg/util/stop" + "github.com/cockroachdb/errors" ) // startListenRPCAndSQL starts the RPC and SQL listeners. @@ -89,11 +90,14 @@ func startListenRPCAndSQL( pgL = m.Match(func(r io.Reader) bool { return pgwire.Match(r) }) - // Also if the pg port is not split, the actual listen - // and advertise addresses for SQL become equal to that - // of RPC, regardless of what was configured. + // Also if the pg port is not split, the actual listen address for + // SQL become equal to that of RPC. cfg.SQLAddr = cfg.Addr - cfg.SQLAdvertiseAddr = cfg.AdvertiseAddr + // Then we update the advertised addr with the right port, if + // the port had been auto-allocated. + if err := UpdateAddrs(ctx, &cfg.SQLAddr, &cfg.SQLAdvertiseAddr, ln.Addr()); err != nil { + return nil, nil, errors.Wrapf(err, "internal error") + } } anyL := m.Match(cmux.Any())