From e6bfd660826d9f09fb594e7d5dfb6c176edb7c9c Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Tue, 4 Jun 2019 12:18:46 -0400
Subject: [PATCH 1/3] Drop list of return values to be ignored in AdminTool

This was an attempt to suppress client uninstallation failure
messages in the server uninstallation script. This method
inadvertently also suppressed client uninstallation messages and
was generally confusing.

This reverts part of b96906156be37a7b29ee74423b82f04070c84e22

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

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 ipaclient/install/ipa_client_install.py |  1 -
 ipapython/admintool.py                  |  3 +--
 ipapython/install/cli.py                | 11 ++---------
 3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/ipaclient/install/ipa_client_install.py b/ipaclient/install/ipa_client_install.py
index 6eaf1c84ed..76375f8d27 100644
--- a/ipaclient/install/ipa_client_install.py
+++ b/ipaclient/install/ipa_client_install.py
@@ -62,7 +62,6 @@ def host_password(self):
     verbose=True,
     console_format='%(message)s',
     uninstall_log_file_name=paths.IPACLIENT_UNINSTALL_LOG,
-    ignore_return_codes=(client.CLIENT_NOT_CONFIGURED,),
 )
 
 
diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index 75bfe28300..30da3fd9e7 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -93,7 +93,6 @@ class AdminTool(object):
     log_file_name = None
     usage = None
     description = None
-    ignore_return_codes = ()
 
     _option_parsers = dict()
 
@@ -184,7 +183,7 @@ def execute(self):
                     return_value = exception.rval  # pylint: disable=no-member
             traceback = sys.exc_info()[2]
             error_message, return_value = self.handle_error(exception)
-            if return_value and return_value not in self.ignore_return_codes:
+            if return_value:
                 self.log_failure(error_message, return_value, exception,
                     traceback)
                 return return_value
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index 2c95a20c6a..1bcac67b94 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -58,8 +58,7 @@ 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,
-                 ignore_return_codes=()):
+                 use_private_ccache=True, uninstall_log_file_name=None):
     """
     Some commands represent multiple related tools, e.g.
     ``ipa-server-install`` and ``ipa-server-install --uninstall`` would be
@@ -73,8 +72,6 @@ def install_tool(configurable_class, command_name, log_file_name,
     :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(
@@ -84,7 +81,6 @@ 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
@@ -102,14 +98,12 @@ 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,
-                   ignore_return_codes=()):
+                   debug_option=False, verbose=False, console_format=None):
     return type(
         'uninstall_tool({0})'.format(configurable_class.__name__),
         (UninstallTool,),
@@ -121,7 +115,6 @@ 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,
         )
     )
 

From 14c396c38c0088fb1407e52af851b2aa22ad478d Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Tue, 4 Jun 2019 12:18:46 -0400
Subject: [PATCH 2/3] Return 0 on uninstall when on_master for case of not
 installed

This is to suppress the spurious error message:

The ipa-client-install command failed.

when the client is not configured.

This is managed by allowing a ScriptError to return SUCCESS (0)
and have this ignored in log_failure().

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

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 ipaclient/install/client.py | 6 +++++-
 ipapython/admintool.py      | 7 ++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index 5388bdc109..819319b5c8 100644
--- a/ipaclient/install/client.py
+++ b/ipaclient/install/client.py
@@ -3177,9 +3177,13 @@ def uninstall_check(options):
     fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
 
     if not is_ipa_client_installed(fstore):
+        if options.on_master:
+            rval = SUCCESS
+        else:
+            rval = CLIENT_NOT_CONFIGURED
         raise ScriptError(
             "IPA client is not configured on this system.",
-            rval=CLIENT_NOT_CONFIGURED)
+            rval=rval)
 
     server_fstore = sysrestore.FileStore(paths.SYSRESTORE)
     if server_fstore.has_files() and not options.on_master:
diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index 30da3fd9e7..d3fee088e5 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -279,7 +279,7 @@ def handle_error(self, exception):
         """Given an exception, return a message (or None) and process exit code
         """
         if isinstance(exception, ScriptError):
-            return exception.msg, exception.rval or 1
+            return exception.msg, exception.rval
         elif isinstance(exception, SystemExit):
             if isinstance(exception.code, int):
                 return None, exception.code
@@ -307,6 +307,11 @@ def log_failure(self, error_message, return_value, exception, backtrace):
                      self.command_name, type(exception).__name__, exception)
         if error_message:
             logger.error('%s', error_message)
+        if return_value == 0:
+            # A script may raise an exception but still want quit gracefully,
+            # like the case of ipa-client-install called from
+            # ipa-server-install.
+            return
         message = "The %s command failed." % self.command_name
         if self.log_file_name and return_value != 2:
             # magic value because this is common between server and client

From 8ccdd3748098c1f5de7e5aea903af7a5df39b117 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Thu, 6 Jun 2019 10:29:38 -0400
Subject: [PATCH 3/3] Fix expected return code in tests when server is
 uninstalled

It is likely that these were fixed by the original change
b96906156be37a7b29ee74423b82f04070c84e22 but was uncaught because
these tests are not executed in CI because the server is configured.

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

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 ipatests/test_cmdline/test_cli.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_cmdline/test_cli.py b/ipatests/test_cmdline/test_cli.py
index c27b950d1e..a6741135ec 100644
--- a/ipatests/test_cmdline/test_cli.py
+++ b/ipatests/test_cmdline/test_cli.py
@@ -367,8 +367,8 @@ def test_cli_fsencoding():
     [
         # Commands delivered by the client pkg
         (['ipa'], 1, None, IPA_CLIENT_NOT_CONFIGURED),
-        (['ipa-certupdate'], 1, None, IPA_CLIENT_NOT_CONFIGURED),
-        (['ipa-client-automount'], 1, IPA_CLIENT_NOT_CONFIGURED, None),
+        (['ipa-certupdate'], 2, None, IPA_CLIENT_NOT_CONFIGURED),
+        (['ipa-client-automount'], 2, IPA_CLIENT_NOT_CONFIGURED, None),
         # Commands delivered by the server pkg
         (['ipa-adtrust-install'], 2, None, IPA_NOT_CONFIGURED),
         (['ipa-advise'], 2, None, IPA_NOT_CONFIGURED),
