manager: get unicast ip only when the config one is not (#386)

Signed-off-by: xhe <xw897002528@gmail.com>
pingcap · Oct 20, 2023 · 28d53d3 · 28d53d3
1 parent 3354c7a
commit 28d53d3
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 3 deletions.
diff --git a/lib/util/sys/sys.go b/lib/util/sys/sys.go
@@ -5,7 +5,7 @@ package sys
 
 import "net"
 
-func GetLocalIP() string {
+func GetGlobalUnicastIP() string {
 	addrs, err := net.InterfaceAddrs()
 	if err == nil {
 		for _, address := range addrs {

diff --git a/pkg/manager/infosync/info.go b/pkg/manager/infosync/info.go
@@ -160,15 +160,21 @@ func (is *InfoSyncer) getTopologyInfo(cfg *config.Config) (*TopologyInfo, error)
 		s = ""
 	}
 	dir := path.Dir(s)
-	ip := sys.GetLocalIP()
-	_, port, err := net.SplitHostPort(cfg.Proxy.Addr)
+	ip, port, err := net.SplitHostPort(cfg.Proxy.Addr)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
 	_, statusPort, err := net.SplitHostPort(cfg.API.Addr)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
+	// reporting a non unicast IP makes no sense, try to find one
+	// loopback/linklocal-unicast are not global unicast IP, but are valid local unicast IP
+	if pip := net.ParseIP(ip); ip == "" || pip.Equal(net.IPv4bcast) || pip.IsUnspecified() || pip.IsMulticast() {
+		if v := sys.GetGlobalUnicastIP(); v != "" {
+			ip = v
+		}
+	}
 	return &TopologyInfo{
 		Version:        versioninfo.TiProxyVersion,
 		GitHash:        versioninfo.TiProxyGitHash,

diff --git a/pkg/manager/infosync/info_test.go b/pkg/manager/infosync/info_test.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net"
 	"net/url"
 	"path"
 	"strings"
@@ -16,6 +17,7 @@ import (
 	tidbinfo "github.com/pingcap/tidb/domain/infosync"
 	"github.com/pingcap/tiproxy/lib/config"
 	"github.com/pingcap/tiproxy/lib/util/logger"
+	"github.com/pingcap/tiproxy/lib/util/sys"
 	"github.com/pingcap/tiproxy/lib/util/waitgroup"
 	"github.com/pingcap/tiproxy/pkg/manager/cert"
 	"github.com/stretchr/testify/require"
@@ -178,6 +180,43 @@ func TestFetchTiDBTopology(t *testing.T) {
 	}
 }
 
+func TestGetTopology(t *testing.T) {
+	ts := newEtcdTestSuite(t)
+	t.Cleanup(ts.close)
+	for _, cas := range []struct {
+		ip          string
+		port        string
+		non_unicast bool
+	}{
+		{":34", "34", true},
+		{"0.0.0.0:34", "34", true},
+		{"255.255.255.255:34", "34", true},
+		{"239.255.255.255:34", "34", true},
+		{"[FF02::1:FF47]:34", "34", true},
+		{"127.0.0.1:34", "34", false},
+		{"[F02::1:FF47]:34", "34", false},
+		{"192.0.0.1:6049", "6049", false},
+	} {
+		is, err := ts.is.getTopologyInfo(&config.Config{
+			Proxy: config.ProxyServer{
+				Addr: cas.ip,
+			},
+			API: config.API{
+				Addr: cas.ip,
+			},
+		})
+		require.NoError(t, err)
+		ip, _, err := net.SplitHostPort(cas.ip)
+		require.NoError(t, err)
+		if cas.non_unicast {
+			ip = sys.GetGlobalUnicastIP()
+		}
+		require.Equal(t, ip, is.IP)
+		require.Equal(t, cas.port, is.Port)
+		require.Equal(t, cas.port, is.StatusPort)
+	}
+}
+
 // Test that fetching retries when etcd server is down until the server is up again.
 func TestEtcdServerDown4Fetch(t *testing.T) {
 	ts := newEtcdTestSuite(t)