Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cluster: fix parsing of paths if there are redundant slashes in it #1068

Merged
merged 4 commits into from
Jan 18, 2021
Merged

cluster: fix parsing of paths if there are redundant slashes in it #1068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
46259e6
cluster: not deleting deploy dir for imported instance
AstroProfundis Jan 14, 2021
b1b0931
cluster/spec: trim slashes in paths
AstroProfundis Jan 14, 2021
3a42a5c
Merge branch 'master' into fix-abs
AstroProfundis Jan 14, 2021
1d270e0
Merge branch 'master' into fix-abs
ti-chi-bot Jan 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/cluster/operation/destroy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
106 changes: 106 additions & 0 deletions pkg/cluster/spec/testdata/countdir.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
7 changes: 5 additions & 2 deletions pkg/cluster/spec/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions pkg/cluster/spec/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 30 additions & 0 deletions pkg/cluster/spec/validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ package spec

import (
"fmt"
"io/ioutil"
"path/filepath"

"github.com/joomcode/errorx"
. "github.com/pingcap/check"
Expand Down Expand Up @@ -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(`
Expand Down