From 4bf823647f0daf57c0d0a438884e7136f0b0c9e9 Mon Sep 17 00:00:00 2001 From: SIGSEGV Date: Fri, 30 Oct 2020 16:26:35 +0800 Subject: [PATCH] Fix labels warning (#869) --- pkg/cluster/api/pdapi.go | 39 +++++++++++++------------------ pkg/cluster/manager.go | 6 ++--- pkg/cluster/spec/spec.go | 31 +++++++----------------- pkg/cluster/spec/validate.go | 22 +++++++---------- pkg/cluster/spec/validate_test.go | 12 +++++----- 5 files changed, 42 insertions(+), 68 deletions(-) diff --git a/pkg/cluster/api/pdapi.go b/pkg/cluster/api/pdapi.go index 62f01280fe..e107d5c26e 100644 --- a/pkg/cluster/api/pdapi.go +++ b/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 diff --git a/pkg/cluster/manager.go b/pkg/cluster/manager.go index 9988f46c45..0066aaa5b0 100644 --- a/pkg/cluster/manager.go +++ b/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) } } diff --git a/pkg/cluster/spec/spec.go b/pkg/cluster/spec/spec.go index 44cffd54b0..ec7452d84a 100644 --- a/pkg/cluster/spec/spec.go +++ b/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. diff --git a/pkg/cluster/spec/validate.go b/pkg/cluster/spec/validate.go index 41265df75f..20ed32da0c 100644 --- a/pkg/cluster/spec/validate.go +++ b/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], diff --git a/pkg/cluster/spec/validate_test.go b/pkg/cluster/spec/validate_test.go index 2c5b3f590a..bc1b55445d 100644 --- a/pkg/cluster/spec/validate_test.go +++ b/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) }