[bug fix] Take into account PD peer port and PD client port when pdAd…

…dresses from TidbCluster manifest used over Discovery service in pd, tidb and tikv startup scripts (#5438) (#5444) Co-authored-by: Sergey <[email protected]>
pingcap · Dec 6, 2023 · 01cc731 · 01cc731
1 parent cddb804
commit 01cc731
Show file tree

Hide file tree

Showing 8 changed files with 42 additions and 12 deletions.
diff --git a/pkg/manager/member/startscript/v2/common.go b/pkg/manager/member/startscript/v2/common.go
@@ -15,6 +15,8 @@ package v2
 
 import (
 	"bytes"
+	"fmt"
+	"net/url"
 	"text/template"
 )
 
@@ -118,3 +120,19 @@ func renderTemplateFunc(tpl *template.Template, model interface{}) (string, erro
 	}
 	return buff.String(), nil
 }
+
+func addressesWithSchemeAndPort(addresses []string, scheme string, port int32) []string {
+	res := make([]string, len(addresses))
+	for i, a := range addresses {
+		u, err := url.Parse(a)
+		if err != nil {
+			res[i] = fmt.Sprintf("%s%s:%d", scheme, a, port)
+		} else if u.Hostname() != "" {
+			res[i] = fmt.Sprintf("%s%s:%d", scheme, u.Hostname(), port)
+		} else {
+			res[i] = fmt.Sprintf("%s%s:%d", scheme, u.Path, port)
+		}
+
+	}
+	return res
+}
diff --git a/pkg/manager/member/startscript/v2/common_test.go b/pkg/manager/member/startscript/v2/common_test.go
@@ -61,3 +61,12 @@ func validateScript(script string) error {
 	_, err := syntax.NewParser().Parse(strings.NewReader(script), "")
 	return err
 }
+
+func TestAddressesWithSchemeAndPort(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+
+	result := addressesWithSchemeAndPort([]string{"example.com", "http://test.com:7777", "https://blah.com"}, "https://", 8080)
+
+	expected := []string{"https://example.com:8080", "https://test.com:8080", "https://blah.com:8080"}
+	g.Expect(result).Should(gomega.Equal(expected))
+}
diff --git a/pkg/manager/member/startscript/v2/pd_start_script.go b/pkg/manager/member/startscript/v2/pd_start_script.go
@@ -60,7 +60,8 @@ func RenderPDStartScript(tc *v1alpha1.TidbCluster) (string, error) {
 	preferPDAddressesOverDiscovery := slices.Contains(
 		tc.Spec.StartScriptV2FeatureFlags, v1alpha1.StartScriptV2FeatureFlagPreferPDAddressesOverDiscovery)
 	if preferPDAddressesOverDiscovery {
-		m.PDAddresses = strings.Join(tc.Spec.PDAddresses, ",")
+		pdAddressesWithSchemeAndPort := addressesWithSchemeAndPort(tc.Spec.PDAddresses, tc.Scheme()+"://", v1alpha1.DefaultPDPeerPort)
+		m.PDAddresses = strings.Join(pdAddressesWithSchemeAndPort, ",")
 	}
 
 	m.DataDir = filepath.Join(constants.PDDataVolumeMountPath, tc.Spec.PD.DataSubDir)

diff --git a/pkg/manager/member/startscript/v2/pd_start_script_test.go b/pkg/manager/member/startscript/v2/pd_start_script_test.go
@@ -117,7 +117,7 @@ exec /pd-server ${ARGS}
 		{
 			name: "with PDAddresses but without preferPDAddressesOverDiscovery",
 			modifyTC: func(tc *v1alpha1.TidbCluster) {
-				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}:2380", "another.pd:2380"}
+				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}", "another.pd"}
 			},
 			expectScript: `#!/bin/sh
 
@@ -201,7 +201,7 @@ exec /pd-server ${ARGS}
 		{
 			name: "with PDAddresses and preferPDAddressesOverDiscovery",
 			modifyTC: func(tc *v1alpha1.TidbCluster) {
-				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}:2380", "another.pd:2380"}
+				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}", "another.pd"}
 				tc.Spec.StartScriptV2FeatureFlags = []v1alpha1.StartScriptV2FeatureFlag{
 					v1alpha1.StartScriptV2FeatureFlagPreferPDAddressesOverDiscovery,
 				}
@@ -265,7 +265,7 @@ ARGS="--data-dir=/var/lib/pd \
 --advertise-client-urls=http://${PD_DOMAIN}:2379 \
 --config=/etc/pd/pd.toml"
 
-ARGS="${ARGS} --join=${PD_DOMAIN}:2380,another.pd:2380"
+ARGS="${ARGS} --join=http://${PD_DOMAIN}:2380,http://another.pd:2380"
 
 echo "starting pd-server ..."
 sleep $((RANDOM % 10))

diff --git a/pkg/manager/member/startscript/v2/tidb_start_script.go b/pkg/manager/member/startscript/v2/tidb_start_script.go
@@ -42,7 +42,8 @@ func RenderTiDBStartScript(tc *v1alpha1.TidbCluster) (string, error) {
 	preferPDAddressesOverDiscovery := slices.Contains(
 		tc.Spec.StartScriptV2FeatureFlags, v1alpha1.StartScriptV2FeatureFlagPreferPDAddressesOverDiscovery)
 	if preferPDAddressesOverDiscovery {
-		m.PDAddresses = strings.Join(tc.Spec.PDAddresses, ",")
+		pdAddressesWithSchemeAndPort := addressesWithSchemeAndPort(tc.Spec.PDAddresses, "", v1alpha1.DefaultPDClientPort)
+		m.PDAddresses = strings.Join(pdAddressesWithSchemeAndPort, ",")
 	}
 	if len(m.PDAddresses) == 0 {
 		if tc.AcrossK8s() {

diff --git a/pkg/manager/member/startscript/v2/tidb_start_script_test.go b/pkg/manager/member/startscript/v2/tidb_start_script_test.go
@@ -77,7 +77,7 @@ exec /tidb-server ${ARGS}
 		{
 			name: "with PDAddresses but without PreferPDAddressesOverDiscovery",
 			modifyTC: func(tc *v1alpha1.TidbCluster) {
-				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}:2380", "another.pd:2380"}
+				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}", "another.pd"}
 			},
 			expectScript: `#!/bin/sh
 
@@ -120,7 +120,7 @@ exec /tidb-server ${ARGS}
 		{
 			name: "with PDAddresses and PreferPDAddressesOverDiscovery",
 			modifyTC: func(tc *v1alpha1.TidbCluster) {
-				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}:2380", "another.pd:2380"}
+				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}", "another.pd"}
 				tc.Spec.StartScriptV2FeatureFlags = []v1alpha1.StartScriptV2FeatureFlag{
 					v1alpha1.StartScriptV2FeatureFlagPreferPDAddressesOverDiscovery,
 				}
@@ -149,7 +149,7 @@ TIDB_POD_NAME=${POD_NAME:-$HOSTNAME}
 ARGS="--store=tikv \
 --advertise-address=${TIDB_POD_NAME}.start-script-test-tidb-peer.start-script-test-ns.svc \
 --host=0.0.0.0 \
---path=${PD_DOMAIN}:2380,another.pd:2380 \
+--path=${PD_DOMAIN}:2379,another.pd:2379 \
 --config=/etc/tidb/tidb.toml"
 
 SLOW_LOG_FILE=${SLOW_LOG_FILE:-""}

diff --git a/pkg/manager/member/startscript/v2/tikv_start_script.go b/pkg/manager/member/startscript/v2/tikv_start_script.go
@@ -50,7 +50,8 @@ func RenderTiKVStartScript(tc *v1alpha1.TidbCluster) (string, error) {
 	preferPDAddressesOverDiscovery := slices.Contains(
 		tc.Spec.StartScriptV2FeatureFlags, v1alpha1.StartScriptV2FeatureFlagPreferPDAddressesOverDiscovery)
 	if preferPDAddressesOverDiscovery {
-		m.PDAddresses = strings.Join(tc.Spec.PDAddresses, ",")
+		pdAddressesWithSchemeAndPort := addressesWithSchemeAndPort(tc.Spec.PDAddresses, "", v1alpha1.DefaultPDClientPort)
+		m.PDAddresses = strings.Join(pdAddressesWithSchemeAndPort, ",")
 	}
 	if len(m.PDAddresses) == 0 {
 		if tc.AcrossK8s() {

diff --git a/pkg/manager/member/startscript/v2/tikv_start_script_test.go b/pkg/manager/member/startscript/v2/tikv_start_script_test.go
@@ -78,7 +78,7 @@ exec /tikv-server ${ARGS}
 		{
 			name: "with PDAddresses but without PDAddressesOverDiscovery",
 			modifyTC: func(tc *v1alpha1.TidbCluster) {
-				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}:2380", "another.pd:2380"}
+				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}", "another.pd"}
 			},
 			expectScript: `#!/bin/sh
 
@@ -122,7 +122,7 @@ exec /tikv-server ${ARGS}
 		{
 			name: "with PDAddresses and PDAddressesOverDiscovery",
 			modifyTC: func(tc *v1alpha1.TidbCluster) {
-				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}:2380", "another.pd:2380"}
+				tc.Spec.PDAddresses = []string{"${PD_DOMAIN}", "another.pd"}
 				tc.Spec.StartScriptV2FeatureFlags = []v1alpha1.StartScriptV2FeatureFlag{
 					v1alpha1.StartScriptV2FeatureFlagPreferPDAddressesOverDiscovery,
 				}
@@ -148,7 +148,7 @@ fi
 
 TIKV_POD_NAME=${POD_NAME:-$HOSTNAME}
 
-ARGS="--pd=${PD_DOMAIN}:2380,another.pd:2380 \
+ARGS="--pd=${PD_DOMAIN}:2379,another.pd:2379 \
 --advertise-addr=${TIKV_POD_NAME}.start-script-test-tikv-peer.start-script-test-ns.svc:20160 \
 --addr=0.0.0.0:20160 \
 --status-addr=0.0.0.0:20180 \