cluster/spec: add test cases for cross cluster conflict check

pingcap · Jul 31, 2020 · ca34913 · ca34913
1 parent e2b54e2
commit ca34913
Show file tree

Hide file tree

Showing 5 changed files with 566 additions and 326 deletions.
diff --git a/pkg/cluster/spec/spec_manager.go b/pkg/cluster/spec/spec_manager.go
@@ -174,7 +174,8 @@ func (s *SpecManager) List() (names []string, err error) {
 }
 
 // GetAllClusters get a metadata list of all clusters deployed by current user
-func (s *SpecManager) GetAllClusters() (clusters map[string]Metadata, err error) {
+func (s *SpecManager) GetAllClusters() (map[string]Metadata, error) {
+	clusters := make(map[string]Metadata)
 	names, err := s.List()
 	if err != nil {
 		return nil, errors.AddStack(err)
@@ -187,7 +188,7 @@ func (s *SpecManager) GetAllClusters() (clusters map[string]Metadata, err error)
 		}
 		clusters[name] = metadata
 	}
-	return
+	return clusters, nil
 }
 
 // ensureDir ensures that the cluster directory exists.

diff --git a/pkg/cluster/spec/spec_manager_test.go b/pkg/cluster/spec/spec_manager_test.go
@@ -161,6 +161,11 @@ func TestSpec(t *testing.T) {
 	assert.Nil(t, err)
 	assert.True(t, exist)
 
+	specList, err := spec.GetAllClusters()
+	assert.Nil(t, err)
+	assert.Equal(t, meta1, specList["name1"])
+	assert.Equal(t, meta2, specList["name2"])
+
 	// remove name1 and check again.
 	err = spec.Remove("name1")
 	assert.Nil(t, err)

diff --git a/pkg/cluster/spec/spec_test.go b/pkg/cluster/spec/spec_test.go
@@ -114,261 +114,6 @@ pd_servers:
 	c.Assert(topo.PDServers[1].DataDir, Equals, "/test-data/pd-12379")
 }
 
-func (s *metaSuiteTopo) TestDirectoryConflicts1(c *C) {
-	topo := Specification{}
-
-	err := yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: "test-deploy"
-  data_dir: "test-data"
-tidb_servers:
-  - host: 172.16.5.138
-    deploy_dir: "/test-1"
-pd_servers:
-  - host: 172.16.5.138
-    data_dir: "/test-1"
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "directory conflict for '/test-1' between 'tidb_servers:172.16.5.138.deploy_dir' and 'pd_servers:172.16.5.138.data_dir'")
-
-	err = yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: "test-deploy"
-  data_dir: "/test-data"
-tikv_servers:
-  - host: 172.16.5.138
-    data_dir: "test-1"
-pd_servers:
-  - host: 172.16.5.138
-    data_dir: "test-1"
-`), &topo)
-	c.Assert(err, IsNil)
-
-	// report conflict if a non-import node use same dir as an imported one
-	err = yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: deploy
-  data_dir: data
-tidb_servers:
-  - host: 172.16.4.190
-    deploy_dir: /home/tidb/deploy
-pd_servers:
-  - host: 172.16.4.190
-    imported: true
-    name: pd_ip-172-16-4-190
-    deploy_dir: /home/tidb/deploy
-    data_dir: /home/tidb/deploy/data.pd
-    log_dir: /home/tidb/deploy/log
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "directory conflict for '/home/tidb/deploy' between 'tidb_servers:172.16.4.190.deploy_dir' and 'pd_servers:172.16.4.190.deploy_dir'")
-
-	// two imported tidb pass the validation, two pd servers (only one is imported) don't
-	err = yaml.Unmarshal([]byte(`
-global:
-  user: "test2"
-  ssh_port: 220
-  deploy_dir: deploy
-  data_dir: /data
-tidb_servers:
-  - host: 172.16.4.190
-    imported: true
-    port: 3306
-    deploy_dir: /home/tidb/deploy1
-  - host: 172.16.4.190
-    imported: true
-    status_port: 3307
-    deploy_dir: /home/tidb/deploy1
-pd_servers:
-  - host: 172.16.4.190
-    imported: true
-    name: pd_ip-172-16-4-190
-    deploy_dir: /home/tidb/deploy
-  - host: 172.16.4.190
-    name: pd_ip-172-16-4-190-2
-    client_port: 2381
-    peer_port: 2382
-    deploy_dir: /home/tidb/deploy
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "directory conflict for '/home/tidb/deploy' between 'pd_servers:172.16.4.190.deploy_dir' and 'pd_servers:172.16.4.190.deploy_dir'")
-}
-
-func (s *metaSuiteTopo) TestPortConflicts(c *C) {
-	topo := Specification{}
-	err := yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: "test-deploy"
-  data_dir: "test-data" 
-tidb_servers:
-  - host: 172.16.5.138
-    port: 1234
-tikv_servers:
-  - host: 172.16.5.138
-    status_port: 1234
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "port conflict for '1234' between 'tidb_servers:172.16.5.138.port' and 'tikv_servers:172.16.5.138.status_port'")
-
-	topo = Specification{}
-	// tispark_masters has "omitempty" in its tag value
-	err = yaml.Unmarshal([]byte(`
-monitored:
-  node_exporter_port: 1234
-tispark_masters:
-  - host: 172.16.5.138
-    port: 1234
-tikv_servers:
-  - host: 172.16.5.138
-    status_port: 2345
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "port conflict for '1234' between 'tispark_masters:172.16.5.138.port' and 'monitored:172.16.5.138.node_exporter_port'")
-}
-
-func (s *metaSuiteTopo) TestPlatformConflicts(c *C) {
-	// aarch64 and arm64 are equal
-	topo := Specification{}
-	err := yaml.Unmarshal([]byte(`
-global:
-  os: "linux"
-  arch: "aarch64"
-tidb_servers:
-  - host: 172.16.5.138
-    arch: "arm64"
-tikv_servers:
-  - host: 172.16.5.138
-`), &topo)
-	c.Assert(err, IsNil)
-
-	// different arch defined for the same host
-	topo = Specification{}
-	err = yaml.Unmarshal([]byte(`
-global:
-  os: "linux"
-tidb_servers:
-  - host: 172.16.5.138
-    arch: "aarch64"
-tikv_servers:
-  - host: 172.16.5.138
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "platform mismatch for '172.16.5.138' between 'tidb_servers:linux/arm64' and 'tikv_servers:linux/amd64'")
-
-	// different os defined for the same host
-	topo = Specification{}
-	err = yaml.Unmarshal([]byte(`
-global:
-  os: "linux"
-  arch: "aarch64"
-tidb_servers:
-  - host: 172.16.5.138
-    os: "darwin"
-tikv_servers:
-  - host: 172.16.5.138
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "platform mismatch for '172.16.5.138' between 'tidb_servers:darwin/arm64' and 'tikv_servers:linux/arm64'")
-}
-
-func (s *metaSuiteTopo) TestCountDir(c *C) {
-	topo := Specification{}
-
-	err := yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: "test-deploy"
-tikv_servers:
-  - host: 172.16.5.138
-    data_dir: "/test-data/data-1"
-pd_servers:
-  - host: 172.16.5.138
-    data_dir: "/test-data/data-2"
-`), &topo)
-	c.Assert(err, IsNil)
-	cnt := topo.CountDir("172.16.5.138", "/home/test1/test-deploy/pd-2379")
-	c.Assert(cnt, Equals, 2)
-	cnt = topo.CountDir("172.16.5.138", "") // the default user home
-	c.Assert(cnt, Equals, 4)
-	cnt = topo.CountDir("172.16.5.138", "/test-data/data")
-	c.Assert(cnt, Equals, 0) // should not match partial path
-
-	err = yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: "/test-deploy"
-tikv_servers:
-  - host: 172.16.5.138
-    data_dir: "/test-data/data-1"
-pd_servers:
-  - host: 172.16.5.138
-    data_dir: "/test-data/data-2"
-`), &topo)
-	c.Assert(err, IsNil)
-	cnt = topo.CountDir("172.16.5.138", "/test-deploy/pd-2379")
-	c.Assert(cnt, Equals, 2)
-	cnt = topo.CountDir("172.16.5.138", "")
-	c.Assert(cnt, Equals, 0)
-	cnt = topo.CountDir("172.16.5.138", "test-data")
-	c.Assert(cnt, Equals, 0)
-
-	err = yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: "/test-deploy"
-  data_dir: "/test-data"
-tikv_servers:
-  - host: 172.16.5.138
-    data_dir: "data-1"
-pd_servers:
-  - host: 172.16.5.138
-    data_dir: "data-2"
-  - host: 172.16.5.139
-`), &topo)
-	c.Assert(err, IsNil)
-	// if per-instance data_dir is set, the global data_dir is ignored, and if it
-	// is a relative path, it will be under the instance's deploy_dir
-	cnt = topo.CountDir("172.16.5.138", "/test-deploy/pd-2379")
-	c.Assert(cnt, Equals, 3)
-	cnt = topo.CountDir("172.16.5.138", "")
-	c.Assert(cnt, Equals, 0)
-	cnt = topo.CountDir("172.16.5.139", "/test-data")
-	c.Assert(cnt, Equals, 1)
-
-	err = yaml.Unmarshal([]byte(`
-global:
-  user: "test1"
-  ssh_port: 220
-  deploy_dir: deploy
-  data_dir: data
-tidb_servers:
-  - host: 172.16.4.190
-    imported: true
-    deploy_dir: /home/tidb/deploy
-pd_servers:
-  - host: 172.16.4.190
-    imported: true
-    name: pd_ip-172-16-4-190
-    deploy_dir: /home/tidb/deploy
-    data_dir: /home/tidb/deploy/data.pd
-    log_dir: /home/tidb/deploy/log
-`), &topo)
-	c.Assert(err, IsNil)
-	cnt = topo.CountDir("172.16.4.190", "/home/tidb/deploy")
-	c.Assert(cnt, Equals, 5)
-}
-
 func (s *metaSuiteTopo) TestGlobalConfig(c *C) {
 	topo := Specification{}
 	err := yaml.Unmarshal([]byte(`
@@ -726,68 +471,3 @@ item7 = 700
 	c.Assert(err, IsNil)
 	c.Assert(string(merge2), DeepEquals, expected)
 }
-
-func (s *metaSuiteTopo) TestTiSparkSpecValidation(c *C) {
-	topo := Specification{}
-	err := yaml.Unmarshal([]byte(`
-pd_servers:
-  - host: 172.16.5.138
-    port: 1234
-tispark_masters:
-  - host: 172.16.5.138
-    port: 1235
-tispark_workers:
-  - host: 172.16.5.138
-    port: 1236
-  - host: 172.16.5.139
-    port: 1235
-`), &topo)
-	c.Assert(err, IsNil)
-
-	topo = Specification{}
-	err = yaml.Unmarshal([]byte(`
-pd_servers:
-  - host: 172.16.5.138
-    port: 1234
-tispark_masters:
-  - host: 172.16.5.138
-    port: 1235
-  - host: 172.16.5.139
-    port: 1235
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "a TiSpark enabled cluster with more than 1 Spark master node is not supported")
-
-	topo = Specification{}
-	err = yaml.Unmarshal([]byte(`
-pd_servers:
-  - host: 172.16.5.138
-    port: 1234
-tispark_workers:
-  - host: 172.16.5.138
-    port: 1235
-  - host: 172.16.5.139
-    port: 1235
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "there must be a Spark master node if you want to use the TiSpark component")
-
-	err = yaml.Unmarshal([]byte(`
-pd_servers:
-  - host: 172.16.5.138
-    port: 1234
-tispark_masters:
-  - host: 172.16.5.138
-    port: 1236
-tispark_workers:
-  - host: 172.16.5.138
-    port: 1235
-  - host: 172.16.5.139
-    port: 1235
-  - host: 172.16.5.139
-    port: 1236
-    web_port: 8089
-`), &topo)
-	c.Assert(err, NotNil)
-	c.Assert(err.Error(), Equals, "the host 172.16.5.139 is duplicated: multiple TiSpark workers on the same host is not supported by Spark")
-}
diff --git a/pkg/cluster/spec/validate.go b/pkg/cluster/spec/validate.go
@@ -214,8 +214,12 @@ func CheckClusterPortConflict(clusterList map[string]Metadata, clusterName strin
 
 		uniqueHosts := set.NewStringSet()
 		metadata.GetTopology().IterInstance(func(inst Instance) {
-			nodeExporterPort := metadata.GetTopology().GetMonitoredOptions().NodeExporterPort
-			blackboxExporterPort := metadata.GetTopology().GetMonitoredOptions().BlackboxExporterPort
+			mOpt := metadata.GetTopology().GetMonitoredOptions()
+			if mOpt == nil {
+				return
+			}
+			nodeExporterPort := mOpt.NodeExporterPort
+			blackboxExporterPort := mOpt.BlackboxExporterPort
 			for _, port := range inst.UsedPorts() {
 				existingEntries = append(existingEntries, Entry{
 					clusterName: name,
@@ -249,16 +253,20 @@ func CheckClusterPortConflict(clusterList map[string]Metadata, clusterName strin
 			})
 		}
 
+		mOpt := topo.GetMonitoredOptions()
+		if mOpt == nil {
+			return
+		}
 		if !uniqueHosts.Exist(inst.GetHost()) {
 			uniqueHosts.Insert(inst.GetHost())
 			currentEntries = append(currentEntries,
 				Entry{
 					instance: inst,
-					port:     topo.GetMonitoredOptions().NodeExporterPort,
+					port:     mOpt.NodeExporterPort,
 				},
 				Entry{
 					instance: inst,
-					port:     topo.GetMonitoredOptions().BlackboxExporterPort,
+					port:     mOpt.BlackboxExporterPort,
 				})
 		}
 	})