Skip to content

Commit

Permalink
address comment
Browse files Browse the repository at this point in the history
Signed-off-by: Cabinfever_B <[email protected]>
  • Loading branch information
CabinfeverB committed Mar 8, 2024
1 parent 52ad50d commit 0c5fec7
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 23 deletions.
21 changes: 10 additions & 11 deletions client/pd_service_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ type ServiceDiscovery interface {

// ServiceClient is an interface that defines a set of operations for a raw PD gRPC client to specific PD server.
type ServiceClient interface {
// GetAddress returns the address with HTTP scheme of the PD server.
// GetAddress returns the address with scheme of the PD server.
GetAddress() string
// GetClientConn returns the gRPC connection of the service client
GetClientConn() *grpc.ClientConn
Expand All @@ -147,18 +147,17 @@ var (
)

type pdServiceClient struct {
addr string
httpAddress string
conn *grpc.ClientConn
isLeader bool
leaderAddr string
addr string
conn *grpc.ClientConn
isLeader bool
leaderAddr string

networkFailure atomic.Bool
}

// NOTE: In the current implementation, the address passed in is bound to have an http scheme,
// because it is processed in `newPDServiceDiscovery`, and the url returned by etcd member is its own.
// When testing, the address is also bound to have an http scheme.
// NOTE: In the current implementation, the address passed in is bound to have a scheme,
// because it is processed in `newPDServiceDiscovery`, and the url returned by etcd member owns the sheme.
// When testing, the address is also bound to have a scheme.
func newPDServiceClient(addr, leaderAddr string, conn *grpc.ClientConn, isLeader bool) ServiceClient {
cli := &pdServiceClient{
addr: addr,
Expand Down Expand Up @@ -1125,12 +1124,12 @@ func addrsToUrls(addrs []string, tlsCfg *tls.Config) []string {
// Add default schema "http://" to addrs.
urls := make([]string, 0, len(addrs))
for _, addr := range addrs {
urls = append(urls, addrToUrl(addr, tlsCfg))
urls = append(urls, addrToURL(addr, tlsCfg))
}
return urls
}

func addrToUrl(addr string, tlsCfg *tls.Config) string {
func addrToURL(addr string, tlsCfg *tls.Config) string {
if tlsCfg == nil {
if strings.HasPrefix(addr, httpsScheme) {
addr = fmt.Sprintf("%s%s", httpScheme, strings.TrimPrefix(addr, httpsScheme))
Expand Down
31 changes: 19 additions & 12 deletions client/pd_service_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"crypto/tls"
"errors"
"fmt"
"log"
"net"
"net/url"
Expand Down Expand Up @@ -141,12 +142,12 @@ func (suite *serviceClientTestSuite) SetupSuite() {
followerConn, err2 := grpc.Dial(suite.followerServer.addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
if err1 == nil && err2 == nil {
suite.followerClient = newPDServiceClient(
addrToUrl(suite.followerServer.addr, nil),
addrToUrl(suite.leaderServer.addr, nil),
addrToURL(suite.followerServer.addr, nil),
addrToURL(suite.leaderServer.addr, nil),
followerConn, false)
suite.leaderClient = newPDServiceClient(
addrToUrl(suite.leaderServer.addr, nil),
addrToUrl(suite.leaderServer.addr, nil),
addrToURL(suite.leaderServer.addr, nil),
addrToURL(suite.leaderServer.addr, nil),
leaderConn, true)
suite.followerServer.server.leaderConn = suite.leaderClient.GetClientConn()
suite.followerServer.server.leaderAddr = suite.leaderClient.GetAddress()
Expand All @@ -172,8 +173,8 @@ func (suite *serviceClientTestSuite) TearDownSuite() {

func (suite *serviceClientTestSuite) TestServiceClient() {
re := suite.Require()
leaderAddress := addrToUrl(suite.leaderServer.addr, nil)
followerAddress := addrToUrl(suite.followerServer.addr, nil)
leaderAddress := addrToURL(suite.leaderServer.addr, nil)
followerAddress := addrToURL(suite.followerServer.addr, nil)

follower := suite.followerClient
leader := suite.leaderClient
Expand Down Expand Up @@ -307,16 +308,22 @@ func (suite *serviceClientTestSuite) TestServiceClientBalancer() {

func TestHTTPScheme(t *testing.T) {
re := require.New(t)
cli := newPDServiceClient(addrToUrl("127.0.0.1:2379", nil), addrToUrl("127.0.0.1:2379", nil), nil, false)
cli := newPDServiceClient(addrToURL("127.0.0.1:2379", nil), addrToURL("127.0.0.1:2379", nil), nil, false)
re.Equal("http://127.0.0.1:2379", cli.GetAddress())
cli = newPDServiceClient(addrToUrl("https://127.0.0.1:2379", nil), addrToUrl("127.0.0.1:2379", nil), nil, false)
cli = newPDServiceClient(addrToURL("https://127.0.0.1:2379", nil), addrToURL("127.0.0.1:2379", nil), nil, false)
re.Equal("http://127.0.0.1:2379", cli.GetAddress())
cli = newPDServiceClient(addrToUrl("http://127.0.0.1:2379", nil), addrToUrl("127.0.0.1:2379", nil), nil, false)
cli = newPDServiceClient(addrToURL("http://127.0.0.1:2379", nil), addrToURL("127.0.0.1:2379", nil), nil, false)
re.Equal("http://127.0.0.1:2379", cli.GetAddress())
cli = newPDServiceClient(addrToUrl("127.0.0.1:2379", &tls.Config{}), addrToUrl("127.0.0.1:2379", &tls.Config{}), nil, false)
cli = newPDServiceClient(addrToURL("127.0.0.1:2379", &tls.Config{}), addrToURL("127.0.0.1:2379", &tls.Config{}), nil, false)
re.Equal("https://127.0.0.1:2379", cli.GetAddress())
cli = newPDServiceClient(addrToUrl("https://127.0.0.1:2379", &tls.Config{}), addrToUrl("127.0.0.1:2379", &tls.Config{}), nil, false)
cli = newPDServiceClient(addrToURL("https://127.0.0.1:2379", &tls.Config{}), addrToURL("127.0.0.1:2379", &tls.Config{}), nil, false)
re.Equal("https://127.0.0.1:2379", cli.GetAddress())
cli = newPDServiceClient(addrToUrl("http://127.0.0.1:2379", &tls.Config{}), addrToUrl("127.0.0.1:2379", &tls.Config{}), nil, false)
cli = newPDServiceClient(addrToURL("http://127.0.0.1:2379", &tls.Config{}), addrToURL("127.0.0.1:2379", &tls.Config{}), nil, false)
re.Equal("https://127.0.0.1:2379", cli.GetAddress())
}

func TestHTTP222e(t *testing.T) {
_, err := url.Parse("sts://127.0.0.1:2379")
fmt.Println(err)
require.NoError(t, err)
}

0 comments on commit 0c5fec7

Please sign in to comment.