Dev: utils: Check node is reachable by using both ping and ssh #1563
Conversation
liangxin1300
force-pushed
the
20240924_improve_ping_node
branch
from
b8ea58a
to
2132b3e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
crmsh/utils.py
Outdated
|
|
||
| user = userdir.getuser() | ||
| rc = check_ssh(user, user, node, timeout=3) |
There was a problem hiding this comment.
It is an overkill to check whether a node is reachable by invoking ssh command. Opening an TCP socket without sending any data is enough.
liangxin1300
force-pushed
the
20240924_improve_ping_node
branch
from
2132b3e
to
31b3392
Compare
crmsh/utils.py
Outdated
| # ping failed, try to connect to ssh port by socket | ||
| error_msg = f"host \"{node}\" is unreachable" | ||
| try: | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: |
There was a problem hiding this comment.
Does not work with IPv6. Please use getaddrinfo first.
There was a problem hiding this comment.
I found we already have utils.check_port_open, should be reused this time
crmsh/utils.py
Outdated
| rc, _, err = ShellUtils().get_stdout_stderr("ping -c 1 {}".format(node)) | ||
| if rc != 0: | ||
| raise ValueError("host \"{}\" is unreachable: {}".format(node, err)) | ||
| rc, _, _ = ShellUtils().get_stdout_stderr(f"ping -c {ping_count} {node}") |
There was a problem hiding this comment.
Please add -n to avoid triggering a reverse DNS lookup.
liangxin1300
force-pushed
the
20240924_improve_ping_node
branch
2 times, most recently
from
54e60c1
to
dc69b4d
Compare
crmsh/utils.py
Outdated
| rc, _, err = ShellUtils().get_stdout_stderr("ping -c 1 {}".format(node)) | ||
| if rc != 0: | ||
| raise ValueError("host \"{}\" is unreachable: {}".format(node, err)) | ||
| rc, _, _ = ShellUtils().get_stdout_stderr(f"ping -n -c {ping_count} {node}") |
There was a problem hiding this comment.
timeout is not applied to the ping command.
crmsh/utils.py
Outdated
| @@ -2460,13 +2467,18 @@ def package_is_installed(pkg, remote_addr=None): | |||
| return rc == 0 | |||
|
|
|||
|
|
|||
| def ping_node(node): | |||
| def node_reachable_checking(node, ping_count=1, port=22, timeout=3): | |||
There was a problem hiding this comment.
"check" is a noun.
| def node_reachable_checking(node, ping_count=1, port=22, timeout=3): | |
| def node_reachable_check(node, ping_count=1, port=22, timeout=3): |
If both ping and socket (to ssh port) fail, the node is considered unreachable.
liangxin1300
force-pushed
the
20240924_improve_ping_node
branch
from
dc69b4d
to
3988820
Compare
If both ping and ssh fail, the node is considered unreachable.
To fix issue #1551