Improve console logging for ipa-server-install

The server installation and uninstallation overlaps both the
server and client installers. The output could be confusing
with a server uninstall finishing with the message:

The ipa-client-install command was successful

This was in part due to the fact that the server was not
configured with a console format and verbose was False which
meant that no logger messages were displayed at all.

In order to suppress client installation errors and avoid
confusion add a list of errors to ignore. If a server install
was not successful and hadn't gotten far enough to do the
client install then we shouldn't complain loudly about it.

https://pagure.io/freeipa/issue/6760

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>

ipaclient/install/ipa_client_install.py

@@ -62,6 +62,7 @@ ClientInstall = cli.install_tool(
     verbose=True,
     console_format='%(message)s',
     uninstall_log_file_name=paths.IPACLIENT_UNINSTALL_LOG,
+    ignore_return_codes=(client.CLIENT_NOT_CONFIGURED,),
 )
 
 

ipapython/admintool.py

@@ -93,6 +93,7 @@ class AdminTool(object):
     log_file_name = None
     usage = None
     description = None
+    ignore_return_codes = ()
 
     _option_parsers = dict()
 
@@ -177,9 +178,13 @@ class AdminTool(object):
             self.setup_logging()
             return_value = self.run()
         except BaseException as exception:
+            if isinstance(exception, ScriptError):
+                # pylint: disable=no-member
+                if exception.rval and exception.rval > return_value:
+                    return_value = exception.rval  # pylint: disable=no-member
             traceback = sys.exc_info()[2]
             error_message, return_value = self.handle_error(exception)
-            if return_value:
+            if return_value and return_value not in self.ignore_return_codes:
                 self.log_failure(error_message, return_value, exception,
                     traceback)
                 return return_value

ipapython/install/cli.py

@@ -58,7 +58,24 @@ def _get_usage(configurable_class):
 
 def install_tool(configurable_class, command_name, log_file_name,
                  debug_option=False, verbose=False, console_format=None,
-                 use_private_ccache=True, uninstall_log_file_name=None):
+                 use_private_ccache=True, uninstall_log_file_name=None,
+                 ignore_return_codes=()):
+    """
+    Some commands represent multiple related tools, e.g.
+    ``ipa-server-install`` and ``ipa-server-install --uninstall`` would be
+    represented by separate classes. Only their options are the same.
+
+    :param configurable_class: the command class for options
+    :param command_name: the command name shown in logs/output
+    :param log_file_name: if None, logging is to stderr only
+    :param debug_option: log level is DEBUG
+    :param verbose: log level is INFO
+    :param console_format: logging format for stderr
+    :param use_private_ccache: a temporary ccache is created and used
+    :param uninstall_log_file_name: if not None the log for uninstall
+    :param ignore_return_codes: tuple of error codes to not log errors
+                                for. Let the caller do it if it wants.
+    """
     if uninstall_log_file_name is not None:
         uninstall_kwargs = dict(
             configurable_class=configurable_class,
@@ -67,6 +84,7 @@ def install_tool(configurable_class, command_name, log_file_name,
             debug_option=debug_option,
             verbose=verbose,
             console_format=console_format,
+            ignore_return_codes=ignore_return_codes,
         )
     else:
         uninstall_kwargs = None
@@ -84,12 +102,14 @@ def install_tool(configurable_class, command_name, log_file_name,
             console_format=console_format,
             uninstall_kwargs=uninstall_kwargs,
             use_private_ccache=use_private_ccache,
+            ignore_return_codes=ignore_return_codes,
         )
     )
 
 
 def uninstall_tool(configurable_class, command_name, log_file_name,
-                   debug_option=False, verbose=False, console_format=None):
+                   debug_option=False, verbose=False, console_format=None,
+                   ignore_return_codes=()):
     return type(
         'uninstall_tool({0})'.format(configurable_class.__name__),
         (UninstallTool,),
@@ -101,6 +121,7 @@ def uninstall_tool(configurable_class, command_name, log_file_name,
             debug_option=debug_option,
             verbose=verbose,
             console_format=console_format,
+            ignore_return_codes=ignore_return_codes,
         )
     )
 

ipaserver/install/ipa_server_install.py

@@ -40,7 +40,9 @@ ServerInstall = cli.install_tool(
     CompatServerMasterInstall,
     command_name='ipa-server-install',
     log_file_name=paths.IPASERVER_INSTALL_LOG,
+    console_format='%(message)s',
     debug_option=True,
+    verbose=True,
     uninstall_log_file_name=paths.IPASERVER_UNINSTALL_LOG,
 )