On 08/16/2016 03:47 PM, Stanislav Laznicka wrote:
On 08/15/2016 02:20 PM, Martin Babinsky wrote:
On 08/15/2016 02:13 PM, Martin Babinsky wrote:
On 08/12/2016 12:08 PM, Stanislav Laznicka wrote:

topology disconnect/last-role-host removal errors would just be logged
during server uninstall even if ignore options are not present. The
would still appear in the topology even after "successful" uninstall.


The patch seems to be ok, however shouldn't we use sys.exit() when
handling ServerRemovalError? Yes raising SystemExit from within a
function is a horrible practice, but it is already done on several other
places instead of letting the exception bubble up to the main handler.

CC'ing Jan for his thoughts on this since I may be wrong.

Hmm, you will definitely need sys.exit() here since otherwise
ipa-server-install reports 0 exit code even if there was an exception

[root@master1 ~]# ipa-server-install --uninstall -U
ipa         : ERROR    Server removal aborted: Deleting this server
will leave your installation without a DNS..
[root@master1 ~]# echo $?

Because of #5750 (remove sys.exit() calls from installer modules) I
rather raise ScriptError in this case. See the modified patch. It
depends on either of my 48-4 or 48-5 patches (pick one).

Make sure you raise it using str(e) because ScriptError does not coerce the argument to string/unicode:

@@ -303,7 +303,7 @@ def remove_master_from_managed_topology(api_instance, options):
             api_instance, api_instance.env.host, **server_del_options)
     except errors.ServerRemovalError as e:
-        raise ScriptError(e)
+        raise ScriptError(str(e))
     except Exception as e:
         # if the master was already deleted we will just get a warning
         root_logger.warning("Failed to delete master: {}".format(e))

Regardless of this fix, I still get exit code 0 after exception is raised:

[root@client1 ~]# ipa-server-install --uninstall -U
This server is active DNSSEC key master. Uninstall could break your DNS system. ipa : ERROR Server removal aborted: Replica is active DNSSEC key master. Uninstall could break your DNS system. Please disable or replace DNSSEC key master first..
[root@client1 ~]# echo $?

I guess there is nothing we can do about this now as the fix for this seems to be beyond the scope of this patch ( or am I mistaken?).

Martin^3 Babinsky

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to