cephadm: UX: Change error message when 'orch host add <host>' fails#35547
cephadm: UX: Change error message when 'orch host add <host>' fails#35547tchaikov merged 1 commit intoceph:masterfrom
Conversation
|
Sample output when add host fails Also, the error message itself is completely up for discussion if anybody has ideas to improve it. Right now it's "Failed to ssh into host as root. Please check troubleshooting ssh-errors section of ceph documentation for more info." |
mgfritch
left a comment
There was a problem hiding this comment.
I think we can keep the error msg very simple since most (or all?) of the actual failure context would be contained in the log lines from stderr ?
otherwise, lgtm!
src/cephadm/cephadm
Outdated
| try: | ||
| cli(['orch', 'host', 'add', host]) | ||
| except RuntimeError as e: | ||
| raise Error('Failed to ssh into host %s as root. Please check troubleshooting ssh-errors section of ceph documentation for more info.' % (host)) |
There was a problem hiding this comment.
| raise Error('Failed to ssh into host %s as root. Please check troubleshooting ssh-errors section of ceph documentation for more info.' % (host)) | |
| raise Error('Unable to add host: %s' % (host)) |
I think this can fail for more reasons than just failure to ssh ..
There was a problem hiding this comment.
I think you're probably right about it possibly being non-ssh issues. I'll remove that part. Do you think the recommendation to check the troubleshooting docs should be taken out entirely, or just changed so it it doesn't mention ssh specifically?
There was a problem hiding this comment.
I think consulting the troubleshooting guide is a given for any error, not just this one, so I'd probably drop that part too ...
@sebastian-philipp wdyt?
There was a problem hiding this comment.
I think we can drop this sentence. I mean, https://docs.ceph.com/docs/master/cephadm/troubleshooting/#ssh-errors is basically same as the error message of the exception.
|
Changed error message to simpler message |
src/cephadm/cephadm
Outdated
| try: | ||
| cli(['orch', 'host', 'add', host]) | ||
| except RuntimeError as e: | ||
| raise Error('Failed to add host: %s' % (host)) |
There was a problem hiding this comment.
Always is interesting to have information about why the command failed.. please include the error message:
raise Error('failed to add host <%s>: %s' % (host,e))
Instead of printing out a traceback if adding the host fails during bootstrapping process, should now print error message telling user host failed to be added Fixes: https://tracker.ceph.com/issues/45097 Signed-off-by: Adam King <adking@redhat.com>
|
Added given error message from exception to the printout |
Instead of printing out a traceback if adding the host fails
during bootstrapping process, should now print error message
telling user host failed to be added
Fixes: https://tracker.ceph.com/issues/45097
Signed-off-by: Adam King adking@redhat.com