cluster: fix the issue when endpoints are empty in scale-out (#1227)

Author: 9547

Makefile

@@ -85,6 +85,7 @@ tidy:
 clean:
 	@rm -rf bin
 	@rm -rf cover
+	@rm -rf tests/*/{bin/*.test,logs,cover/*.out}
 
 test: failpoint-enable run-tests failpoint-disable
 

pkg/cluster/api/pdapi.go

@@ -87,6 +87,9 @@ var (
 )
 
 func tryURLs(endpoints []string, f func(endpoint string) ([]byte, error)) ([]byte, error) {
+	if len(endpoints) == 0 {
+		return nil, errors.New("no endpoint available")
+	}
 	var err error
 	var bytes []byte
 	for _, endpoint := range endpoints {

pkg/cluster/ctxt/context.go

@@ -28,6 +28,11 @@ const (
 	ctxKey = contextKey("TASK_CONTEXT")
 )
 
+const (
+	// CtxBaseTopo is key of store the base topology in context.Context
+	CtxBaseTopo = contextKey("BASE_TOPO")
+)
+
 type (
 	// Executor is the executor interface for TiUP, all tasks will in the end
 	// be passed to a executor and then be actually performed.

pkg/cluster/manager/scale_out.go

@@ -148,7 +148,9 @@ func (m *Manager) ScaleOut(
 		return err
 	}
 
-	if err := t.Execute(ctxt.New(context.Background())); err != nil {
+	ctx := ctxt.New(context.Background())
+	ctx = context.WithValue(ctx, ctxt.CtxBaseTopo, topo)
+	if err := t.Execute(ctx); err != nil {
 		if errorx.Cast(err) != nil {
 			// FIXME: Map possible task errors and give suggestions.
 			return err

pkg/cluster/operation/action.go

@@ -470,7 +470,7 @@ func StartComponent(ctx context.Context, instances []spec.Instance, options Opti
 		// of checkpoint context every time put it into a new goroutine.
 		nctx := checkpoint.NewContext(ctx)
 		errg.Go(func() error {
-			if err := ins.PrepareStart(tlsCfg); err != nil {
+			if err := ins.PrepareStart(nctx, tlsCfg); err != nil {
 				return err
 			}
 			err := startInstance(nctx, ins, options.OptTimeout)

pkg/cluster/spec/instance.go

@@ -84,7 +84,7 @@ type Instance interface {
 	Ready(context.Context, ctxt.Executor, uint64) error
 	InitConfig(ctx context.Context, e ctxt.Executor, clusterName string, clusterVersion string, deployUser string, paths meta.DirPaths) error
 	ScaleConfig(ctx context.Context, e ctxt.Executor, topo Topology, clusterName string, clusterVersion string, deployUser string, paths meta.DirPaths) error
-	PrepareStart(tlsCfg *tls.Config) error
+	PrepareStart(ctx context.Context, tlsCfg *tls.Config) error
 	ComponentName() string
 	InstanceName() string
 	ServiceName() string
@@ -398,7 +398,7 @@ func (i *BaseInstance) SetPatched(p bool) {
 }
 
 // PrepareStart checks instance requirements before starting
-func (i *BaseInstance) PrepareStart(tlsCfg *tls.Config) error {
+func (i *BaseInstance) PrepareStart(ctx context.Context, tlsCfg *tls.Config) error {
 	return nil
 }
 

pkg/cluster/spec/tiflash.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 	"time"
 
+	perrs "github.com/pingcap/errors"
 	"github.com/pingcap/tiup/pkg/cluster/api"
 	"github.com/pingcap/tiup/pkg/cluster/ctxt"
 	"github.com/pingcap/tiup/pkg/cluster/template/scripts"
@@ -526,7 +527,7 @@ func (i *TiFlashInstance) InitConfig(
 	}
 	tidbStatusStr := strings.Join(tidbStatusAddrs, ",")
 
-	pdStr := strings.Join(i.getEndpoints(), ",")
+	pdStr := strings.Join(i.getEndpoints(i.topo), ",")
 
 	cfg := scripts.NewTiFlashScript(
 		i.GetHost(),
@@ -652,25 +653,37 @@ type replicateConfig struct {
 	EnablePlacementRules string `json:"enable-placement-rules"`
 }
 
-func (i *TiFlashInstance) getEndpoints() []string {
+func (i *TiFlashInstance) getEndpoints(topo Topology) []string {
 	var endpoints []string
-	for _, pd := range i.topo.(*Specification).PDServers {
+	for _, pd := range topo.(*Specification).PDServers {
 		endpoints = append(endpoints, fmt.Sprintf("%s:%d", pd.Host, uint64(pd.ClientPort)))
 	}
 	return endpoints
 }
 
 // PrepareStart checks TiFlash requirements before starting
-func (i *TiFlashInstance) PrepareStart(tlsCfg *tls.Config) error {
-	endPoints := i.getEndpoints()
+func (i *TiFlashInstance) PrepareStart(ctx context.Context, tlsCfg *tls.Config) error {
 	// set enable-placement-rules to true via PDClient
-	pdClient := api.NewPDClient(endPoints, 10*time.Second, tlsCfg)
 	enablePlacementRules, err := json.Marshal(replicateConfig{
 		EnablePlacementRules: "true",
 	})
+	// this should not failed, else exit
 	if err != nil {
-		return nil
+		return perrs.Annotate(err, "failed to marshal replicate config")
+	}
+
+	var topo Topology
+	if topoVal := ctx.Value(ctxt.CtxBaseTopo); topoVal != nil { // in scale-out phase
+		var ok bool
+		topo, ok = topoVal.(Topology)
+		if !ok {
+			return perrs.New("base topology in context is invalid")
+		}
+	} else { // in start phase
+		topo = i.topo
 	}
 
+	endpoints := i.getEndpoints(topo)
+	pdClient := api.NewPDClient(endpoints, 10*time.Second, tlsCfg)
 	return pdClient.UpdateReplicateConfig(bytes.NewBuffer(enablePlacementRules))
 }

tests/tiup-cluster/script/scale_tools.sh

@@ -16,7 +16,7 @@ function scale_tools() {
     if [ $test_tls = true ]; then
         topo=./topo/full_tls.yaml
     else
-        topo=./topo/full.yaml
+        topo=./topo/full_without_tiflash.yaml
     fi
 
     tiup-cluster $client --yes deploy $name $version $topo -i ~/.ssh/id_rsa
@@ -29,9 +29,6 @@ function scale_tools() {
     for item in pump drainer tidb tikv pd grafana node_exporter blackbox_exporter; do
         tiup-cluster $client exec $name -N n1 --command "grep $item /home/tidb/deploy/prometheus-9090/conf/prometheus.yml"
     done
-    if [ $test_tls = false ]; then
-        tiup-cluster $client exec $name -N n1 --command "grep tiflash /home/tidb/deploy/prometheus-9090/conf/prometheus.yml"
-    fi
 
     tiup-cluster $client list | grep "$name"
 
@@ -46,9 +43,9 @@ function scale_tools() {
         total=19
         total_add_one=20
     else
-        total_sub_one=21
-        total=22
-        total_add_one=23
+        total_sub_one=20
+        total=21
+        total_add_one=22
     fi
 
     echo "start scale in pump"
@@ -95,6 +92,13 @@ function scale_tools() {
     # currently tiflash is not supported in TLS enabled cluster
     # and only Tiflash support data-dir in multipath
     if [ $test_tls = false ]; then
+        echo "start scale out tiflash(first time)"
+        topo=./topo/full_scale_in_tiflash.yaml
+        tiup-cluster $client --yes scale-out $name $topo
+        tiup-cluster $client exec $name -N n1 --command "grep tiflash /home/tidb/deploy/prometheus-9090/conf/prometheus.yml"
+        # ensure scale-out will mark pd.enable-placement-rules to true. ref https://github.com/pingcap/tiup/issues/1226
+        curl n3:2379/pd/api/v1/config 2>/dev/null | grep '"enable-placement-rules": "true"'
+
         # ensure tiflash's data dir exists
         tiup-cluster $client exec $name -N n3 --command "ls /home/tidb/deploy/tiflash-9000/data1"
         tiup-cluster $client exec $name -N n3 --command "ls /data/tiflash-data"
@@ -103,10 +107,10 @@ function scale_tools() {
         tiup-cluster $client display $name | grep Tombstone
         echo "start prune tiflash"
         yes | tiup-cluster $client prune $name
-        wait_instance_num_reach $name $total_sub_one $native_ssh
+        wait_instance_num_reach $name $total $native_ssh
         ! tiup-cluster $client exec $name -N n3 --command "ls /home/tidb/deploy/tiflash-9000/data1"
         ! tiup-cluster $client exec $name -N n3 --command "ls /data/tiflash-data"
-        echo "start scale out tiflash"
+        echo "start scale out tiflash(second time)"
         topo=./topo/full_scale_in_tiflash.yaml
         tiup-cluster $client --yes scale-out $name $topo
     fi

tests/tiup-cluster/topo/full_scale_in_tiflash.yaml

@@ -1,2 +1,3 @@
 tiflash_servers:
   - host: n3
+    data_dir: "data1,/data/tiflash-data"

tests/tiup-cluster/topo/full_without_tiflash.yaml

@@ -0,0 +1,63 @@
+global:
+  user: tidb
+  group: pingcap
+
+server_configs:
+  tidb:
+    binlog.enable: true
+    binlog.ignore-error: false
+  tikv:
+    storage.reserve-space: 1K
+  pump:
+    storage.stop-write-at-available-space: 1 mib
+
+tidb_servers:
+  - host: n1
+  - host: n2
+
+pd_servers:
+  - host: n3
+  - host: n4
+  - host: n5
+
+# Note if only 3 instance, when scale-in one of it.
+# It may not be tombstone.
+tikv_servers:
+  - host: n1
+  - host: n3
+    data_dir: "/home/tidb/my_kv_data"
+  - host: n4
+  - host: n5
+
+pump_servers:
+  - host: n3
+  - host: n4
+  - host: n5
+
+drainer_servers:
+  - host: n1
+    data_dir: /home/tidb/data/drainer-8249/data
+    commit_ts: -1
+    config:
+      syncer.db-type: "file"
+
+cdc_servers:
+  - host: n3
+  - host: n4
+  - host: n5
+
+tispark_masters:
+  - host: n3
+
+tispark_workers:
+  - host: n4
+
+monitoring_servers:
+  - host: n1
+    rule_dir: /tmp/local/prometheus
+grafana_servers:
+  - host: n1
+    dashboard_dir: /tmp/local/grafana
+alertmanager_servers:
+  - host: n1
+    config_file: /tmp/local/alertmanager/alertmanager.yml