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.

Sign up for GitHub

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

Jump to bottom

Extract check cmd to pkg #1215

Merged
merged 7 commits into from
Mar 17, 2021
Merged

Extract check cmd to pkg #1215

merged 7 commits into from
Mar 17, 2021

Conversation

baurine
Copy link
Contributor

@baurine baurine commented Mar 16, 2021 •
edited
Loading

What problem does this PR solve?

Similar to PR #1139 , this PR is used to support PR #1172 (tiup web UI), so we can show the check result in the web UI without parsing the command line output.

WeCom20210316-174455@2x

What is changed and how it works?

Extract the tiup cluster check interface to the pkg

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

Release notes:

NONE

Manual test:

Output by this PR:

> bin/tiup-cluster check multi-host --cluster
+ Download necessary tools
  - Downloading check tools for linux/amd64 ... Done
+ Collect basic system information
+ Collect basic system information
+ Collect basic system information
+ Collect basic system information
+ Collect basic system information
  - Getting system info of 10.0.1.24:22 ... Done
  - Getting system info of 10.0.1.21:22 ... Done
  - Getting system info of 10.0.1.22:22 ... Done
  - Getting system info of 10.0.1.23:22 ... Done
+ Check system requirements
+ Check system requirements
  - Checking node 10.0.1.24 ... ⠼ Shell: host=10.0.1.24, sudo=false, command=`ss -lnt`
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
  - Checking node 10.0.1.24 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.22 ... Done
  - Checking node 10.0.1.23 ... Done
  - Checking node 10.0.1.22 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.22 ... Done
  - Checking node 10.0.1.23 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.21 ... Done
+ Cleanup check files
  - Cleanup check files on 10.0.1.24:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.22:22 ... Done
  - Cleanup check files on 10.0.1.23:22 ... Done
  - Cleanup check files on 10.0.1.22:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.22:22 ... Done
  - Cleanup check files on 10.0.1.23:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
Node       Check           Result  Message
----       -----           ------  -------
10.0.1.22  permission      Pass    /home/tidb/tidb-deploy/tidb-4000 is writable
10.0.1.22  permission      Pass    /home/tidb/tidb-deploy/pd-2379 is writable
10.0.1.22  permission      Pass    /home/tidb/tidb-deploy/tikv-20160 is writable
10.0.1.22  permission      Pass    /home/tidb/tidb-deploy/tikv-20160/tidb-data is writable
10.0.1.22  permission      Pass    /home/tidb/tidb-deploy/pd-2379/tidb-data is writable
10.0.1.22  os-version      Pass    OS is Ubuntu 18.04.3 LTS 18.04.3
10.0.1.22  cpu-cores       Pass    number of CPU cores / threads: 1
10.0.1.22  swap            Fail    swap is enabled, please disable it for best performance
10.0.1.22  memory          Pass    memory size is 0MB
10.0.1.22  network         Fail    network speed of eth0 is 1000MB too low, needs 1GB or more
10.0.1.22  network         Fail    network speed of eth1 is 1000MB too low, needs 1GB or more
10.0.1.22  disk            Fail    mount point / does not have 'nodelalloc' option set
...

Output by code in origin/master:

> bin/tiup-cluster check multi-host --cluster
+ Download necessary tools
  - Downloading check tools for linux/amd64 ... Done
+ Collect basic system information
+ Collect basic system information
  - Getting system info of 10.0.1.24:22 ... ⠋ CopyComponent: component=insight, version=, remote=10.0.1.24:/tmp/tiup os=linux, arch=amd64
+ Collect basic system information
+ Collect basic system information
  - Getting system info of 10.0.1.24:22 ... Done
  - Getting system info of 10.0.1.21:22 ... Done
  - Getting system info of 10.0.1.22:22 ... Done
  - Getting system info of 10.0.1.23:22 ... Done
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
  - Checking node 10.0.1.24 ... ⠋ Shell: host=10.0.1.24, sudo=true, command=`sysctl -a`
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
+ Check system requirements
  - Checking node 10.0.1.24 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.22 ... Done
  - Checking node 10.0.1.23 ... Done
  - Checking node 10.0.1.22 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.22 ... Done
  - Checking node 10.0.1.23 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.21 ... Done
  - Checking node 10.0.1.21 ... Done
+ Cleanup check files
  - Cleanup check files on 10.0.1.24:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.22:22 ... Done
  - Cleanup check files on 10.0.1.23:22 ... Done
  - Cleanup check files on 10.0.1.22:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.22:22 ... Done
  - Cleanup check files on 10.0.1.23:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
  - Cleanup check files on 10.0.1.21:22 ... Done
Node       Check           Result  Message
----       -----           ------  -------
10.0.1.24  permission      Pass    /home/tidb/tidb-deploy/tiflash-9000 is writable
10.0.1.24  permission      Pass    /home/tidb/tidb-deploy/tiflash-9000/tidb-data is writable
10.0.1.24  os-version      Pass    OS is Ubuntu 18.04.3 LTS 18.04.3
10.0.1.24  cpu-cores       Pass    number of CPU cores / threads: 1
10.0.1.24  swap            Fail    swap is enabled, please disable it for best performance
10.0.1.24  memory          Pass    memory size is 0MB
10.0.1.24  network         Fail    network speed of eth0 is 1000MB too low, needs 1GB or more
10.0.1.24  network         Fail    network speed of eth1 is 1000MB too low, needs 1GB or more
10.0.1.24  disk            Fail    mount point / does not have 'nodelalloc' option set
10.0.1.24  disk            Warn    mount point / does not have 'noatime' option set
10.0.1.24  listening-port  Fail    port 8123 is already in use
10.0.1.24  listening-port  Fail    port 3930 is already in use
10.0.1.24  listening-port  Fail    port 20170 is already in use
10.0.1.24  listening-port  Fail    port 20292 is already in use
10.0.1.24  listening-port  Fail    port 8234 is already in use
10.0.1.24  listening-port  Fail    port 9000 is already in use
...

They are not exactly the same line by line, but the final result are the same.

@ti-chi-bot ti-chi-bot requested a review from lucklove March 16, 2021 10:03
@baurine baurine changed the title Extract check cmd to manager Extract check cmd to pkg Mar 16, 2021
@ti-chi-bot ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 16, 2021
@codecov-io
Copy link

codecov-io commented Mar 16, 2021 •
edited
Loading

Codecov Report

Merging #1215 (b979685) into master (bd3cf97) will decrease coverage by 4.23%.
The diff coverage is 2.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1215      +/-   ##
==========================================
- Coverage   53.40%   49.16%   -4.24%     
==========================================
  Files         288      289       +1     
  Lines       20573    20589      +16     
==========================================
- Hits        10986    10122     -864     
- Misses       7870     8862     +992     
+ Partials     1717     1605     -112
Flag Coverage Δ
cluster 38.94% <2.97%> (-6.02%) ⬇️
dm 23.69% <0.00%> (-1.91%) ⬇️
integrate 43.44% <2.97%> (-4.41%) ⬇️
playground 3.09% <ø> (ø)
tiup 16.53% <ø> (-0.03%) ⬇️
unittest 22.76% <2.98%> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
pkg/cluster/manager/check.go 0.00% <0.00%> (ø)
pkg/cluster/operation/check.go 0.00% <0.00%> (-47.77%) ⬇️
components/cluster/command/check.go 78.57% <84.61%> (+8.73%) ⬆️
pkg/cluster/task/limits.go 0.00% <0.00%> (-68.75%) ⬇️
pkg/cluster/task/sysctl.go 0.00% <0.00%> (-66.67%) ⬇️
pkg/cluster/task/copy_file.go 0.00% <0.00%> (-54.55%) ⬇️
components/cluster/command/audit.go 27.27% <0.00%> (-54.55%) ⬇️
pkg/cluster/manager/patch.go 0.00% <0.00%> (-50.52%) ⬇️
pkg/cluster/task/rmdir.go 0.00% <0.00%> (-50.00%) ⬇️
pkg/cluster/manager/cleanup.go 0.00% <0.00%> (-47.62%) ⬇️
... and 45 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 bd3cf97...b979685. Read the comment docs.

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • AstroProfundis

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 Mar 17, 2021
@AstroProfundis
Copy link
Contributor

/merge

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 17, 2021
@AstroProfundis
Copy link
Contributor

network speed of eth0 is 1000MB too low, needs 1GB or more

This output is not as expected, could you also change the pkg/cluster/operation/check.go file at near line 247 to use >= instead of >?

@AstroProfundis
Copy link
Contributor

/merge cancel

@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 17, 2021
@baurine
Copy link
Contributor Author

baurine commented Mar 17, 2021

network speed of eth0 is 1000MB too low, needs 1GB or more

This output is not as expected, could you also change the pkg/cluster/operation/check.go file at near line 247 to use >= instead of >?

sure! will do it soon.

@baurine
Copy link
Contributor Author

baurine commented Mar 17, 2021

done!

image

@AstroProfundis
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: b979685

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 17, 2021
@ti-chi-bot ti-chi-bot merged commit 671b5f4 into pingcap:master Mar 17, 2021
@baurine baurine deleted the extract-check-cmd-to-manager branch March 17, 2021 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AstroProfundis AstroProfundis AstroProfundis approved these changes

@lucklove lucklove Awaiting requested review from lucklove

Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@baurine @codecov-io @ti-chi-bot @AstroProfundis