Feature/disable log or data dir overlapped

pkg/cluster/spec/validate.go

@@ -27,13 +27,15 @@ import (
 	"github.com/pingcap/tiup/pkg/errutil"
 	"github.com/pingcap/tiup/pkg/meta"
 	"github.com/pingcap/tiup/pkg/set"
+	"github.com/pingcap/tiup/pkg/utils"
 	"go.uber.org/zap"
 )
 
 // pre defined error types
 var (
 	errNSDeploy              = errNS.NewSubNamespace("deploy")
 	errDeployDirConflict     = errNSDeploy.NewType("dir_conflict", errutil.ErrTraitPreCheck)
+	errDeployDirOverlap      = errNSDeploy.NewType("dir_overlap", errutil.ErrTraitPreCheck)
 	errDeployPortConflict    = errNSDeploy.NewType("port_conflict", errutil.ErrTraitPreCheck)
 	ErrNoTiSparkMaster       = errors.New("there must be a Spark master node if you want to use the TiSpark component")
 	ErrMultipleTiSparkMaster = errors.New("a TiSpark enabled cluster with more than 1 Spark master node is not supported")
@@ -62,13 +64,13 @@ func fixDir(topo Topology) func(string) string {
 	}
 }
 
-// CheckClusterDirConflict checks cluster dir conflict
-func CheckClusterDirConflict(clusterList map[string]Metadata, clusterName string, topo Topology) error {
-	type DirAccessor struct {
-		dirKind  string
-		accessor func(Instance, Topology) string
-	}
+// DirAccessor stands for a directory accessor for an instance
+type DirAccessor struct {
+	dirKind  string
+	accessor func(Instance, Topology) string
+}
 
+func dirAccessors() ([]DirAccessor, []DirAccessor) {
 	instanceDirAccessor := []DirAccessor{
 		{dirKind: "deploy directory", accessor: func(instance Instance, topo Topology) string { return instance.DeployDir() }},
 		{dirKind: "data directory", accessor: func(instance Instance, topo Topology) string { return instance.DataDir() }},
@@ -80,90 +82,82 @@ func CheckClusterDirConflict(clusterList map[string]Metadata, clusterName string
 			if m == nil {
 				return ""
 			}
-			return topo.BaseTopo().MonitoredOptions.DeployDir
+			return m.DeployDir
 		}},
 		{dirKind: "monitor data directory", accessor: func(instance Instance, topo Topology) string {
 			m := topo.BaseTopo().MonitoredOptions
 			if m == nil {
 				return ""
 			}
-			return topo.BaseTopo().MonitoredOptions.DataDir
+			return m.DataDir
 		}},
 		{dirKind: "monitor log directory", accessor: func(instance Instance, topo Topology) string {
 			m := topo.BaseTopo().MonitoredOptions
 			if m == nil {
 				return ""
 			}
-			return topo.BaseTopo().MonitoredOptions.LogDir
+			return m.LogDir
 		}},
 	}
 
-	type Entry struct {
-		clusterName string
-		dirKind     string
-		dir         string
-		instance    Instance
+	return instanceDirAccessor, hostDirAccessor
+}
+
+// DirEntry stands for a directory with attributes and instance
+type DirEntry struct {
+	clusterName string
+	dirKind     string
+	dir         string
+	instance    Instance
+}
+
+func appendEntries(name string, topo Topology, inst Instance, dirAccessor DirAccessor, targets []DirEntry) []DirEntry {
+	for _, dir := range strings.Split(fixDir(topo)(dirAccessor.accessor(inst, topo)), ",") {
+		targets = append(targets, DirEntry{
+			clusterName: name,
+			dirKind:     dirAccessor.dirKind,
+			dir:         dir,
+			instance:    inst,
+		})
 	}
 
-	currentEntries := []Entry{}
-	existingEntries := []Entry{}
+	return targets
+}
 
+// CheckClusterDirConflict checks cluster dir conflict or overlap
+func CheckClusterDirConflict(clusterList map[string]Metadata, clusterName string, topo Topology) error {
+	instanceDirAccessor, hostDirAccessor := dirAccessors()
+	currentEntries := []DirEntry{}
+	existingEntries := []DirEntry{}
+
+	// rebuild existing disk status
 	for name, metadata := range clusterList {
 		if name == clusterName {
 			continue
 		}
 
 		topo := metadata.GetTopology()
 
-		f := fixDir(topo)
 		topo.IterInstance(func(inst Instance) {
 			for _, dirAccessor := range instanceDirAccessor {
-				for _, dir := range strings.Split(f(dirAccessor.accessor(inst, topo)), ",") {
-					existingEntries = append(existingEntries, Entry{
-						clusterName: name,
-						dirKind:     dirAccessor.dirKind,
-						dir:         dir,
-						instance:    inst,
-					})
-				}
+				existingEntries = appendEntries(name, topo, inst, dirAccessor, existingEntries)
 			}
 		})
 		IterHost(topo, func(inst Instance) {
 			for _, dirAccessor := range hostDirAccessor {
-				for _, dir := range strings.Split(f(dirAccessor.accessor(inst, topo)), ",") {
-					existingEntries = append(existingEntries, Entry{
-						clusterName: name,
-						dirKind:     dirAccessor.dirKind,
-						dir:         dir,
-						instance:    inst,
-					})
-				}
+				existingEntries = appendEntries(name, topo, inst, dirAccessor, existingEntries)
 			}
 		})
 	}
 
-	f := fixDir(topo)
 	topo.IterInstance(func(inst Instance) {
 		for _, dirAccessor := range instanceDirAccessor {
-			for _, dir := range strings.Split(f(dirAccessor.accessor(inst, topo)), ",") {
-				currentEntries = append(currentEntries, Entry{
-					dirKind:  dirAccessor.dirKind,
-					dir:      dir,
-					instance: inst,
-				})
-			}
+			currentEntries = appendEntries(clusterName, topo, inst, dirAccessor, currentEntries)
 		}
 	})
-
 	IterHost(topo, func(inst Instance) {
 		for _, dirAccessor := range hostDirAccessor {
-			for _, dir := range strings.Split(f(dirAccessor.accessor(inst, topo)), ",") {
-				currentEntries = append(currentEntries, Entry{
-					dirKind:  dirAccessor.dirKind,
-					dir:      dir,
-					instance: inst,
-				})
-			}
+			currentEntries = appendEntries(clusterName, topo, inst, dirAccessor, currentEntries)
 		}
 	})
 
@@ -207,6 +201,54 @@ Please change to use another directory or another host.
 		}
 	}
 
+	return CheckClusterDirOverlap(currentEntries)
+}
+
+// CheckClusterDirOverlap checks cluster dir overlaps with data or log
+// we don't allow to deploy log under data, and vise versa
+// ref https://github.com/pingcap/tiup/issues/1047#issuecomment-761711508
+func CheckClusterDirOverlap(entries []DirEntry) error {
+	ignore := func(d1, d2 DirEntry) bool {
+		return (d1.instance.GetHost() != d2.instance.GetHost()) ||
+			d1.dir == "" || d2.dir == "" ||
+			strings.HasSuffix(d1.dirKind, "deploy directory") ||
+			strings.HasSuffix(d2.dirKind, "deploy directory")
+	}
+	for i := 0; i < len(entries)-1; i++ {
+		d1 := entries[i]
+		for j := i + 1; j < len(entries); j++ {
+			d2 := entries[j]
+			if ignore(d1, d2) {
+				continue
+			}
+
+			if utils.IsSubDir(d1.dir, d2.dir) || utils.IsSubDir(d2.dir, d1.dir) {
+				properties := map[string]string{
+					"ThisDirKind":   d1.dirKind,
+					"ThisDir":       d1.dir,
+					"ThisComponent": d1.instance.ComponentName(),
+					"ThisHost":      d1.instance.GetHost(),
+					"ThatDirKind":   d2.dirKind,
+					"ThatDir":       d2.dir,
+					"ThatComponent": d2.instance.ComponentName(),
+					"ThatHost":      d2.instance.GetHost(),
+				}
+				zap.L().Info("Meet deploy directory overlap", zap.Any("info", properties))
+				return errDeployDirOverlap.New("Deploy directory overlaps to another instance").WithProperty(cliutil.SuggestionFromTemplate(`
+The directory you specified in the topology file is:
+  Directory: {{ColorKeyword}}{{.ThisDirKind}} {{.ThisDir}}{{ColorReset}}
+  Component: {{ColorKeyword}}{{.ThisComponent}} {{.ThisHost}}{{ColorReset}}
+
+It overlaps to another instance:
+  Other Directory: {{ColorKeyword}}{{.ThatDirKind}} {{.ThatDir}}{{ColorReset}}
+  Other Component: {{ColorKeyword}}{{.ThatComponent}} {{.ThatHost}}{{ColorReset}}
+
+Please modify the topology file and try again.
+`, properties))
+			}
+		}
+	}
+
 	return nil
 }
 

pkg/cluster/spec/validate_test.go

@@ -992,3 +992,143 @@ global:
 	c.Assert(topo.GlobalOptions.User, Equals, "tidb")
 	c.Assert(topo.GlobalOptions.Group, Equals, "")
 }
+
+func (s *metaSuiteTopo) TestLogDirUnderDataDir(c *C) {
+	topo := Specification{}
+	clsList := make(map[string]Metadata)
+
+	err := yaml.Unmarshal([]byte(`
+global:
+  user: "test1"
+  ssh_port: 220
+  deploy_dir: deploy
+  data_dir: data
+tikv_servers:
+  - host: n1
+    port: 32160
+    status_port: 32180
+    log_dir: "/home/tidb6wu/tidb1-data/tikv-32160/log"
+    data_dir: "/home/tidb6wu/tidb1-data/tikv-32160"
+`), &topo)
+	c.Assert(err, IsNil)
+	err = CheckClusterDirConflict(clsList, "topo", &topo)
+	c.Assert(err, NotNil)
+	c.Assert(err.Error(), Equals, "spec.deploy.dir_overlap: Deploy directory overlaps to another instance")
+	suggestion, ok := errorx.ExtractProperty(err, errutil.ErrPropSuggestion)
+	c.Assert(ok, IsTrue)
+	c.Assert(suggestion, Equals, `The directory you specified in the topology file is:
+  Directory: data directory /home/tidb6wu/tidb1-data/tikv-32160
+  Component: tikv n1
+
+It overlaps to another instance:
+  Other Directory: log directory /home/tidb6wu/tidb1-data/tikv-32160/log
+  Other Component: tikv n1
+
+Please modify the topology file and try again.`)
+
+	goodTopos := []string{
+		`
+tikv_servers:
+  - host: n1
+    log_dir: 'tikv-data/log'
+  - host: n2
+    data_dir: 'tikv-data'
+`,
+		`
+tikv_servers:
+  - host: n1
+    port: 32160
+    status_port: 32180
+    log_dir: "/home/tidb6wu/tidb1-data/tikv-32160-log"
+    data_dir: "/home/tidb6wu/tidb1-data/tikv-32160"
+`,
+	}
+	for _, s := range goodTopos {
+		err = yaml.Unmarshal([]byte(s), &topo)
+		c.Assert(err, IsNil)
+		err = CheckClusterDirConflict(clsList, "topo", &topo)
+		c.Assert(err, IsNil)
+	}
+
+	overlapTopos := []string{
+		`
+tikv_servers:
+  - host: n1
+    log_dir: 'tikv-data/log'
+    data_dir: 'tikv-data'
+`,
+		`
+tikv_servers:
+  - host: n1
+    log_dir: '/home/tidb6wu/tidb1-data/tikv-32160/log'
+    data_dir: '/home/tidb6wu/tidb1-data/tikv-32160'
+`,
+		`
+tikv_servers:
+  - host: n1
+    log_dir: '/home/tidb6wu/tidb1-data/tikv-32160/log'
+    data_dir: '/home/tidb6wu/tidb1-data/tikv-32160/log/data'
+`,
+		`
+tikv_servers:
+  - host: n1
+    log_dir: 'tikv-log'
+    data_dir: 'tikv-log/data'
+`,
+		`
+tikv_servers:
+  - host: n1
+    data_dir: '/home/tidb6wu/tidb1-data/tikv-32160/log'
+
+tidb_servers:
+  - host: n1
+    log_dir: '/home/tidb6wu/tidb1-data/tikv-32160/log/data'
+`,
+		`
+tikv_servers:
+  - host: n1
+    log_dir: '/home/tidb6wu/tidb1-data/tikv-32160/log'
+
+tidb_servers:
+  - host: n1
+    log_dir: '/home/tidb6wu/tidb1-data/tikv-32160/log/log'
+`,
+		`
+tikv_servers:
+  - host: n1
+    data_dir: '/home/tidb6wu/tidb1-data/tikv-32160/data'
+
+pd_servers:
+  - host: n1
+    data_dir: '/home/tidb6wu/tidb1-data/tikv-32160'
+`,
+		`
+global:
+  user: "test1"
+  deploy_dir: deploy
+  data_dir: data
+tikv_servers:
+  - host: n1
+    log_dir: "/home/test1/deploy/tikv-20160/ddd/log"
+    data_dir: "ddd"
+`,
+		`
+global:
+  user: "test1"
+  deploy_dir: deploy
+  data_dir: data
+tikv_servers:
+  - host: n1
+    log_dir: "log"
+    data_dir: "/home/test1/deploy/tikv-20160/log/data"
+`,
+	}
+
+	for _, s := range overlapTopos {
+		err = yaml.Unmarshal([]byte(s), &topo)
+		c.Assert(err, IsNil)
+		err = CheckClusterDirConflict(clsList, "topo", &topo)
+		c.Assert(err, NotNil)
+		c.Assert(err.Error(), Equals, "spec.deploy.dir_overlap: Deploy directory overlaps to another instance")
+	}
+}

pkg/utils/ioutil.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strings"
 
 	"github.com/otiai10/copy"
 	"github.com/pingcap/errors"
@@ -70,6 +71,20 @@ func IsExecBinary(path string) bool {
 	return !info.IsDir() && info.Mode()&0111 == 0111
 }
 
+// IsSubDir returns if sub is a sub directory of parent
+func IsSubDir(parent, sub string) bool {
+	up := ".." + string(os.PathSeparator)
+
+	rel, err := filepath.Rel(parent, sub)
+	if err != nil {
+		return false
+	}
+	if !strings.HasPrefix(rel, up) && rel != ".." {
+		return true
+	}
+	return false
+}
+
 // Untar decompresses the tarball
 func Untar(reader io.Reader, to string) error {
 	gr, err := gzip.NewReader(reader)