Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cluster: fix custom port handling for drainer on importing #806

Merged
merged 2 commits into from
Sep 23, 2020

Conversation

AstroProfundis
Copy link
Contributor

What problem does this PR solve?

The custom drainer_port for host is not read on import, so the value in group_vars is used instead.

Check List

Tests

  • Unit test

Code changes

  • Has persistent data change

Side effects

  • None

Related changes

  • Need to cherry-pick to the release branch

Release notes:

Fix per-host custom port for drainer not handled correctly on importing

@AstroProfundis AstroProfundis added the type/bug-fix Categorizes PR as a bug-fix label Sep 23, 2020
@AstroProfundis AstroProfundis self-assigned this Sep 23, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2020

Codecov Report

Merging #806 into master will decrease coverage by 2.61%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #806      +/-   ##
==========================================
- Coverage   51.32%   48.70%   -2.62%     
==========================================
  Files         262      262              
  Lines       18844    18850       +6     
==========================================
- Hits         9671     9181     -490     
- Misses       7629     8210     +581     
+ Partials     1544     1459      -85     
Flag Coverage Δ
#cluster 39.09% <0.00%> (-3.36%) ⬇️
#dm 25.33% <0.00%> (-0.08%) ⬇️
#integrate 43.82% <0.00%> (-2.26%) ⬇️
#playground 22.22% <ø> (ø)
#tiup 10.93% <ø> (ø)
#unittest 18.56% <33.33%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
pkg/cluster/ansible/service.go 3.64% <0.00%> (ø)
pkg/cluster/ansible/inventory.go 54.16% <100.00%> (+0.94%) ⬆️
components/cluster/command/check.go 6.39% <0.00%> (-72.73%) ⬇️
pkg/cluster/task/limits.go 0.00% <0.00%> (-68.75%) ⬇️
pkg/cluster/task/sysctl.go 0.00% <0.00%> (-66.67%) ⬇️
components/cluster/command/audit.go 27.27% <0.00%> (-54.55%) ⬇️
pkg/cluster/operation/check.go 0.00% <0.00%> (-51.95%) ⬇️
pkg/cluster/task/rmdir.go 0.00% <0.00%> (-50.00%) ⬇️
pkg/cluster/operation/operation.go 34.78% <0.00%> (-43.48%) ⬇️
components/cluster/command/rename.go 25.00% <0.00%> (-41.67%) ⬇️
... and 42 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 251998a...896d57f. Read the comment docs.

Copy link
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 23, 2020
@lonng lonng merged commit ab2232a into pingcap:master Sep 23, 2020
@AstroProfundis AstroProfundis deleted the fix-import branch September 23, 2020 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT1 Indicates that a PR has LGTM 1. type/bug-fix Categorizes PR as a bug-fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants