Fix labels warning (#869)

pkg/cluster/api/pdapi.go

@@ -677,37 +677,30 @@ func (pc *PDClient) GetLocationLabels() ([]string, error) {
 	return rc.LocationLabels, nil
 }
 
-// StoreList implements StoreLabelProvider
-func (pc *PDClient) StoreList() ([]string, error) {
-	r, err := pc.GetStores()
-	if err != nil {
-		return nil, err
-	}
-	addrs := []string{}
-	for _, s := range r.Stores {
-		addrs = append(addrs, s.Store.GetAddress())
-	}
-	return addrs, nil
-}
-
-// GetStoreLabels implements StoreLabelProvider
-func (pc *PDClient) GetStoreLabels(address string) (map[string]string, error) {
+// GetTiKVLabels implements TiKVLabelProvider
+func (pc *PDClient) GetTiKVLabels() (map[string]map[string]string, error) {
 	r, err := pc.GetStores()
 	if err != nil {
 		return nil, err
 	}
 
+	locationLabels := map[string]map[string]string{}
 	for _, s := range r.Stores {
-		if address == s.Store.GetAddress() {
-			lbs := s.Store.GetLabels()
-			labels := map[string]string{}
-			for _, lb := range lbs {
-				labels[lb.GetKey()] = lb.GetValue()
-			}
-			return labels, nil
+		if s.Store.StateName != "Up" {
+			continue
+		}
+		lbs := s.Store.GetLabels()
+		labels := map[string]string{}
+		for _, lb := range lbs {
+			labels[lb.GetKey()] = lb.GetValue()
+		}
+		// Skip tiflash
+		if labels["engine"] == "tiflash" {
+			continue
 		}
+		locationLabels[s.Store.GetAddress()] = labels
 	}
-	return nil, errors.Errorf("store %s not found", address)
+	return locationLabels, nil
 }
 
 // UpdateScheduleConfig updates the PD schedule config

pkg/cluster/manager.go

@@ -604,7 +604,7 @@ func (m *Manager) Display(clusterName string, opt operator.Options) error {
 		pdClient := api.NewPDClient(pdList, 10*time.Second, tlsCfg)
 		if lbs, err := pdClient.GetLocationLabels(); err != nil {
 			color.Yellow("\nWARN: get location labels from pd failed: %v", err)
-		} else if err := spec.CheckTiKVLocationLabels(lbs, pdClient); err != nil {
+		} else if err := spec.CheckTiKVLabels(lbs, pdClient); err != nil {
 			color.Yellow("\nWARN: there is something wrong with TiKV labels, which may cause data losing:\n%v", err)
 		}
 
@@ -1087,7 +1087,7 @@ func (m *Manager) Deploy(
 		if err != nil {
 			return err
 		}
-		if err := spec.CheckTiKVLocationLabels(lbs, topo); err != nil {
+		if err := spec.CheckTiKVLabels(lbs, topo); err != nil {
 			return perrs.Errorf("check TiKV label failed, please fix that before continue:\n%s", err)
 		}
 	}
@@ -1526,7 +1526,7 @@ func (m *Manager) ScaleOut(
 		if err != nil {
 			return err
 		}
-		if err := spec.CheckTiKVLocationLabels(lbs, mergedTopo.(*spec.Specification)); err != nil {
+		if err := spec.CheckTiKVLabels(lbs, mergedTopo.(*spec.Specification)); err != nil {
 			return perrs.Errorf("check TiKV label failed, please fix that before continue:\n%s", err)
 		}
 	}

pkg/cluster/spec/spec.go

@@ -237,33 +237,18 @@ func (s *Specification) LocationLabels() ([]string, error) {
 	return lbs, nil
 }
 
-// StoreList implements StoreLabelProvider
-func (s *Specification) StoreList() ([]string, error) {
+// GetTiKVLabels implements TiKVLabelProvider
+func (s *Specification) GetTiKVLabels() (map[string]map[string]string, error) {
 	kvs := s.TiKVServers
-	addrs := []string{}
+	locationLabels := map[string]map[string]string{}
 	for _, kv := range kvs {
-		if kv.IsImported() {
-			// FIXME: this function implements StoreLabelProvider, which is used to
-			//        detect if the label config is missing. However, we do that
-			//        base on the meta.yaml, whose server.labels field is empty
-			//        for imported TiKV servers, to workaround that, we just skip the
-			//        imported TiKV servers at this time.
-			continue
+		address := fmt.Sprintf("%s:%d", kv.Host, kv.GetMainPort())
+		var err error
+		if locationLabels[address], err = kv.Labels(); err != nil {
+			return nil, err
 		}
-		addrs = append(addrs, fmt.Sprintf("%s:%d", kv.Host, kv.GetMainPort()))
 	}
-	return addrs, nil
-}
-
-// GetStoreLabels implements StoreLabelProvider
-func (s *Specification) GetStoreLabels(address string) (map[string]string, error) {
-	kvs := s.TiKVServers
-	for _, kv := range kvs {
-		if address == fmt.Sprintf("%s:%d", kv.Host, kv.GetMainPort()) {
-			return kv.Labels()
-		}
-	}
-	return nil, errors.Errorf("store %s not found", address)
+	return locationLabels, nil
 }
 
 // AllComponentNames contains the names of all components.

pkg/cluster/spec/validate.go

@@ -339,38 +339,34 @@ func (e *TiKVLabelError) Error() string {
 	return str
 }
 
-// StoreLabelProvider provide store labels information
-type StoreLabelProvider interface {
-	StoreList() ([]string, error)
-	GetStoreLabels(address string) (map[string]string, error)
+// TiKVLabelProvider provide store labels information
+type TiKVLabelProvider interface {
+	GetTiKVLabels() (map[string]map[string]string, error)
 }
 
 func getHostFromAddress(addr string) string {
 	return strings.Split(addr, ":")[0]
 }
 
-// CheckTiKVLocationLabels will check if tikv missing label or have wrong label
-func CheckTiKVLocationLabels(pdLocLabels []string, slp StoreLabelProvider) error {
+// CheckTiKVLabels will check if tikv missing label or have wrong label
+func CheckTiKVLabels(pdLocLabels []string, slp TiKVLabelProvider) error {
 	lerr := &TiKVLabelError{
 		TiKVInstances: make(map[string][]error),
 	}
 	lbs := set.NewStringSet(pdLocLabels...)
 	hosts := make(map[string]int)
 
-	kvs, err := slp.StoreList()
+	storeLabels, err := slp.GetTiKVLabels()
 	if err != nil {
 		return err
 	}
-	for _, kv := range kvs {
+	for kv := range storeLabels {
 		host := getHostFromAddress(kv)
 		hosts[host] = hosts[host] + 1
 	}
-	for _, kv := range kvs {
+
+	for kv, ls := range storeLabels {
 		host := getHostFromAddress(kv)
-		ls, err := slp.GetStoreLabels(kv)
-		if err != nil {
-			return err
-		}
 		if len(ls) == 0 && hosts[host] > 1 {
 			lerr.TiKVInstances[kv] = append(
 				lerr.TiKVInstances[kv],

pkg/cluster/spec/validate_test.go

@@ -620,9 +620,9 @@ tikv_servers:
     status_port: 20180
 `), &topo)
 	c.Assert(err, IsNil)
-	err = CheckTiKVLocationLabels(nil, &topo)
+	err = CheckTiKVLabels(nil, &topo)
 	c.Assert(err, IsNil)
-	err = CheckTiKVLocationLabels([]string{}, &topo)
+	err = CheckTiKVLabels([]string{}, &topo)
 	c.Assert(err, IsNil)
 
 	// 2 tikv on the same host without label
@@ -637,7 +637,7 @@ tikv_servers:
     status_port: 20181
 `), &topo)
 	c.Assert(err, IsNil)
-	err = CheckTiKVLocationLabels(nil, &topo)
+	err = CheckTiKVLabels(nil, &topo)
 	c.Assert(err, NotNil)
 
 	// 2 tikv on the same host with unacquainted label
@@ -656,7 +656,7 @@ tikv_servers:
       server.labels: { zone: "zone1", host: "172.16.5.140" }
 `), &topo)
 	c.Assert(err, IsNil)
-	err = CheckTiKVLocationLabels(nil, &topo)
+	err = CheckTiKVLabels(nil, &topo)
 	c.Assert(err, NotNil)
 
 	// 2 tikv on the same host with correct label
@@ -675,7 +675,7 @@ tikv_servers:
       server.labels: { zone: "zone1", host: "172.16.5.140" }
 `), &topo)
 	c.Assert(err, IsNil)
-	err = CheckTiKVLocationLabels([]string{"zone", "host"}, &topo)
+	err = CheckTiKVLabels([]string{"zone", "host"}, &topo)
 	c.Assert(err, IsNil)
 
 	// 2 tikv on the same host with diffrent config style
@@ -697,6 +697,6 @@ tikv_servers:
         host: "172.16.5.140"
 `), &topo)
 	c.Assert(err, IsNil)
-	err = CheckTiKVLocationLabels([]string{"zone", "host"}, &topo)
+	err = CheckTiKVLabels([]string{"zone", "host"}, &topo)
 	c.Assert(err, IsNil)
 }