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: improved error for missing tar #2499

Merged
merged 3 commits into from
Jan 22, 2025

Conversation

dveeden
Copy link
Contributor

@dveeden dveeden commented Jan 21, 2025

What problem does this PR solve?

With a minimal install of Rocky Linux 9.5 tar isn't installed and the error could be improved

Without this PR:

$ tiup cluster check topology.yaml 
The SSH identity key is encrypted. Input its passphrase: 

+ Detect CPU Arch Name
  - Detecting node 192.168.122.196 Arch info ... Done

+ Detect CPU OS Name
  - Detecting node 192.168.122.196 OS info ... Done
+ Download necessary tools
  - Downloading check tools for linux/amd64 ... Done
+ Collect basic system information
  - Getting system info of 192.168.122.196:22 ... Error

Error: stderr: bash: line 1: tar: command not found
: executor.ssh.execute_failed: Failed to execute command over SSH for '[email protected]:22' {ssh_stderr: bash: line 1: tar: command not found
, ssh_stdout: , ssh_command: export LANG=C; PATH=$PATH:/bin:/sbin:/usr/bin:/usr/sbin; /usr/bin/sudo -H bash -c "tar --no-same-owner -zxf /tmp/tiup/bin/insight-v0.4.2-linux-amd64.tar.gz -C /tmp/tiup/bin && rm /tmp/tiup/bin/insight-v0.4.2-linux-amd64.tar.gz"}, cause: Process exited with status 127

Verbose debug logs has been written to /home/dvaneeden/.tiup/logs/tiup-cluster-debug-2025-01-21-08-16-43.log.

With this PR:

$ tiup cluster check topology.yaml 
The SSH identity key is encrypted. Input its passphrase: 

+ Detect CPU Arch Name
  - Detecting node 192.168.122.196 Arch info ... Done

+ Detect CPU OS Name
  - Detecting node 192.168.122.196 OS info ... Done
+ Download necessary tools
  - Downloading check tools for linux/amd64 ... Done
+ Collect basic system information
  - Getting system info of 192.168.122.196:22 ... Error

Error: tar command was not found, please install it

Verbose debug logs has been written to /home/dvaneeden/.tiup/logs/tiup-cluster-debug-2025-01-21-08-42-24.log.

What is changed and how it works?

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Release notes:

NONE

Copy link
Contributor

ti-chi-bot bot commented Jan 21, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign bb7133 for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot requested a review from breezewish January 21, 2025 07:45
@ti-chi-bot ti-chi-bot bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 21, 2025
@@ -51,6 +52,9 @@ func (c *InstallPackage) Execute(ctx context.Context) error {

_, stderr, err := exec.Execute(ctx, cmd, false)
if err != nil {
if bytes.Contains(stderr, []byte("command not found")) {
Copy link
Member

@breezewish breezewish Jan 21, 2025

Choose a reason for hiding this comment

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

How about polish the text to install tar in the host xxx (xxx is the dest host name)? Then user will not misunderstood to install in the current machine. The rest LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 16da107

$ tiup cluster check topology.yaml 
The SSH identity key is encrypted. Input its passphrase: 

+ Detect CPU Arch Name
  - Detecting node 192.168.122.196 Arch info ... Done

+ Detect CPU OS Name
  - Detecting node 192.168.122.196 OS info ... Done
+ Download necessary tools
  - Downloading check tools for linux/amd64 ... Done
+ Collect basic system information
  - Getting system info of 192.168.122.196:22 ... Error

Error: tar command was not found on 192.168.122.196, please install it

Verbose debug logs has been written to /home/dvaneeden/.tiup/logs/tiup-cluster-debug-2025-01-21-13-26-46.log.

@ti-chi-bot ti-chi-bot bot added the lgtm label Jan 21, 2025
Copy link
Contributor

ti-chi-bot bot commented Jan 21, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-01-21 12:45:02.476805382 +0000 UTC m=+184829.807724781: ☑️ agreed by breezewish.

@breezewish
Copy link
Member

/cc @xhebox

@ti-chi-bot ti-chi-bot bot requested a review from xhebox January 21, 2025 12:45
@xhebox xhebox merged commit 8e15a03 into pingcap:master Jan 22, 2025
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants