cli,server: fix --sql-advertise-addr when --sql-addr is not speci…

…fied

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

@@ -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

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 {

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())