Skip to content

Support tikv label check#800

Merged
lucklove merged 20 commits intopingcap:masterfrom
lucklove:tikv-label
Sep 27, 2020
Merged

Support tikv label check#800
lucklove merged 20 commits intopingcap:masterfrom
lucklove:tikv-label

Conversation

@lucklove
Copy link
Copy Markdown
Member

@lucklove lucklove commented Sep 21, 2020

What problem does this PR solve?

Check if tikv's label is set correctly: fix #787

What is changed and how it works?

  • apply check on deploy
  • apply check on scale-out
  • apply check on display
  • add test

Check List

Tests

  • Unit test
  • Manual test

Deploy

If both TiKV labels and PD labels not set:

屏幕快照 2020-09-22 下午4 57 31

If TiVK labels are set but PD's not:

屏幕快照 2020-09-22 下午4 59 01

Display

If both TiKV labels and PD labels not set:

屏幕快照 2020-09-22 下午6 42 55

If TiVK labels are set but PD's not:

In this case, TiKV will refuse to start

[2020/09/22 18:30:07.660 +08:00] [FATAL] [server.rs:576] ["failed to start node: Grpc(RpcFailure(RpcStatus { status: 2-UNKNOWN, details: Some(\"key matching the label was not found in the PD, store label key: host \") }))"]

ScaleOut

The result is the same with deploy

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

Release notes:

Automaticlly check if TiKV's label is set

Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
@lucklove lucklove marked this pull request as draft September 21, 2020 13:08
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 21, 2020

Codecov Report

Merging #800 into master will increase coverage by 4.07%.
The diff coverage is 59.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #800      +/-   ##
==========================================
+ Coverage   48.65%   52.73%   +4.07%     
==========================================
  Files         262      262              
  Lines       18850    18958     +108     
==========================================
+ Hits         9172     9998     +826     
+ Misses       8221     7403     -818     
- Partials     1457     1557     +100     
Flag Coverage Δ
#cluster 44.70% <33.85%> (+5.67%) ⬆️
#dm 25.15% <3.20%> (-0.19%) ⬇️
#integrate 47.75% <33.85%> (+3.97%) ⬆️
#playground 22.19% <0.00%> (-0.03%) ⬇️
#tiup 10.84% <0.00%> (-0.09%) ⬇️
#unittest 18.82% <49.16%> (+0.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/cluster/manager.go 66.27% <34.21%> (+6.57%) ⬆️
pkg/cluster/api/pdapi.go 54.19% <42.85%> (+2.37%) ⬆️
pkg/cluster/spec/tikv.go 55.61% <60.00%> (+0.23%) ⬆️
pkg/cluster/spec/validate.go 93.50% <60.00%> (-2.97%) ⬇️
pkg/cluster/spec/spec.go 89.38% <84.61%> (+3.18%) ⬆️
pkg/cluster/spec/server_config.go 65.07% <86.36%> (+5.61%) ⬆️
components/cluster/command/deploy.go 74.00% <100.00%> (+0.53%) ⬆️
components/cluster/command/scale_out.go 70.21% <100.00%> (+0.64%) ⬆️
pkg/cluster/spec/tispark.go 67.39% <0.00%> (+1.08%) ⬆️
pkg/cluster/operation/action.go 51.51% <0.00%> (+1.86%) ⬆️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfab8de...86b9bb5. Read the comment docs.

Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
@lonng lonng added the type/new-feature Categorizes pr as related to a new feature. label Sep 22, 2020
@lucklove lucklove marked this pull request as ready for review September 22, 2020 10:44
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Comment thread pkg/cluster/manager.go Outdated
Comment thread pkg/cluster/manager.go
Comment thread pkg/cluster/spec/spec.go Outdated
Comment thread pkg/cluster/spec/tikv.go Outdated
Comment thread pkg/cluster/spec/validate.go
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Comment thread pkg/cluster/api/pdapi.go Outdated
Comment thread pkg/cluster/spec/spec.go Outdated
Comment thread pkg/cluster/spec/validate.go Outdated
Comment thread pkg/cluster/spec/validate.go
Copy link
Copy Markdown
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

Co-authored-by: Lonng <heng@lonng.org>
lucklove and others added 4 commits September 24, 2020 17:28
Signed-off-by: lucklove <gnu.crazier@gmail.com>
Signed-off-by: lucklove <gnu.crazier@gmail.com>
@lonng lonng added this to the v1.1.3 milestone Sep 25, 2020
Copy link
Copy Markdown
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 25, 2020
@lucklove lucklove removed this from the v1.1.3 milestone Sep 25, 2020
@lonng lonng added this to the v1.2.0 milestone Sep 26, 2020
@lonng lonng removed the status/PTAL label Sep 26, 2020
@lonng
Copy link
Copy Markdown
Contributor

lonng commented Sep 26, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 26, 2020
@ti-srebot
Copy link
Copy Markdown
Contributor

Your auto merge job has been accepted, waiting for:

  • 817
  • 816
  • 815

@ti-srebot
Copy link
Copy Markdown
Contributor

/run-all-tests

@ti-srebot
Copy link
Copy Markdown
Contributor

@lucklove merge failed.

@lucklove lucklove merged commit 34e3e2d into pingcap:master Sep 27, 2020
@lucklove lucklove deleted the tikv-label branch September 27, 2020 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/new-feature Categorizes pr as related to a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add host label at deploy stage by default

4 participants