Merge branch 'master' into fix/prometheus-rules-missing

pkg/cluster/executor/local.go

@@ -42,9 +42,9 @@ var _ Executor = &Local{}
 func (l *Local) Execute(cmd string, sudo bool, timeout ...time.Duration) ([]byte, []byte, error) {
 	// try to acquire root permission
 	if l.Sudo || sudo {
-		cmd = fmt.Sprintf("sudo -H -u root bash -c \"cd; %s\"", cmd)
+		cmd = fmt.Sprintf("sudo -H -u root bash -c 'cd; %s'", cmd)
 	} else {
-		cmd = fmt.Sprintf("sudo -H -u %s bash -c \"cd; %s\"", l.Config.User, cmd)
+		cmd = fmt.Sprintf("sudo -H -u %s bash -c 'cd; %s'", l.Config.User, cmd)
 	}
 
 	// set a basic PATH in case it's empty on login

pkg/cluster/executor/local_test.go

@@ -14,6 +14,7 @@
 package executor
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/user"
@@ -66,3 +67,27 @@ func TestWrongIP(t *testing.T) {
 	assert.NotNil(err)
 	assert.Contains(err.Error(), "not found")
 }
+
+func TestLocalExecuteWithQuotes(t *testing.T) {
+	assert := require.New(t)
+	user, err := user.Current()
+	assert.Nil(err)
+	local, err := New(SSHTypeNone, false, SSHConfig{Host: "127.0.0.1", User: user.Username})
+	assert.Nil(err)
+
+	deployDir, err := ioutil.TempDir("", "tiup-*")
+	assert.Nil(err)
+	defer os.RemoveAll(deployDir)
+
+	cmds := []string{
+		fmt.Sprintf(`find %s -type f -exec sed -i "s/\${DS_.*-CLUSTER}/hello/g" {} \;`, deployDir),
+		fmt.Sprintf(`find %s -type f -exec sed -i "s/DS_.*-CLUSTER/hello/g" {} \;`, deployDir),
+		`ls '/tmp'`,
+	}
+	for _, cmd := range cmds {
+		for _, sudo := range []bool{true, false} {
+			_, _, err = local.Execute(cmd, sudo)
+			assert.Nil(err)
+		}
+	}
+}

pkg/cluster/operation/destroy.go

@@ -385,7 +385,7 @@ func DestroyComponent(getter ExecutorGetter, instances []spec.Instance, cls spec
 				dpCnt++
 			}
 		}
-		if cls.CountDir(ins.GetHost(), deployDir)-dpCnt == 1 {
+		if !ins.IsImported() && cls.CountDir(ins.GetHost(), deployDir)-dpCnt == 1 {
 			delPaths.Insert(deployDir)
 		}
 

pkg/cluster/spec/testdata/countdir.yaml

@@ -0,0 +1,106 @@
+user: tidb
+tidb_version: v4.0.2
+last_ops_ver: |-
+  v1.3.0 tiup
+  Go Version: go1.13
+  Git Branch: release-1.3
+  GitHash: edb12b8
+topology:
+  global:
+    user: tidb
+    ssh_port: 22
+    ssh_type: builtin
+    deploy_dir: deploy
+    data_dir: data
+    os: linux
+    arch: amd64
+  monitored:
+    node_exporter_port: 21580
+    blackbox_exporter_port: 21581
+    deploy_dir: /home/tidb/deploy/monitor-21580
+    data_dir: /home/tidb/deploy/monitor-21580/data/monitor-21580
+    log_dir: /home/tidb/deploy/monitor-21580/deploy/monitor-21580/log
+  tidb_servers:
+  - host: 172.17.0.4
+    ssh_port: 22
+    imported: true
+    port: 21500
+    status_port: 21501
+    deploy_dir: /foo/bar/sometidbpath123/
+    log_dir: /foo/bar/sometidbpath123//log
+    arch: amd64
+    os: linux
+  tikv_servers:
+  - host: 172.16.81.162
+    ssh_port: 22
+    imported: true
+    port: 21520
+    status_port: 21530
+    deploy_dir: /work/foobar456
+    data_dir: /work/foobar456/data
+    log_dir: /work/foobar456/log
+    arch: amd64
+    os: linux
+  - host: 172.16.81.205
+    ssh_port: 22
+    imported: true
+    port: 21520
+    status_port: 21530
+    deploy_dir: /work/foobar456
+    data_dir: /work/foobar456/data
+    log_dir: /work/foobar456/log
+    arch: amd64
+    os: linux
+  - host: 172.16.100.199
+    ssh_port: 22
+    imported: true
+    port: 21520
+    status_port: 21530
+    deploy_dir: /work/foobar456
+    data_dir: /work/foobar456/data
+    log_dir: /work/foobar456/log
+    arch: amd64
+    os: linux
+  tiflash_servers: []
+  pd_servers:
+  - host: 172.18.222.51
+    ssh_port: 22
+    name: pd-172.18.222.51-21550
+    client_port: 21550
+    peer_port: 21551
+    deploy_dir: /foo/bar/sometidbpath123/deploy/pd-21550
+    data_dir: /foo/bar/sometidbpath123/deploy/pd-21550/data
+    log_dir: /foo/bar/sometidbpath123/log/pd-21550
+    arch: amd64
+    os: linux
+  - host: 172.18.4.42
+    ssh_port: 22
+    name: pd-172.18.4.42-21550
+    client_port: 21550
+    peer_port: 21551
+    deploy_dir: /foo/bar/sometidbpath123/deploy/pd-21550
+    data_dir: /foo/bar/sometidbpath123/deploy/pd-21550/data
+    log_dir: /foo/bar/sometidbpath123/log/pd-21550
+    arch: amd64
+    os: linux
+  - host: 172.18.10.16
+    ssh_port: 22
+    name: pd-172.18.10.16-21550
+    client_port: 21550
+    peer_port: 21551
+    deploy_dir: /foo/bar/sometidbpath123/deploy/pd-21550
+    data_dir: /foo/bar/sometidbpath123/deploy/pd-21550/data
+    log_dir: /foo/bar/sometidbpath123/log/pd-21550
+    arch: amd64
+    os: linux
+  monitoring_servers: []
+  grafana_servers:
+  - host: 172.17.0.4
+    ssh_port: 22
+    imported: true
+    port: 21570
+    deploy_dir: /foo/bar/sometidbpath123/
+    arch: amd64
+    os: linux
+    username: admin
+    password: admin

pkg/cluster/spec/util.go

@@ -133,10 +133,13 @@ func statusByURL(url string, tlsCfg *tls.Config) string {
 
 // Abs returns the absolute path
 func Abs(user, path string) string {
+	// trim whitespaces before joining
+	user = strings.TrimSpace(user)
+	path = strings.TrimSpace(path)
 	if !strings.HasPrefix(path, "/") {
-		return filepath.Join("/home", user, path)
+		path = filepath.Join("/home", user, path)
 	}
-	return path
+	return filepath.Clean(path)
 }
 
 // MultiDirAbs returns the absolute path for multi-dir separated by comma

pkg/cluster/spec/util_test.go

@@ -21,6 +21,30 @@ type utilSuite struct{}
 
 var _ = check.Suite(&utilSuite{})
 
+func (s utilSuite) TestAbs(c *check.C) {
+	var path string
+	path = Abs(" foo", "")
+	c.Assert(path, check.Equals, "/home/foo")
+	path = Abs("foo ", " ")
+	c.Assert(path, check.Equals, "/home/foo")
+	path = Abs("foo", "bar")
+	c.Assert(path, check.Equals, "/home/foo/bar")
+	path = Abs("foo", " bar")
+	c.Assert(path, check.Equals, "/home/foo/bar")
+	path = Abs("foo", "bar ")
+	c.Assert(path, check.Equals, "/home/foo/bar")
+	path = Abs("foo", " bar ")
+	c.Assert(path, check.Equals, "/home/foo/bar")
+	path = Abs("foo", "/bar")
+	c.Assert(path, check.Equals, "/bar")
+	path = Abs("foo", " /bar")
+	c.Assert(path, check.Equals, "/bar")
+	path = Abs("foo", "/bar ")
+	c.Assert(path, check.Equals, "/bar")
+	path = Abs("foo", " /bar ")
+	c.Assert(path, check.Equals, "/bar")
+}
+
 func (s *utilSuite) TestMultiDirAbs(c *check.C) {
 	paths := MultiDirAbs("tidb", "")
 	c.Assert(len(paths), check.Equals, 0)

pkg/cluster/spec/validate_test.go

@@ -15,6 +15,8 @@ package spec
 
 import (
 	"fmt"
+	"io/ioutil"
+	"path/filepath"
 
 	"github.com/joomcode/errorx"
 	. "github.com/pingcap/check"
@@ -278,6 +280,34 @@ pd_servers:
 	c.Assert(cnt, Equals, 5)
 }
 
+func (s *metaSuiteTopo) TestCountDir2(c *C) {
+	file := filepath.Join("testdata", "countdir.yaml")
+	meta := ClusterMeta{}
+	yamlFile, err := ioutil.ReadFile(file)
+	c.Assert(err, IsNil)
+	err = yaml.UnmarshalStrict(yamlFile, &meta)
+	c.Assert(err, IsNil)
+	topo := meta.Topology
+
+	// If the imported dir is somehow containing paths ens with slash,
+	// or having multiple slash in it, the count result should not
+	// be different.
+	cnt := topo.CountDir("172.17.0.4", "/foo/bar/sometidbpath123")
+	c.Assert(cnt, Equals, 3)
+	cnt = topo.CountDir("172.17.0.4", "/foo/bar/sometidbpath123/")
+	c.Assert(cnt, Equals, 3)
+	cnt = topo.CountDir("172.17.0.4", "/foo/bar/sometidbpath123//")
+	c.Assert(cnt, Equals, 3)
+	cnt = topo.CountDir("172.17.0.4", "/foo/bar/sometidbpath123/log")
+	c.Assert(cnt, Equals, 1)
+	cnt = topo.CountDir("172.17.0.4", "/foo/bar/sometidbpath123//log")
+	c.Assert(cnt, Equals, 1)
+	cnt = topo.CountDir("172.17.0.4", "/foo/bar/sometidbpath123/log/")
+	c.Assert(cnt, Equals, 1)
+	cnt = topo.CountDir("172.17.0.4", "/foo/bar/sometidbpath123//log/")
+	c.Assert(cnt, Equals, 1)
+}
+
 func (s *metaSuiteTopo) TestTiSparkSpecValidation(c *C) {
 	topo := Specification{}
 	err := yaml.Unmarshal([]byte(`