[Freeipa-devel] [PATCH 0063] Raise error on topology disconnect/last-role-host removal during server uninstall

[Stanislav Laznicka]

On 08/17/2016 02:17 PM, Martin Babinsky wrote:
> 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:
>>>>> Hello,
>>>>>
>>>>> topology disconnect/last-role-host removal errors would just be 
>>>>> logged
>>>>> during server uninstall even if ignore options are not present. The
>>>>> host
>>>>> would still appear in the topology even after "successful" uninstall.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/6168
>>>>>
>>>>>
>>>>>
>>>>
>>>> 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
>>> thrown:
>>>
>>> """
>>> [root at master1 ~]# ipa-server-install --uninstall -U
>>> ipa         : ERROR    Server removal aborted: Deleting this server
>>> will leave your installation without a DNS..
>>> [root at master1 ~]# echo $?
>>> 0
>>> """
>>>
>> 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):
>          replication.run_server_del_as_cli(
>              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))
>
Oops, sorry, attached is the fixed patch.
> Regardless of this fix, I still get exit code 0 after exception is 
> raised:
>
> """
> [root at 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 at client1 ~]# echo $?
> 0
> """
>
> 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?).
>
Dandy. The actual relevant ticket to zero return value no matter what is 
already there - https://fedorahosted.org/freeipa/ticket/5725.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-slaznick-0063-3-Fail-on-topology-disconnect-last-role-removal.patch
Type: text/x-patch
Size: 1646 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160817/8b121114/attachment.bin>