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: mark patched nodes in display #1125

Merged
merged 7 commits into from
Feb 24, 2021

Conversation

AstroProfundis
Copy link
Contributor

@AstroProfundis AstroProfundis commented Feb 3, 2021

What problem does this PR solve?

Close #1037

图片

Release notes:

cluster: mark patched nodes in display output

@AstroProfundis AstroProfundis added type/new-feature Categorizes pr as related to a new feature. category/usability Categorizes issue or PR as a usability enhancement. labels Feb 3, 2021
@AstroProfundis AstroProfundis self-assigned this Feb 3, 2021
@ti-chi-bot ti-chi-bot requested review from july2993 and nrc February 3, 2021 06:43
@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 3, 2021
@codecov-io
Copy link

codecov-io commented Feb 3, 2021

Codecov Report

Merging #1125 (ee0a255) into master (4607799) will decrease coverage by 9.23%.
The diff coverage is 65.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1125      +/-   ##
==========================================
- Coverage   53.52%   44.29%   -9.24%     
==========================================
  Files         284      283       -1     
  Lines       20279    20265      -14     
==========================================
- Hits        10854     8976    -1878     
- Misses       7754     9902    +2148     
+ Partials     1671     1387     -284     
Flag Coverage Δ
cluster 36.40% <47.88%> (-8.61%) ⬇️
dm ?
integrate 38.86% <47.88%> (-9.03%) ⬇️
playground 2.93% <ø> (ø)
tiup 16.38% <0.00%> (+0.01%) ⬆️
unittest 22.84% <34.39%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
components/dm/spec/logic.go 0.60% <0.00%> (-79.27%) ⬇️
pkg/cluster/ansible/service.go 3.62% <0.00%> (ø)
pkg/cluster/manager/manager.go 69.23% <0.00%> (ø)
pkg/cluster/manager/patch.go 0.00% <0.00%> (-49.46%) ⬇️
pkg/cluster/manager/scale_out.go 50.00% <0.00%> (-0.77%) ⬇️
pkg/cluster/manager/transfer.go 18.66% <0.00%> (-37.34%) ⬇️
pkg/cluster/manager/upgrade.go 5.88% <0.00%> (-56.19%) ⬇️
components/cluster/command/test.go 40.35% <50.00%> (-19.30%) ⬇️
pkg/cluster/ansible/inventory.go 54.74% <50.00%> (ø)
pkg/cluster/manager/display.go 80.66% <50.00%> (-2.49%) ⬇️
... and 121 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 4607799...ee0a255. Read the comment docs.

@lucklove
Copy link
Member

lucklove commented Feb 4, 2021

It seems you haven't set Patched = true? how it works...

@AstroProfundis
Copy link
Contributor Author

/hold

@ti-chi-bot ti-chi-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 19, 2021
@ti-chi-bot ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 22, 2021
@AstroProfundis AstroProfundis removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 22, 2021
Copy link
Member

@lucklove lucklove left a comment

Choose a reason for hiding this comment

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

Not handle the case that scale-out and upgrade after patch

pkg/cluster/manager/patch.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2021
@lucklove
Copy link
Member

/lgtm

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • lucklove

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.

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 24, 2021
@lucklove
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: ee0a255

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 24, 2021
@ti-chi-bot ti-chi-bot merged commit e92af4f into pingcap:master Feb 24, 2021
@AstroProfundis AstroProfundis deleted the display-patched branch February 24, 2021 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/usability Categorizes issue or PR as a usability enhancement. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. 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.

Let tiup cluster display tell me which node has been patched.
4 participants