add store labels #3784

L3T · 2021-02-05T09:55:45Z

What problem does this PR solve?

Fix #3783

What is changed and how does it work?

Code changes

Has Go code change
Has CI related scripts change

Tests

Unit test
E2E test
Manual test
No code

Side effects

Breaking backward compatibility
Other side effects:

Related changes

Need to cherry-pick to the release branch
Need to update the documentation

Release Notes

Please refer to Release Notes Language Style Guide before writing the release note.

Support setting customized store labels according to the node labels

pkg/apis/pingcap/v1alpha1/types.go

pkg/manager/member/tikv_member_manager.go

pkg/apis/pingcap/v1alpha1/types.go

Co-authored-by: Lonng <heng@lonng.org>

DanielZhangQD · 2021-02-24T06:42:18Z

@L3T Could you please fix the CI and update the test result in the PR description?

codecov-io · 2021-02-24T07:58:30Z

Codecov Report

Merging #3784 (cd5caca) into master (ff03e86) will not change coverage.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##           master    #3784   +/-   ##
=======================================
  Coverage   62.35%   62.35%           
=======================================
  Files         169      169           
  Lines       17962    17962           
=======================================
  Hits        11200    11200           
  Misses       5677     5677           
  Partials     1085     1085

Flag	Coverage Δ
unittest	`62.35% <66.66%> (?)`

DanielZhangQD · 2021-02-24T10:42:08Z

/run-all-tests

DanielZhangQD · 2021-02-24T10:42:24Z

@L3T Please fix the CI.

DanielZhangQD · 2021-02-24T11:58:58Z

/run-all-tests

DanielZhangQD · 2021-02-24T12:09:21Z

/run-all-tests

L3T · 2021-02-24T13:09:27Z

手动进行测试，更新 crd 以及 operator 版本到本分支
为 tikv 添加
storeLabels:
- testLabel
观察 operator 日志

为 tikv-0 tikv-1 所在节点添加 testLabel:x label
观察operator 日志

观察 store 状态

观察operator确认设置成功后没有重复设置

DanielZhangQD · 2021-02-24T13:35:06Z

@L3T There is not location labels in the test result, could you please verify the case including the location labels and customized labels?

DanielZhangQD

LGTM

lonng · 2021-02-24T14:25:50Z

pkg/manager/member/tikv_member_manager.go

@@ -816,8 +816,8 @@ func (m *tikvMemberManager) setStoreLabelsForTiKV(tc *v1alpha1.TidbCluster) (int
 		return setCount, err
 	}

-	locationLabels := []string(config.Replication.LocationLabels)
-	if locationLabels == nil {
+	storeLabels := append(config.Replication.LocationLabels, tc.Spec.TiKV.StoreLabels...)


What if there are some duplicated labels?

这里重复的话，下面逻辑都是根据 storeLabels 对 map 赋值，所以对下面逻辑不会有影响，改了还要多几行代码(，这里就不改了吧(

L3T · 2021-02-25T06:37:29Z

添加 host location_labels

观察 store label

能争取设置 store labels 和 location labels

lonng · 2021-02-25T07:03:00Z

/lgtm

ti-chi-bot · 2021-02-25T07:03:03Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

DanielZhangQD
lonng

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

DanielZhangQD · 2021-02-25T07:03:03Z

/run-all-tests

DanielZhangQD · 2021-02-25T07:08:34Z

/merge

ti-chi-bot · 2021-02-25T07:08:36Z

This pull request has been accepted and is ready to merge.

Commit hash: cd5caca

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2021-02-25T08:16:48Z

cherry pick to release-1.1 in PR #3813

* cherry pick #3784 to release-1.1 Signed-off-by: ti-srebot <ti-srebot@pingcap.com> * resolve conflict Co-authored-by: zyy <zhangyunyu@zhihu.com> Co-authored-by: DanielZhangQD <zhanghailong810@aliyun.com> Co-authored-by: Ti Chi Robot <71242396+ti-chi-bot@users.noreply.github.com>

add store labels

2b2c767

ti-chi-bot requested a review from AstroProfundis February 5, 2021 09:55

lonng reviewed Feb 18, 2021

View reviewed changes

pkg/apis/pingcap/v1alpha1/types.go Outdated Show resolved Hide resolved

lonng reviewed Feb 18, 2021

View reviewed changes

pkg/manager/member/tikv_member_manager.go Outdated Show resolved Hide resolved

lonng reviewed Feb 18, 2021

View reviewed changes

pkg/apis/pingcap/v1alpha1/types.go Outdated Show resolved Hide resolved

L3T and others added 4 commits February 23, 2021 16:12

Update pkg/apis/pingcap/v1alpha1/types.go

0b52008

Co-authored-by: Lonng <heng@lonng.org>

Update pkg/manager/member/tikv_member_manager.go

34f9c2b

Co-authored-by: Lonng <heng@lonng.org>

Update pkg/apis/pingcap/v1alpha1/types.go

9f7ca6f

Co-authored-by: Lonng <heng@lonng.org>

Merge branch 'master' into store_labels

3575792

fix var

f4b449f

fix

f4fc0bf

DanielZhangQD added the needs-cherry-pick-1.1 label Feb 24, 2021

generate code

0571921

Merge branch 'master' into store_labels

e04ffe8

DanielZhangQD approved these changes Feb 24, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 label Feb 24, 2021

lonng reviewed Feb 24, 2021

View reviewed changes

Merge branch 'master' into store_labels

cd5caca

ti-chi-bot added status/LGT2 and removed status/LGT1 labels Feb 25, 2021

ti-chi-bot added the status/can-merge label Feb 25, 2021

ti-chi-bot merged commit a3b0e6c into pingcap:master Feb 25, 2021

ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Feb 25, 2021

cherry pick pingcap#3784 to release-1.1

eb70506

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Feb 25, 2021

add store labels (#3784) #3813

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add store labels #3784

add store labels #3784

L3T commented Feb 5, 2021 •

edited

DanielZhangQD commented Feb 24, 2021

codecov-io commented Feb 24, 2021 •

edited

DanielZhangQD commented Feb 24, 2021

DanielZhangQD commented Feb 24, 2021

DanielZhangQD commented Feb 24, 2021

DanielZhangQD commented Feb 24, 2021

L3T commented Feb 24, 2021 •

edited

DanielZhangQD commented Feb 24, 2021

DanielZhangQD left a comment

lonng Feb 24, 2021

L3T Feb 25, 2021

L3T commented Feb 25, 2021

lonng commented Feb 25, 2021

ti-chi-bot commented Feb 25, 2021

DanielZhangQD commented Feb 25, 2021

DanielZhangQD commented Feb 25, 2021

ti-chi-bot commented Feb 25, 2021

ti-srebot commented Feb 25, 2021

add store labels #3784

add store labels #3784

Conversation

L3T commented Feb 5, 2021 • edited

What problem does this PR solve?

What is changed and how does it work?

Code changes

Tests

Side effects

Related changes

Release Notes

DanielZhangQD commented Feb 24, 2021

codecov-io commented Feb 24, 2021 • edited

Codecov Report

DanielZhangQD commented Feb 24, 2021

DanielZhangQD commented Feb 24, 2021

DanielZhangQD commented Feb 24, 2021

DanielZhangQD commented Feb 24, 2021

L3T commented Feb 24, 2021 • edited

DanielZhangQD commented Feb 24, 2021

DanielZhangQD left a comment

Choose a reason for hiding this comment

lonng Feb 24, 2021

Choose a reason for hiding this comment

L3T Feb 25, 2021

Choose a reason for hiding this comment

L3T commented Feb 25, 2021

lonng commented Feb 25, 2021

ti-chi-bot commented Feb 25, 2021

DanielZhangQD commented Feb 25, 2021

DanielZhangQD commented Feb 25, 2021

ti-chi-bot commented Feb 25, 2021

ti-srebot commented Feb 25, 2021

L3T commented Feb 5, 2021 •

edited

codecov-io commented Feb 24, 2021 •

edited

L3T commented Feb 24, 2021 •

edited