cephadm: make host add failure message more friendly

When a host add fails, the output was showing passed
and failed checks. This patch uses an ERROR prefix for
all failed checks, and then filters on them when showing
the result, so the admin sees only the failed items. The
text has also been tidied up to remove the [] within a
[] syntax

Signed-off-by: Paul Cuzner <pcuzner@redhat.com>

diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm
index 2b18283dfefe4981498a81a47e03606c25849da9..a7b216017848f6ade8843ae9de301eaa75de0177 100755 (executable)
--- a/src/cephadm/cephadm
+++ b/src/cephadm/cephadm
@@ -4521,7 +4521,7 @@ def command_check_host():
             except Exception as e:
                 logger.debug('Could not locate %s: %s' % (i, e))
         if not container_path:
-            errors.append('Unable to locate any of %s' % CONTAINER_PREFERENCE)
+            errors.append('ERROR: Unable to locate a supported container engine ({})'.format(' or '.join(CONTAINER_PREFERENCE)))
         else:
             logger.info('podman|docker (%s) is present' % container_path)
 
@@ -4530,15 +4530,15 @@ def command_check_host():
             find_program(command)
             logger.info('%s is present' % command)
         except ValueError:
-            errors.append('%s binary does not appear to be installed' % command)
+            errors.append('ERROR: %s binary does not appear to be installed' % command)
 
     # check for configured+running chronyd or ntp
     if not check_time_sync():
-        errors.append('No time synchronization is active')
+        errors.append('ERROR: No time synchronization is active')
 
     if 'expect_hostname' in args and args.expect_hostname:
         if get_hostname().lower() != args.expect_hostname.lower():
-            errors.append('hostname "%s" does not match expected hostname "%s"' % (
+            errors.append('ERROR: hostname "%s" does not match expected hostname "%s"' % (
                 get_hostname(), args.expect_hostname))
         logger.info('Hostname "%s" matches what is expected.',
                     args.expect_hostname)

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index db7c83ed2513b9093ac6cf119fa100ec9e96815c..9ded4926cc1cbd0636a8875e64ac6fe15bebf177 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -1306,8 +1306,11 @@ To check that the host is reachable:
                                            addr=spec.addr,
                                            error_ok=True, no_fsid=True)
         if code:
-            raise OrchestratorError('New host %s (%s) failed check: %s' % (
-                spec.hostname, spec.addr, err))
+            # err will contain stdout and stderr, so we filter on the message text to 
+            # only show the errors
+            errors = [_i.replace("ERROR: ", "") for _i in err if _i.startswith('ERROR')]
+            raise OrchestratorError('New host %s (%s) failed check(s): %s' % (
+                spec.hostname, spec.addr, errors))
 
         self.inventory.add_host(spec)
         self.cache.prime_empty_host(spec.hostname)