From 508ba5e6b45fc47b60fd170d6c3afbe8155535a5 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Tue, 17 Nov 2020 05:33:04 +0800
Subject: [PATCH 01/13] feat(cluster): delete public key if node was
 destroyed/pruned

---
 components/cluster/command/prune.go |  4 +-
 pkg/cluster/executor/executor.go    | 30 ++++++++++
 pkg/cluster/manager.go              |  4 +-
 pkg/cluster/operation/destroy.go    | 90 +++++++++++++++++++++++------
 pkg/cluster/operation/scale_in.go   |  8 ++-
 pkg/cluster/task/action.go          |  6 +-
 pkg/cluster/task/env_init.go        | 28 +--------
 7 files changed, 118 insertions(+), 52 deletions(-)

diff --git a/components/cluster/command/prune.go b/components/cluster/command/prune.go
index 3faca3fe6f..c1635f02b0 100644
--- a/components/cluster/command/prune.go
+++ b/components/cluster/command/prune.go
@@ -73,7 +73,7 @@ func destroyTombstoneIfNeed(clusterName string, metadata *spec.ClusterMeta, opt
 		return perrs.AddStack(err)
 	}
 
-	nodes, err := operator.DestroyTombstone(ctx, topo, true /* returnNodesOnly */, opt, tlsCfg)
+	nodes, err := operator.DestroyTombstone(ctx, topo, true /* returnNodesOnly */, opt, tlsCfg, ctx.PublicKeyPath)
 	if err != nil {
 		return perrs.AddStack(err)
 	}
@@ -93,7 +93,7 @@ func destroyTombstoneIfNeed(clusterName string, metadata *spec.ClusterMeta, opt
 
 	log.Infof("Start destroy Tombstone nodes: %v ...", nodes)
 
-	_, err = operator.DestroyTombstone(ctx, topo, false /* returnNodesOnly */, opt, tlsCfg)
+	_, err = operator.DestroyTombstone(ctx, topo, false /* returnNodesOnly */, opt, tlsCfg, ctx.PublicKeyPath)
 	if err != nil {
 		return perrs.AddStack(err)
 	}
diff --git a/pkg/cluster/executor/executor.go b/pkg/cluster/executor/executor.go
index 4815afa58f..9042767a1f 100644
--- a/pkg/cluster/executor/executor.go
+++ b/pkg/cluster/executor/executor.go
@@ -47,6 +47,9 @@ var (
 	// It's used to predict if the connection can establish success in the future.
 	// Its main purpose is to avoid sshpass hang when user speficied a wrong prompt.
 	connectionTestCommand = "echo connection test, if killed, check the password prompt"
+
+	// SSH authorized_keys file
+	defaultSSHAuthorizedKeys = "~/.ssh/authorized_keys"
 )
 
 // Executor is the executor interface for TiOps, all tasks will in the end
@@ -155,3 +158,30 @@ func checkLocalIP(ip string) error {
 
 	return fmt.Errorf("address %s not found in all interfaces, found ips: %s", ip, strings.Join(foundIps, ","))
 }
+
+// FindSSHAuthorizedKeysFile finds the correct path of SSH authorized keys file
+func FindSSHAuthorizedKeysFile(exec Executor) string {
+	// detect if custom path of authorized keys file is set
+	// NOTE: we do not yet support:
+	//   - custom config for user (~/.ssh/config)
+	//   - sshd started with custom config (other than /etc/ssh/sshd_config)
+	//   - ssh server implementations other than OpenSSH (such as dropbear)
+	sshAuthorizedKeys := defaultSSHAuthorizedKeys
+	cmd := "grep -Ev '^\\s*#|^\\s*$' /etc/ssh/sshd_config"
+	stdout, _, _ := exec.Execute(cmd, true) // error ignored as we have default value
+	for _, line := range strings.Split(string(stdout), "\n") {
+		if !strings.Contains(line, "AuthorizedKeysFile") {
+			continue
+		}
+		fields := strings.Fields(line)
+		if len(fields) >= 2 {
+			sshAuthorizedKeys = fields[1]
+			break
+		}
+	}
+
+	if !strings.HasPrefix(sshAuthorizedKeys, "/") && !strings.HasPrefix(sshAuthorizedKeys, "~") {
+		sshAuthorizedKeys = fmt.Sprintf("~/%s", sshAuthorizedKeys)
+	}
+	return sshAuthorizedKeys
+}
diff --git a/pkg/cluster/manager.go b/pkg/cluster/manager.go
index ad661556b7..65e2764356 100644
--- a/pkg/cluster/manager.go
+++ b/pkg/cluster/manager.go
@@ -384,7 +384,7 @@ func (m *Manager) DestroyCluster(clusterName string, gOpt operator.Options, dest
 			}, tlsCfg)
 		}).
 		Func("DestroyCluster", func(ctx *task.Context) error {
-			return operator.Destroy(ctx, topo, destroyOpt)
+			return operator.Destroy(ctx, topo, ctx.PublicKeyPath, destroyOpt)
 		}).
 		Build()
 
@@ -612,7 +612,7 @@ func (m *Manager) Display(clusterName string, opt operator.Options) error {
 		}
 
 		// Check if there is some instance in tombstone state
-		nodes, _ := operator.DestroyTombstone(ctx, t, true /* returnNodesOnly */, opt, tlsCfg)
+		nodes, _ := operator.DestroyTombstone(ctx, t, true /* returnNodesOnly */, opt, tlsCfg, ctx.PublicKeyPath)
 		if len(nodes) != 0 {
 			color.Green("There are some nodes can be pruned: \n\tNodes: %+v\n\tYou can destroy them with the command: `tiup cluster prune %s`", nodes, clusterName)
 		}
diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go
index 0387b86df3..7d67f7e6f2 100644
--- a/pkg/cluster/operation/destroy.go
+++ b/pkg/cluster/operation/destroy.go
@@ -16,6 +16,7 @@ package operator
 import (
 	"crypto/tls"
 	"fmt"
+	"io/ioutil"
 	"path"
 	"path/filepath"
 	"strconv"
@@ -23,7 +24,9 @@ import (
 	"time"
 
 	"github.com/pingcap/errors"
+	perrs "github.com/pingcap/errors"
 	"github.com/pingcap/tiup/pkg/cluster/api"
+	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/module"
 	"github.com/pingcap/tiup/pkg/cluster/spec"
 	"github.com/pingcap/tiup/pkg/logger/log"
@@ -52,14 +55,15 @@ func Cleanup(
 func Destroy(
 	getter ExecutorGetter,
 	cluster spec.Topology,
+	publicKeyPath string,
 	options Options,
 ) error {
-	uniqueHosts := set.NewStringSet()
 	coms := cluster.ComponentsByStopOrder()
 
 	instCount := map[string]int{}
 	cluster.IterInstance(func(inst spec.Instance) {
-		instCount[inst.GetHost()] = instCount[inst.GetHost()] + 1
+		host := inst.GetHost()
+		instCount[host] = instCount[host] + 1
 	})
 
 	for _, com := range coms {
@@ -80,9 +84,15 @@ func Destroy(
 		}
 	}
 
+	gOpts := cluster.BaseTopo().GlobalOptions
+
 	// Delete all global deploy directory
-	for host := range uniqueHosts {
-		if err := DeleteGlobalDirs(getter, host, cluster.BaseTopo().GlobalOptions); err != nil {
+	for host := range instCount {
+		if err := DeleteGlobalDirs(getter, host, gOpts); err != nil {
+			return nil
+		}
+
+		if err := DeletePublicKey(getter, host, publicKeyPath); err != nil {
 			return nil
 		}
 	}
@@ -93,7 +103,7 @@ func Destroy(
 // StopAndDestroyInstance stop and destroy the instance,
 // if this instance is the host's last one, and the host has monitor deployed,
 // we need to destroy the monitor, either
-func StopAndDestroyInstance(getter ExecutorGetter, cluster spec.Topology, instance spec.Instance, options Options, destroyMonitor bool) error {
+func StopAndDestroyInstance(getter ExecutorGetter, cluster spec.Topology, instance spec.Instance, options Options, destroyNode bool, publicKeyPath string) error {
 	ignoreErr := options.Force
 	compName := instance.ComponentName()
 
@@ -111,22 +121,32 @@ func StopAndDestroyInstance(getter ExecutorGetter, cluster spec.Topology, instan
 		log.Warnf("failed to destroy %s: %v", compName, err)
 	}
 
-	// monitoredOptions for dm cluster is nil
-	monitoredOptions := cluster.GetMonitoredOptions()
+	if destroyNode {
+		// monitoredOptions for dm cluster is nil
+		monitoredOptions := cluster.GetMonitoredOptions()
 
-	if destroyMonitor && monitoredOptions != nil {
-		if err := StopMonitored(getter, instance, monitoredOptions, options.OptTimeout); err != nil {
-			if !ignoreErr {
-				return errors.Annotatef(err, "failed to stop monitor")
+		if monitoredOptions != nil {
+			if err := StopMonitored(getter, instance, monitoredOptions, options.OptTimeout); err != nil {
+				if !ignoreErr {
+					return errors.Annotatef(err, "failed to stop monitor")
+				}
+				log.Warnf("failed to stop %s: %v", "monitor", err)
+			}
+			if err := DestroyMonitored(getter, instance, monitoredOptions, options.OptTimeout); err != nil {
+				if !ignoreErr {
+					return errors.Annotatef(err, "failed to destroy monitor")
+				}
+				log.Warnf("failed to destroy %s: %v", "monitor", err)
 			}
-			log.Warnf("failed to stop %s: %v", "monitor", err)
 		}
-		if err := DestroyMonitored(getter, instance, monitoredOptions, options.OptTimeout); err != nil {
+
+		if err := DeletePublicKey(getter, instance.GetHost(), publicKeyPath); err != nil {
 			if !ignoreErr {
-				return errors.Annotatef(err, "failed to destroy monitor")
+				return errors.Annotatef(err, "failed to delete public key")
 			}
-			log.Warnf("failed to destroy %s: %v", "monitor", err)
+			log.Warnf("failed to delete public key")
 		}
+
 	}
 	return nil
 }
@@ -171,6 +191,40 @@ func DeleteGlobalDirs(getter ExecutorGetter, host string, options *spec.GlobalOp
 	return nil
 }
 
+// DeletePublicKey deletes the SSH public key from host
+func DeletePublicKey(getter ExecutorGetter, host, pubKeyPath string) error {
+	e := getter.Get(host)
+	log.Infof("Delete public key %s", host)
+	publicKey, err := ioutil.ReadFile(pubKeyPath)
+	if err != nil {
+		return perrs.Trace(err)
+	}
+	pubKey := strings.ReplaceAll(string(publicKey), "/", "\\/")
+	pubKeysFile := executor.FindSSHAuthorizedKeysFile(e)
+
+	c := module.ShellModuleConfig{
+		Command:  fmt.Sprintf("sed -i '/%s/d' %s", pubKey, pubKeysFile),
+		Chdir:    "",
+		UseShell: true,
+	}
+	shell := module.NewShellModule(c)
+	stdout, stderr, err := shell.Execute(e)
+
+	if len(stdout) > 0 {
+		fmt.Println(string(stdout))
+	}
+	if len(stderr) > 0 {
+		log.Errorf(string(stderr))
+	}
+
+	if err != nil {
+		return errors.Annotatef(err, "failed to delete pulblic key on: %s", host)
+	}
+
+	log.Infof("Delete public key %s success", host)
+	return nil
+}
+
 // DestroyMonitored destroy the monitored service.
 func DestroyMonitored(getter ExecutorGetter, inst spec.Instance, options *spec.MonitoredOptions, timeout uint64) error {
 	e := getter.Get(inst.GetHost())
@@ -417,8 +471,9 @@ func DestroyTombstone(
 	returNodesOnly bool,
 	options Options,
 	tlsCfg *tls.Config,
+	publicKey string,
 ) (nodes []string, err error) {
-	return DestroyClusterTombstone(getter, cluster, returNodesOnly, options, tlsCfg)
+	return DestroyClusterTombstone(getter, cluster, returNodesOnly, options, tlsCfg, publicKey)
 }
 
 // DestroyClusterTombstone remove the tombstone node in spec and destroy them.
@@ -429,6 +484,7 @@ func DestroyClusterTombstone(
 	returNodesOnly bool,
 	options Options,
 	tlsCfg *tls.Config,
+	publicKey string,
 ) (nodes []string, err error) {
 	instCount := map[string]int{}
 	for _, component := range cluster.ComponentsByStartOrder() {
@@ -458,7 +514,7 @@ func DestroyClusterTombstone(
 
 		for _, instance := range instances {
 			instCount[instance.GetHost()]--
-			err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0)
+			err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0, publicKey)
 			if err != nil {
 				return errors.AddStack(err)
 			}
diff --git a/pkg/cluster/operation/scale_in.go b/pkg/cluster/operation/scale_in.go
index 3b8ee1a171..c54ae9ece6 100644
--- a/pkg/cluster/operation/scale_in.go
+++ b/pkg/cluster/operation/scale_in.go
@@ -73,8 +73,9 @@ func ScaleIn(
 	cluster *spec.Specification,
 	options Options,
 	tlsCfg *tls.Config,
+	publicKeyPath string,
 ) error {
-	return ScaleInCluster(getter, cluster, options, tlsCfg)
+	return ScaleInCluster(getter, cluster, options, tlsCfg, publicKeyPath)
 }
 
 // ScaleInCluster scales in the cluster
@@ -83,6 +84,7 @@ func ScaleInCluster(
 	cluster *spec.Specification,
 	options Options,
 	tlsCfg *tls.Config,
+	publicKeyPath string,
 ) error {
 	// instances by uuid
 	instances := map[string]spec.Instance{}
@@ -169,7 +171,7 @@ func ScaleInCluster(
 				}
 
 				instCount[instance.GetHost()]--
-				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0); err != nil {
+				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0, publicKeyPath); err != nil {
 					log.Warnf("failed to stop/destroy %s: %v", compName, err)
 				}
 
@@ -242,7 +244,7 @@ func ScaleInCluster(
 
 			if !asyncOfflineComps.Exist(instance.ComponentName()) {
 				instCount[instance.GetHost()]--
-				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0); err != nil {
+				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0, publicKeyPath); err != nil {
 					return err
 				}
 			} else {
diff --git a/pkg/cluster/task/action.go b/pkg/cluster/task/action.go
index e600998a7b..11b5a8c5a9 100644
--- a/pkg/cluster/task/action.go
+++ b/pkg/cluster/task/action.go
@@ -58,18 +58,18 @@ func (c *ClusterOperate) Execute(ctx *Context) error {
 		}
 		operator.PrintClusterStatus(ctx, c.spec)
 	case operator.DestroyOperation:
-		err := operator.Destroy(ctx, c.spec, c.options)
+		err := operator.Destroy(ctx, c.spec, ctx.PublicKeyPath, c.options)
 		if err != nil {
 			return errors.Annotate(err, "failed to destroy")
 		}
 	case operator.DestroyTombstoneOperation:
-		_, err := operator.DestroyTombstone(ctx, c.spec, false, c.options, c.tlsCfg)
+		_, err := operator.DestroyTombstone(ctx, c.spec, false, c.options, c.tlsCfg, ctx.PublicKeyPath)
 		if err != nil {
 			return errors.Annotate(err, "failed to destroy")
 		}
 	// print nothing
 	case operator.ScaleInOperation:
-		err := operator.ScaleIn(ctx, c.spec, c.options, c.tlsCfg)
+		err := operator.ScaleIn(ctx, c.spec, c.options, c.tlsCfg, ctx.PublicKeyPath)
 		if err != nil {
 			return errors.Annotate(err, "failed to scale in")
 		}
diff --git a/pkg/cluster/task/env_init.go b/pkg/cluster/task/env_init.go
index 74c6e41e31..6c70f97d47 100644
--- a/pkg/cluster/task/env_init.go
+++ b/pkg/cluster/task/env_init.go
@@ -19,6 +19,7 @@ import (
 	"strings"
 
 	"github.com/joomcode/errorx"
+	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/module"
 )
 
@@ -27,8 +28,6 @@ var (
 	errEnvInitSubCommandFailed = errNSEnvInit.NewType("sub_command_failed")
 	// ErrEnvInitFailed is ErrEnvInitFailed
 	ErrEnvInitFailed = errNSEnvInit.NewType("failed")
-	// SSH authorized_keys file
-	defaultSSHAuthorizedKeys = "~/.ssh/authorized_keys"
 )
 
 // EnvInit is used to initialize the remote environment, e.g:
@@ -76,36 +75,15 @@ func (e *EnvInit) execute(ctx *Context) error {
 	}
 
 	// Authorize
-	cmd := `su - ` + e.deployUser + ` -c 'test -d ~/.ssh || mkdir -p ~/.ssh && chmod 700 ~/.ssh'`
+	cmd := `su - ` + e.deployUser + ` -c 'mkdir -p ~/.ssh && chmod 700 ~/.ssh'`
 	_, _, err = exec.Execute(cmd, true)
 	if err != nil {
 		return wrapError(errEnvInitSubCommandFailed.
 			Wrap(err, "Failed to create '~/.ssh' directory for user '%s'", e.deployUser))
 	}
 
-	// detect if custom path of authorized keys file is set
-	// NOTE: we do not yet support:
-	//   - custom config for user (~/.ssh/config)
-	//   - sshd started with custom config (other than /etc/ssh/sshd_config)
-	//   - ssh server implementations other than OpenSSH (such as dropbear)
-	sshAuthorizedKeys := defaultSSHAuthorizedKeys
-	cmd = "grep -Ev '^\\s*#|^\\s*$' /etc/ssh/sshd_config"
-	stdout, _, _ := exec.Execute(cmd, true) // error ignored as we have default value
-	for _, line := range strings.Split(string(stdout), "\n") {
-		if !strings.Contains(line, "AuthorizedKeysFile") {
-			continue
-		}
-		fields := strings.Fields(line)
-		if len(fields) >= 2 {
-			sshAuthorizedKeys = fields[1]
-		}
-	}
-
-	if !strings.HasPrefix(sshAuthorizedKeys, "/") && !strings.HasPrefix(sshAuthorizedKeys, "~") {
-		sshAuthorizedKeys = fmt.Sprintf("~/%s", sshAuthorizedKeys)
-	}
-
 	pk := strings.TrimSpace(string(pubKey))
+	sshAuthorizedKeys := executor.FindSSHAuthorizedKeysFile(exec)
 	cmd = fmt.Sprintf(`su - %[1]s -c 'grep $(echo %[2]s) %[3]s || echo %[2]s >> %[3]s && chmod 600 %[3]s'`,
 		e.deployUser, pk, sshAuthorizedKeys)
 	_, _, err = exec.Execute(cmd, true)

From 4f589db40cb6f81a37550d1433d923cf33a1953c Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Tue, 17 Nov 2020 05:35:13 +0800
Subject: [PATCH 02/13] feat(dm): delete public key if node was scaled in

---
 components/dm/command/scale_in.go | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/components/dm/command/scale_in.go b/components/dm/command/scale_in.go
index cf66c65845..ba19051924 100644
--- a/components/dm/command/scale_in.go
+++ b/components/dm/command/scale_in.go
@@ -46,7 +46,7 @@ func newScaleInCmd() *cobra.Command {
 				b.Func(
 					fmt.Sprintf("ScaleInCluster: options=%+v", gOpt),
 					func(ctx *task.Context) error {
-						return ScaleInDMCluster(ctx, metadata.Topology, gOpt)
+						return ScaleInDMCluster(ctx, metadata.Topology, gOpt, ctx.PublicKeyPath)
 					},
 				).Serial(dmtask.NewUpdateDMMeta(clusterName, metadata, gOpt.Nodes))
 			}
@@ -77,14 +77,17 @@ func ScaleInDMCluster(
 	getter operator.ExecutorGetter,
 	topo *dm.Specification,
 	options operator.Options,
+	publicKeyPath string,
 ) error {
 	// instances by uuid
 	instances := map[string]dm.Instance{}
+	instCount := map[string]int{}
 
 	// make sure all nodeIds exists in topology
 	for _, component := range topo.ComponentsByStartOrder() {
 		for _, instance := range component.Instances() {
 			instances[instance.ID()] = instance
+			instCount[instance.GetHost()] = instCount[instance.GetHost()] + 1
 		}
 	}
 
@@ -110,7 +113,8 @@ func ScaleInDMCluster(
 				if !deletedNodes.Exist(instance.ID()) {
 					continue
 				}
-				if err := operator.StopAndDestroyInstance(getter, topo, instance, options, false); err != nil {
+				instCount[instance.GetHost()]--
+				if err := operator.StopAndDestroyInstance(getter, topo, instance, options, instCount[instance.GetHost()] == 0, publicKeyPath); err != nil {
 					log.Warnf("failed to stop/destroy %s: %v", component.Name(), err)
 				}
 			}
@@ -162,6 +166,14 @@ func ScaleInDMCluster(
 			if err := operator.DestroyComponent(getter, []dm.Instance{instance}, topo, options); err != nil {
 				return errors.Annotatef(err, "failed to destroy %s", component.Name())
 			}
+
+			instCount[instance.GetHost()]--
+			if instCount[instance.GetHost()] == 0 {
+				if err := operator.DeletePublicKey(getter, instance.GetHost(), publicKeyPath); err != nil {
+					return errors.Annotatef(err, "failed to delete public key")
+				}
+			}
+
 		}
 	}
 

From ed2b10e42ac001388998965d63b91b01c05b8753 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Tue, 17 Nov 2020 06:40:28 +0800
Subject: [PATCH 03/13] fix(cluster): trim public key's space

---
 pkg/cluster/operation/destroy.go | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go
index 7d67f7e6f2..e6e4d3871e 100644
--- a/pkg/cluster/operation/destroy.go
+++ b/pkg/cluster/operation/destroy.go
@@ -14,6 +14,7 @@
 package operator
 
 import (
+	"bytes"
 	"crypto/tls"
 	"fmt"
 	"io/ioutil"
@@ -91,7 +92,10 @@ func Destroy(
 		if err := DeleteGlobalDirs(getter, host, gOpts); err != nil {
 			return nil
 		}
+	}
 
+	// after all things done, try to remove SSH public key
+	for host := range instCount {
 		if err := DeletePublicKey(getter, host, publicKeyPath); err != nil {
 			return nil
 		}
@@ -199,13 +203,13 @@ func DeletePublicKey(getter ExecutorGetter, host, pubKeyPath string) error {
 	if err != nil {
 		return perrs.Trace(err)
 	}
-	pubKey := strings.ReplaceAll(string(publicKey), "/", "\\/")
+	pubKey := string(bytes.TrimSpace(publicKey))
+	pubKey = strings.ReplaceAll(pubKey, "/", "\\/")
 	pubKeysFile := executor.FindSSHAuthorizedKeysFile(e)
 
 	c := module.ShellModuleConfig{
 		Command:  fmt.Sprintf("sed -i '/%s/d' %s", pubKey, pubKeysFile),
-		Chdir:    "",
-		UseShell: true,
+		UseShell: false,
 	}
 	shell := module.NewShellModule(c)
 	stdout, stderr, err := shell.Execute(e)

From a86a3b97e985ccf0beea27129cbd1e35d210d40e Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Tue, 17 Nov 2020 06:55:36 +0800
Subject: [PATCH 04/13] test(cluster,dm): test public key should be deleted
 from remote

---
 tests/tiup-cluster/script/cmd_subtest.sh |  5 +++++
 tests/tiup-cluster/script/scale_core.sh  |  2 ++
 tests/tiup-dm/test_cmd.sh                | 11 ++++++++---
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/tests/tiup-cluster/script/cmd_subtest.sh b/tests/tiup-cluster/script/cmd_subtest.sh
index cc1ff0e929..e32d4edc8a 100755
--- a/tests/tiup-cluster/script/cmd_subtest.sh
+++ b/tests/tiup-cluster/script/cmd_subtest.sh
@@ -104,5 +104,10 @@ function cmd_subtest() {
 
     ! tiup-cluster $client _test $name data
 
+    cp "~/.tiup/storage/cluster/$name/ssh/id_rsa" "/tmp/$name.id_rsa"
     tiup-cluster $client --yes destroy $name
+
+    # after destroy the cluster, the public key should be deleted
+    ! ssh -i "/tmp/$name.id_rsa" tidb@$ipprefix.101 "ls"
+    unlink "/tmp/$name.id_rsa"
 }
diff --git a/tests/tiup-cluster/script/scale_core.sh b/tests/tiup-cluster/script/scale_core.sh
index 8936aaccf8..a031939009 100755
--- a/tests/tiup-cluster/script/scale_core.sh
+++ b/tests/tiup-cluster/script/scale_core.sh
@@ -78,6 +78,8 @@ function scale_core() {
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ls /home/tidb/deploy/monitor-9100/deploy/monitor-9100"
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ps aux | grep node_exporter | grep -qv grep"
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ps aux | grep blackbox_exporter | grep -qv grep"
+    # public key should be deleted
+    ! ssh -i "~/.tiup/storage/cluster/$name/ssh/id_rsa" tidb@$ipprefix.102 "ls"
 
     echo "start scale out tidb"
     topo=./topo/full_scale_in_tidb.yaml
diff --git a/tests/tiup-dm/test_cmd.sh b/tests/tiup-dm/test_cmd.sh
index f2fb070ace..216f1c6445 100755
--- a/tests/tiup-dm/test_cmd.sh
+++ b/tests/tiup-dm/test_cmd.sh
@@ -54,8 +54,8 @@ total_sub_one=12
 echo "start scale in dm-master"
 tiup-dm --yes scale-in $name -N $ipprefix.101:8261
 wait_instance_num_reach $name $total_sub_one false
-echo "start scale out dm-master"
 
+echo "start scale out dm-master"
 topo_master=./topo/full_scale_in_dm-master.yaml
 sed "s/__IPPREFIX__/$ipprefix/g" $topo_master.tpl > $topo_master
 tiup-dm --yes scale-out $name $topo_master
@@ -72,8 +72,13 @@ yes | tiup-dm scale-out $name $topo_worker
 # test create a task and can replicate data
 ./script/task/run.sh
 
-tiup-dm --yes destroy $name
-
 # test dm log dir
 tiup-dm notfound-command 2>&1 | grep $HOME/.tiup/logs/tiup-dm-debug
 TIUP_LOG_PATH=/tmp/a/b tiup-dm notfound-command 2>&1 | grep /tmp/a/b/tiup-dm-debug
+
+cp "~/.tiup/storage/dm/clusters/$name/ssh/id_rsa" "/tmp/$name.id_rsa"
+tiup-dm --yes destroy $name
+
+# after destroy the cluster, the public key should be deleted
+! ssh -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls"
+unlink "/tmp/$name.id_rsa"

From dac9d09567fde0a97157e72592f51780f8a0f5cf Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Tue, 17 Nov 2020 22:45:15 +0800
Subject: [PATCH 05/13] fix(tests): don't double quote '~'

---
 tests/tiup-cluster/script/cmd_subtest.sh | 4 ++--
 tests/tiup-dm/test_cmd.sh                | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/tiup-cluster/script/cmd_subtest.sh b/tests/tiup-cluster/script/cmd_subtest.sh
index e32d4edc8a..076b94adec 100755
--- a/tests/tiup-cluster/script/cmd_subtest.sh
+++ b/tests/tiup-cluster/script/cmd_subtest.sh
@@ -104,10 +104,10 @@ function cmd_subtest() {
 
     ! tiup-cluster $client _test $name data
 
-    cp "~/.tiup/storage/cluster/$name/ssh/id_rsa" "/tmp/$name.id_rsa"
+    cp ~/.tiup/storage/cluster/$name/ssh/id_rsa "/tmp/$name.id_rsa"
     tiup-cluster $client --yes destroy $name
 
     # after destroy the cluster, the public key should be deleted
-    ! ssh -i "/tmp/$name.id_rsa" tidb@$ipprefix.101 "ls"
+    ! ssh -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.101 "ls"
     unlink "/tmp/$name.id_rsa"
 }
diff --git a/tests/tiup-dm/test_cmd.sh b/tests/tiup-dm/test_cmd.sh
index 216f1c6445..773840d796 100755
--- a/tests/tiup-dm/test_cmd.sh
+++ b/tests/tiup-dm/test_cmd.sh
@@ -76,9 +76,9 @@ yes | tiup-dm scale-out $name $topo_worker
 tiup-dm notfound-command 2>&1 | grep $HOME/.tiup/logs/tiup-dm-debug
 TIUP_LOG_PATH=/tmp/a/b tiup-dm notfound-command 2>&1 | grep /tmp/a/b/tiup-dm-debug
 
-cp "~/.tiup/storage/dm/clusters/$name/ssh/id_rsa" "/tmp/$name.id_rsa"
+cp ~/.tiup/storage/dm/clusters/$name/ssh/id_rsa "/tmp/$name.id_rsa"
 tiup-dm --yes destroy $name
 
 # after destroy the cluster, the public key should be deleted
-! ssh -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls"
+! ssh -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls"
 unlink "/tmp/$name.id_rsa"

From 5ce654c22d1a186866f6f037c3bddb1eb10e5c77 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Tue, 17 Nov 2020 22:52:09 +0800
Subject: [PATCH 06/13] feat(cluster): extend ExecutorGetter to get sshkey

---
 components/dm/command/scale_in.go  |  4 ++--
 pkg/cluster/manager.go             |  2 +-
 pkg/cluster/operation/destroy.go   | 14 ++++++++------
 pkg/cluster/operation/operation.go |  2 ++
 pkg/cluster/operation/scale_in.go  |  4 ++--
 pkg/cluster/task/action.go         |  2 +-
 pkg/cluster/task/task.go           |  9 +++++++--
 7 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/components/dm/command/scale_in.go b/components/dm/command/scale_in.go
index ba19051924..a56510b345 100644
--- a/components/dm/command/scale_in.go
+++ b/components/dm/command/scale_in.go
@@ -114,7 +114,7 @@ func ScaleInDMCluster(
 					continue
 				}
 				instCount[instance.GetHost()]--
-				if err := operator.StopAndDestroyInstance(getter, topo, instance, options, instCount[instance.GetHost()] == 0, publicKeyPath); err != nil {
+				if err := operator.StopAndDestroyInstance(getter, topo, instance, options, instCount[instance.GetHost()] == 0); err != nil {
 					log.Warnf("failed to stop/destroy %s: %v", component.Name(), err)
 				}
 			}
@@ -169,7 +169,7 @@ func ScaleInDMCluster(
 
 			instCount[instance.GetHost()]--
 			if instCount[instance.GetHost()] == 0 {
-				if err := operator.DeletePublicKey(getter, instance.GetHost(), publicKeyPath); err != nil {
+				if err := operator.DeletePublicKey(getter, instance.GetHost()); err != nil {
 					return errors.Annotatef(err, "failed to delete public key")
 				}
 			}
diff --git a/pkg/cluster/manager.go b/pkg/cluster/manager.go
index 65e2764356..7de201c9f3 100644
--- a/pkg/cluster/manager.go
+++ b/pkg/cluster/manager.go
@@ -384,7 +384,7 @@ func (m *Manager) DestroyCluster(clusterName string, gOpt operator.Options, dest
 			}, tlsCfg)
 		}).
 		Func("DestroyCluster", func(ctx *task.Context) error {
-			return operator.Destroy(ctx, topo, ctx.PublicKeyPath, destroyOpt)
+			return operator.Destroy(ctx, topo, destroyOpt)
 		}).
 		Build()
 
diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go
index e6e4d3871e..8e3e4c4321 100644
--- a/pkg/cluster/operation/destroy.go
+++ b/pkg/cluster/operation/destroy.go
@@ -56,7 +56,6 @@ func Cleanup(
 func Destroy(
 	getter ExecutorGetter,
 	cluster spec.Topology,
-	publicKeyPath string,
 	options Options,
 ) error {
 	coms := cluster.ComponentsByStopOrder()
@@ -96,7 +95,7 @@ func Destroy(
 
 	// after all things done, try to remove SSH public key
 	for host := range instCount {
-		if err := DeletePublicKey(getter, host, publicKeyPath); err != nil {
+		if err := DeletePublicKey(getter, host); err != nil {
 			return nil
 		}
 	}
@@ -107,7 +106,7 @@ func Destroy(
 // StopAndDestroyInstance stop and destroy the instance,
 // if this instance is the host's last one, and the host has monitor deployed,
 // we need to destroy the monitor, either
-func StopAndDestroyInstance(getter ExecutorGetter, cluster spec.Topology, instance spec.Instance, options Options, destroyNode bool, publicKeyPath string) error {
+func StopAndDestroyInstance(getter ExecutorGetter, cluster spec.Topology, instance spec.Instance, options Options, destroyNode bool) error {
 	ignoreErr := options.Force
 	compName := instance.ComponentName()
 
@@ -144,7 +143,7 @@ func StopAndDestroyInstance(getter ExecutorGetter, cluster spec.Topology, instan
 			}
 		}
 
-		if err := DeletePublicKey(getter, instance.GetHost(), publicKeyPath); err != nil {
+		if err := DeletePublicKey(getter, instance.GetHost()); err != nil {
 			if !ignoreErr {
 				return errors.Annotatef(err, "failed to delete public key")
 			}
@@ -196,17 +195,20 @@ func DeleteGlobalDirs(getter ExecutorGetter, host string, options *spec.GlobalOp
 }
 
 // DeletePublicKey deletes the SSH public key from host
-func DeletePublicKey(getter ExecutorGetter, host, pubKeyPath string) error {
+func DeletePublicKey(getter ExecutorGetter, host string) error {
 	e := getter.Get(host)
 	log.Infof("Delete public key %s", host)
+	_, pubKeyPath := getter.GetSSHKeySet()
 	publicKey, err := ioutil.ReadFile(pubKeyPath)
 	if err != nil {
 		return perrs.Trace(err)
 	}
+
 	pubKey := string(bytes.TrimSpace(publicKey))
 	pubKey = strings.ReplaceAll(pubKey, "/", "\\/")
 	pubKeysFile := executor.FindSSHAuthorizedKeysFile(e)
 
+	// delete the public key with Linux `sed` toolkit
 	c := module.ShellModuleConfig{
 		Command:  fmt.Sprintf("sed -i '/%s/d' %s", pubKey, pubKeysFile),
 		UseShell: false,
@@ -518,7 +520,7 @@ func DestroyClusterTombstone(
 
 		for _, instance := range instances {
 			instCount[instance.GetHost()]--
-			err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0, publicKey)
+			err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0)
 			if err != nil {
 				return errors.AddStack(err)
 			}
diff --git a/pkg/cluster/operation/operation.go b/pkg/cluster/operation/operation.go
index 645a80269c..46b63073f4 100644
--- a/pkg/cluster/operation/operation.go
+++ b/pkg/cluster/operation/operation.go
@@ -120,4 +120,6 @@ func FilterInstance(instances []spec.Instance, nodes set.StringSet) (res []spec.
 // ExecutorGetter get the executor by host.
 type ExecutorGetter interface {
 	Get(host string) (e executor.Executor)
+	// GetSSHKeySet gets the SSH private and public key path
+	GetSSHKeySet() (privateKeyPath, publicKeyPath string)
 }
diff --git a/pkg/cluster/operation/scale_in.go b/pkg/cluster/operation/scale_in.go
index c54ae9ece6..fbdb68c7ed 100644
--- a/pkg/cluster/operation/scale_in.go
+++ b/pkg/cluster/operation/scale_in.go
@@ -171,7 +171,7 @@ func ScaleInCluster(
 				}
 
 				instCount[instance.GetHost()]--
-				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0, publicKeyPath); err != nil {
+				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0); err != nil {
 					log.Warnf("failed to stop/destroy %s: %v", compName, err)
 				}
 
@@ -244,7 +244,7 @@ func ScaleInCluster(
 
 			if !asyncOfflineComps.Exist(instance.ComponentName()) {
 				instCount[instance.GetHost()]--
-				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0, publicKeyPath); err != nil {
+				if err := StopAndDestroyInstance(getter, cluster, instance, options, instCount[instance.GetHost()] == 0); err != nil {
 					return err
 				}
 			} else {
diff --git a/pkg/cluster/task/action.go b/pkg/cluster/task/action.go
index 11b5a8c5a9..d46d46716a 100644
--- a/pkg/cluster/task/action.go
+++ b/pkg/cluster/task/action.go
@@ -58,7 +58,7 @@ func (c *ClusterOperate) Execute(ctx *Context) error {
 		}
 		operator.PrintClusterStatus(ctx, c.spec)
 	case operator.DestroyOperation:
-		err := operator.Destroy(ctx, c.spec, ctx.PublicKeyPath, c.options)
+		err := operator.Destroy(ctx, c.spec, c.options)
 		if err != nil {
 			return errors.Annotate(err, "failed to destroy")
 		}
diff --git a/pkg/cluster/task/task.go b/pkg/cluster/task/task.go
index 11f8da2ab9..abea809da8 100644
--- a/pkg/cluster/task/task.go
+++ b/pkg/cluster/task/task.go
@@ -56,7 +56,7 @@ type (
 			checkResults map[string][]*operator.CheckResult
 		}
 
-		// The public/private key is used to access remote server via the user `tidb`
+		// The private/public key is used to access remote server via the user `tidb`
 		PrivateKeyPath string
 		PublicKeyPath  string
 	}
@@ -95,7 +95,7 @@ func NewContext() *Context {
 	}
 }
 
-// Get implements operation ExecutorGetter interface.
+// Get implements the operation.ExecutorGetter interface.
 func (ctx *Context) Get(host string) (e executor.Executor) {
 	ctx.exec.Lock()
 	e, ok := ctx.exec.executors[host]
@@ -107,6 +107,11 @@ func (ctx *Context) Get(host string) (e executor.Executor) {
 	return
 }
 
+// GetSSHKeySet implements the operation.ExecutorGetter interface.
+func (ctx *Context) GetSSHKeySet() (privateKeyPath, publicKeyPath string) {
+	return ctx.PrivateKeyPath, ctx.PublicKeyPath
+}
+
 // GetExecutor get the executor.
 func (ctx *Context) GetExecutor(host string) (e executor.Executor, ok bool) {
 	// Mock point for unit test

From 730c92626f09ce77a1cb7342d4e8d3a8ac9ed625 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Tue, 17 Nov 2020 23:15:01 +0800
Subject: [PATCH 07/13] fix(tests/tiup-cluster): wrong cluster path

---
 tests/tiup-cluster/script/cmd_subtest.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tiup-cluster/script/cmd_subtest.sh b/tests/tiup-cluster/script/cmd_subtest.sh
index 076b94adec..5a88d0394f 100755
--- a/tests/tiup-cluster/script/cmd_subtest.sh
+++ b/tests/tiup-cluster/script/cmd_subtest.sh
@@ -104,7 +104,7 @@ function cmd_subtest() {
 
     ! tiup-cluster $client _test $name data
 
-    cp ~/.tiup/storage/cluster/$name/ssh/id_rsa "/tmp/$name.id_rsa"
+    cp ~/.tiup/storage/cluster/clusters/$name/ssh/id_rsa "/tmp/$name.id_rsa"
     tiup-cluster $client --yes destroy $name
 
     # after destroy the cluster, the public key should be deleted

From 7c7cfea4e652c99e299a1ce9e61921881e7c6564 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Mon, 23 Nov 2020 23:09:23 +0800
Subject: [PATCH 08/13] fix(cluster,dm): don't pass ctx.PublicKeyPath

---
 components/cluster/command/prune.go | 4 ++--
 components/dm/command/scale_in.go   | 3 +--
 pkg/cluster/manager.go              | 2 +-
 pkg/cluster/operation/destroy.go    | 4 +---
 pkg/cluster/operation/scale_in.go   | 4 +---
 pkg/cluster/task/action.go          | 4 ++--
 6 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/components/cluster/command/prune.go b/components/cluster/command/prune.go
index c1635f02b0..3faca3fe6f 100644
--- a/components/cluster/command/prune.go
+++ b/components/cluster/command/prune.go
@@ -73,7 +73,7 @@ func destroyTombstoneIfNeed(clusterName string, metadata *spec.ClusterMeta, opt
 		return perrs.AddStack(err)
 	}
 
-	nodes, err := operator.DestroyTombstone(ctx, topo, true /* returnNodesOnly */, opt, tlsCfg, ctx.PublicKeyPath)
+	nodes, err := operator.DestroyTombstone(ctx, topo, true /* returnNodesOnly */, opt, tlsCfg)
 	if err != nil {
 		return perrs.AddStack(err)
 	}
@@ -93,7 +93,7 @@ func destroyTombstoneIfNeed(clusterName string, metadata *spec.ClusterMeta, opt
 
 	log.Infof("Start destroy Tombstone nodes: %v ...", nodes)
 
-	_, err = operator.DestroyTombstone(ctx, topo, false /* returnNodesOnly */, opt, tlsCfg, ctx.PublicKeyPath)
+	_, err = operator.DestroyTombstone(ctx, topo, false /* returnNodesOnly */, opt, tlsCfg)
 	if err != nil {
 		return perrs.AddStack(err)
 	}
diff --git a/components/dm/command/scale_in.go b/components/dm/command/scale_in.go
index a56510b345..f2459e59ed 100644
--- a/components/dm/command/scale_in.go
+++ b/components/dm/command/scale_in.go
@@ -46,7 +46,7 @@ func newScaleInCmd() *cobra.Command {
 				b.Func(
 					fmt.Sprintf("ScaleInCluster: options=%+v", gOpt),
 					func(ctx *task.Context) error {
-						return ScaleInDMCluster(ctx, metadata.Topology, gOpt, ctx.PublicKeyPath)
+						return ScaleInDMCluster(ctx, metadata.Topology, gOpt)
 					},
 				).Serial(dmtask.NewUpdateDMMeta(clusterName, metadata, gOpt.Nodes))
 			}
@@ -77,7 +77,6 @@ func ScaleInDMCluster(
 	getter operator.ExecutorGetter,
 	topo *dm.Specification,
 	options operator.Options,
-	publicKeyPath string,
 ) error {
 	// instances by uuid
 	instances := map[string]dm.Instance{}
diff --git a/pkg/cluster/manager.go b/pkg/cluster/manager.go
index 7de201c9f3..ad661556b7 100644
--- a/pkg/cluster/manager.go
+++ b/pkg/cluster/manager.go
@@ -612,7 +612,7 @@ func (m *Manager) Display(clusterName string, opt operator.Options) error {
 		}
 
 		// Check if there is some instance in tombstone state
-		nodes, _ := operator.DestroyTombstone(ctx, t, true /* returnNodesOnly */, opt, tlsCfg, ctx.PublicKeyPath)
+		nodes, _ := operator.DestroyTombstone(ctx, t, true /* returnNodesOnly */, opt, tlsCfg)
 		if len(nodes) != 0 {
 			color.Green("There are some nodes can be pruned: \n\tNodes: %+v\n\tYou can destroy them with the command: `tiup cluster prune %s`", nodes, clusterName)
 		}
diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go
index 8e3e4c4321..58b6a6a403 100644
--- a/pkg/cluster/operation/destroy.go
+++ b/pkg/cluster/operation/destroy.go
@@ -477,9 +477,8 @@ func DestroyTombstone(
 	returNodesOnly bool,
 	options Options,
 	tlsCfg *tls.Config,
-	publicKey string,
 ) (nodes []string, err error) {
-	return DestroyClusterTombstone(getter, cluster, returNodesOnly, options, tlsCfg, publicKey)
+	return DestroyClusterTombstone(getter, cluster, returNodesOnly, options, tlsCfg)
 }
 
 // DestroyClusterTombstone remove the tombstone node in spec and destroy them.
@@ -490,7 +489,6 @@ func DestroyClusterTombstone(
 	returNodesOnly bool,
 	options Options,
 	tlsCfg *tls.Config,
-	publicKey string,
 ) (nodes []string, err error) {
 	instCount := map[string]int{}
 	for _, component := range cluster.ComponentsByStartOrder() {
diff --git a/pkg/cluster/operation/scale_in.go b/pkg/cluster/operation/scale_in.go
index fbdb68c7ed..3b8ee1a171 100644
--- a/pkg/cluster/operation/scale_in.go
+++ b/pkg/cluster/operation/scale_in.go
@@ -73,9 +73,8 @@ func ScaleIn(
 	cluster *spec.Specification,
 	options Options,
 	tlsCfg *tls.Config,
-	publicKeyPath string,
 ) error {
-	return ScaleInCluster(getter, cluster, options, tlsCfg, publicKeyPath)
+	return ScaleInCluster(getter, cluster, options, tlsCfg)
 }
 
 // ScaleInCluster scales in the cluster
@@ -84,7 +83,6 @@ func ScaleInCluster(
 	cluster *spec.Specification,
 	options Options,
 	tlsCfg *tls.Config,
-	publicKeyPath string,
 ) error {
 	// instances by uuid
 	instances := map[string]spec.Instance{}
diff --git a/pkg/cluster/task/action.go b/pkg/cluster/task/action.go
index d46d46716a..e600998a7b 100644
--- a/pkg/cluster/task/action.go
+++ b/pkg/cluster/task/action.go
@@ -63,13 +63,13 @@ func (c *ClusterOperate) Execute(ctx *Context) error {
 			return errors.Annotate(err, "failed to destroy")
 		}
 	case operator.DestroyTombstoneOperation:
-		_, err := operator.DestroyTombstone(ctx, c.spec, false, c.options, c.tlsCfg, ctx.PublicKeyPath)
+		_, err := operator.DestroyTombstone(ctx, c.spec, false, c.options, c.tlsCfg)
 		if err != nil {
 			return errors.Annotate(err, "failed to destroy")
 		}
 	// print nothing
 	case operator.ScaleInOperation:
-		err := operator.ScaleIn(ctx, c.spec, c.options, c.tlsCfg, ctx.PublicKeyPath)
+		err := operator.ScaleIn(ctx, c.spec, c.options, c.tlsCfg)
 		if err != nil {
 			return errors.Annotate(err, "failed to scale in")
 		}

From c38df19a1c4ae54ef22295fbe2710db5b6e94ecf Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Wed, 25 Nov 2020 05:20:55 +0800
Subject: [PATCH 09/13] style(*): a = a+1 -> a++

---
 components/dm/command/scale_in.go | 2 +-
 pkg/cluster/operation/destroy.go  | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/components/dm/command/scale_in.go b/components/dm/command/scale_in.go
index f2459e59ed..9e3a935f13 100644
--- a/components/dm/command/scale_in.go
+++ b/components/dm/command/scale_in.go
@@ -86,7 +86,7 @@ func ScaleInDMCluster(
 	for _, component := range topo.ComponentsByStartOrder() {
 		for _, instance := range component.Instances() {
 			instances[instance.ID()] = instance
-			instCount[instance.GetHost()] = instCount[instance.GetHost()] + 1
+			instCount[instance.GetHost()]++
 		}
 	}
 
diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go
index 58b6a6a403..2d2a4b51ac 100644
--- a/pkg/cluster/operation/destroy.go
+++ b/pkg/cluster/operation/destroy.go
@@ -62,8 +62,7 @@ func Destroy(
 
 	instCount := map[string]int{}
 	cluster.IterInstance(func(inst spec.Instance) {
-		host := inst.GetHost()
-		instCount[host] = instCount[host] + 1
+		instCount[inst.GetHost()]++
 	})
 
 	for _, com := range coms {

From db8265a0aba9ffe6d85c7726a2ce444fce6052cd Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Wed, 25 Nov 2020 05:24:11 +0800
Subject: [PATCH 10/13] style(*): format the other instCount++ style

---
 pkg/cluster/operation/action.go   | 2 +-
 pkg/cluster/operation/destroy.go  | 2 +-
 pkg/cluster/operation/scale_in.go | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/cluster/operation/action.go b/pkg/cluster/operation/action.go
index 4bb0c4182f..622a6f198a 100644
--- a/pkg/cluster/operation/action.go
+++ b/pkg/cluster/operation/action.go
@@ -114,7 +114,7 @@ func Stop(
 
 	instCount := map[string]int{}
 	cluster.IterInstance(func(inst spec.Instance) {
-		instCount[inst.GetHost()] = instCount[inst.GetHost()] + 1
+		instCount[inst.GetHost()]++
 	})
 
 	for _, comp := range components {
diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go
index 2d2a4b51ac..8283a3f656 100644
--- a/pkg/cluster/operation/destroy.go
+++ b/pkg/cluster/operation/destroy.go
@@ -492,7 +492,7 @@ func DestroyClusterTombstone(
 	instCount := map[string]int{}
 	for _, component := range cluster.ComponentsByStartOrder() {
 		for _, instance := range component.Instances() {
-			instCount[instance.GetHost()] = instCount[instance.GetHost()] + 1
+			instCount[instance.GetHost()]++
 		}
 	}
 
diff --git a/pkg/cluster/operation/scale_in.go b/pkg/cluster/operation/scale_in.go
index 3b8ee1a171..4b14c0ca1a 100644
--- a/pkg/cluster/operation/scale_in.go
+++ b/pkg/cluster/operation/scale_in.go
@@ -92,7 +92,7 @@ func ScaleInCluster(
 	for _, component := range cluster.ComponentsByStartOrder() {
 		for _, instance := range component.Instances() {
 			instances[instance.ID()] = instance
-			instCount[instance.GetHost()] = instCount[instance.GetHost()] + 1
+			instCount[instance.GetHost()]++
 		}
 	}
 

From c221ee70257e1cb5b276be15c698d320ba151810 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Wed, 25 Nov 2020 06:22:16 +0800
Subject: [PATCH 11/13] fix(tests): add StrictHostKeyChecking=no to disable
 prompt input

---
 tests/tiup-cluster/script/cmd_subtest.sh | 2 +-
 tests/tiup-cluster/script/scale_core.sh  | 4 ++--
 tests/tiup-dm/test_cmd.sh                | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/tiup-cluster/script/cmd_subtest.sh b/tests/tiup-cluster/script/cmd_subtest.sh
index 5a88d0394f..96b16b8e3b 100755
--- a/tests/tiup-cluster/script/cmd_subtest.sh
+++ b/tests/tiup-cluster/script/cmd_subtest.sh
@@ -108,6 +108,6 @@ function cmd_subtest() {
     tiup-cluster $client --yes destroy $name
 
     # after destroy the cluster, the public key should be deleted
-    ! ssh -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.101 "ls"
+    ssh -o "StrictHostKeyChecking=no" -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls" 2>&1 | grep "Permission denied"
     unlink "/tmp/$name.id_rsa"
 }
diff --git a/tests/tiup-cluster/script/scale_core.sh b/tests/tiup-cluster/script/scale_core.sh
index a031939009..18d679a72b 100755
--- a/tests/tiup-cluster/script/scale_core.sh
+++ b/tests/tiup-cluster/script/scale_core.sh
@@ -78,8 +78,8 @@ function scale_core() {
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ls /home/tidb/deploy/monitor-9100/deploy/monitor-9100"
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ps aux | grep node_exporter | grep -qv grep"
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ps aux | grep blackbox_exporter | grep -qv grep"
-    # public key should be deleted
-    ! ssh -i "~/.tiup/storage/cluster/$name/ssh/id_rsa" tidb@$ipprefix.102 "ls"
+    # after all components on the node were scale-ined, the SSH public is automatically deleted
+    ssh -o "StrictHostKeyChecking=no "-o "PasswordAuthentication=no" -i ~/.tiup/storage/cluster/$name/ssh/id_rsa tidb@$ipprefix.102 "ls" 2>&1 | grep "Permission denied"
 
     echo "start scale out tidb"
     topo=./topo/full_scale_in_tidb.yaml
diff --git a/tests/tiup-dm/test_cmd.sh b/tests/tiup-dm/test_cmd.sh
index 773840d796..223f75bf91 100755
--- a/tests/tiup-dm/test_cmd.sh
+++ b/tests/tiup-dm/test_cmd.sh
@@ -80,5 +80,5 @@ cp ~/.tiup/storage/dm/clusters/$name/ssh/id_rsa "/tmp/$name.id_rsa"
 tiup-dm --yes destroy $name
 
 # after destroy the cluster, the public key should be deleted
-! ssh -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls"
+ssh -o "StrictHostKeyChecking=no" -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls" 2>&1 | grep "Permission denied"
 unlink "/tmp/$name.id_rsa"

From 5c2f7cd6b261ff6d183c4a8bc97eb943299a4ad6 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Wed, 25 Nov 2020 06:22:36 +0800
Subject: [PATCH 12/13] style(cluster): duplicate log

---
 pkg/cluster/operation/destroy.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go
index 8283a3f656..f09b9581d3 100644
--- a/pkg/cluster/operation/destroy.go
+++ b/pkg/cluster/operation/destroy.go
@@ -235,7 +235,6 @@ func DestroyMonitored(getter ExecutorGetter, inst spec.Instance, options *spec.M
 	e := getter.Get(inst.GetHost())
 	log.Infof("Destroying monitored %s", inst.GetHost())
 
-	log.Infof("Destroying monitored")
 	log.Infof("\tDestroying instance %s", inst.GetHost())
 
 	// Stop by systemd.

From 4e5f4e9f2682306c1d80b4637bcd0a8ca87b4695 Mon Sep 17 00:00:00 2001
From: 9547 <nivefive9547@gmail.com>
Date: Wed, 25 Nov 2020 07:48:46 +0800
Subject: [PATCH 13/13] fix(tests): when set -e the error raised first then
 pipe

---
 tests/tiup-cluster/script/cmd_subtest.sh | 2 +-
 tests/tiup-cluster/script/scale_core.sh  | 2 +-
 tests/tiup-dm/test_cmd.sh                | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/tiup-cluster/script/cmd_subtest.sh b/tests/tiup-cluster/script/cmd_subtest.sh
index 96b16b8e3b..5089efa5cf 100755
--- a/tests/tiup-cluster/script/cmd_subtest.sh
+++ b/tests/tiup-cluster/script/cmd_subtest.sh
@@ -108,6 +108,6 @@ function cmd_subtest() {
     tiup-cluster $client --yes destroy $name
 
     # after destroy the cluster, the public key should be deleted
-    ssh -o "StrictHostKeyChecking=no" -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls" 2>&1 | grep "Permission denied"
+    ! ssh -o "StrictHostKeyChecking=no" -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.101 "ls"
     unlink "/tmp/$name.id_rsa"
 }
diff --git a/tests/tiup-cluster/script/scale_core.sh b/tests/tiup-cluster/script/scale_core.sh
index 18d679a72b..f18dfaef6c 100755
--- a/tests/tiup-cluster/script/scale_core.sh
+++ b/tests/tiup-cluster/script/scale_core.sh
@@ -79,7 +79,7 @@ function scale_core() {
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ps aux | grep node_exporter | grep -qv grep"
     ! tiup-cluster $client exec $name -N $ipprefix.102 --command "ps aux | grep blackbox_exporter | grep -qv grep"
     # after all components on the node were scale-ined, the SSH public is automatically deleted
-    ssh -o "StrictHostKeyChecking=no "-o "PasswordAuthentication=no" -i ~/.tiup/storage/cluster/$name/ssh/id_rsa tidb@$ipprefix.102 "ls" 2>&1 | grep "Permission denied"
+    ! ssh -o "StrictHostKeyChecking=no "-o "PasswordAuthentication=no" -i ~/.tiup/storage/cluster/$name/ssh/id_rsa tidb@$ipprefix.102 "ls"
 
     echo "start scale out tidb"
     topo=./topo/full_scale_in_tidb.yaml
diff --git a/tests/tiup-dm/test_cmd.sh b/tests/tiup-dm/test_cmd.sh
index 223f75bf91..6631337a02 100755
--- a/tests/tiup-dm/test_cmd.sh
+++ b/tests/tiup-dm/test_cmd.sh
@@ -80,5 +80,5 @@ cp ~/.tiup/storage/dm/clusters/$name/ssh/id_rsa "/tmp/$name.id_rsa"
 tiup-dm --yes destroy $name
 
 # after destroy the cluster, the public key should be deleted
-ssh -o "StrictHostKeyChecking=no" -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls" 2>&1 | grep "Permission denied"
+! ssh -o "StrictHostKeyChecking=no" -o "PasswordAuthentication=no" -i "/tmp/$name.id_rsa" tidb@$ipprefix.102 "ls"
 unlink "/tmp/$name.id_rsa"