diff --git a/pkg/cluster/executor/local.go b/pkg/cluster/executor/local.go index 17d2937ce9..601847b9d4 100644 --- a/pkg/cluster/executor/local.go +++ b/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 diff --git a/pkg/cluster/executor/local_test.go b/pkg/cluster/executor/local_test.go index c82ecfa568..d11a63c7d6 100644 --- a/pkg/cluster/executor/local_test.go +++ b/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) + } + } +} diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go index 97f195eb73..a93691ad1f 100644 --- a/pkg/cluster/operation/destroy.go +++ b/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) } diff --git a/pkg/cluster/spec/testdata/countdir.yaml b/pkg/cluster/spec/testdata/countdir.yaml new file mode 100644 index 0000000000..01fd510907 --- /dev/null +++ b/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 diff --git a/pkg/cluster/spec/util.go b/pkg/cluster/spec/util.go index c7dedef99f..9c23c6d26e 100644 --- a/pkg/cluster/spec/util.go +++ b/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 diff --git a/pkg/cluster/spec/util_test.go b/pkg/cluster/spec/util_test.go index 5469861569..fb5d487d79 100644 --- a/pkg/cluster/spec/util_test.go +++ b/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) diff --git a/pkg/cluster/spec/validate_test.go b/pkg/cluster/spec/validate_test.go index e36f2921a6..e38afe0a48 100644 --- a/pkg/cluster/spec/validate_test.go +++ b/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(`