[Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Petr Vobornik]

On 11/10/2015 05:43 PM, Martin Babinsky wrote:
> On 11/04/2015 06:50 PM, Petr Vobornik wrote:
>> On 11/04/2015 01:30 PM, Martin Babinsky wrote:
>>> On 10/30/2015 05:06 PM, Martin Babinsky wrote:
>>>> On 10/30/2015 03:38 PM, Petr Vobornik wrote:
>>>>> On 10/30/2015 03:26 PM, Martin Babinsky wrote:
>>>>>> patch for https://fedorahosted.org/freeipa/ticket/5309
>>>>>>
>>>>>> The ticket itself is about connectivity checks in topology suffixes,
>>>>>> but
>>>>>> there is a code (install/tools/ipa-replica-manage starting at line
>>>>>> 788
>>>>>> after applying my patch) which monitors whether the segments pointing
>>>>>> to/from the deleted host are already deleted.
>>>>>>
>>>>>> These checks are currently hardcoded for 'realm' prefix, should we
>>>>>> generalize them as well or is it a part of other effort?
>>>>>>
>>>>>
>>>>> Could be separate patch but yes.
>>>> Ok I have included it in the attached patch so that both of these
>>>> operations are performed for all suffixes.
>>>>
>>>>
>>>>
>>> Hmm, I'm thinking whether the 'check_last_link_managed' and
>>> 'check_deleted_segments' should not be called per-suffix, but should
>>> themselves check all suffixes available. This could make the fix for
>>> https://fedorahosted.org/freeipa/ticket/5409 also easier.
>>>
>>
>> Depends if the output is reusable. If so then why not.
>> check_last_link_managed basically adds text to several
>> get_topology_connection_errors calls.
>
> Attaching updated patch.
>

I'm not sure about (pseudo code):

     topo_errors = ([], [])
     for each suffix:
     	topo_errors[0].extend(orig_errors)
         topo_errors[1].extend(new_errors)
     return topo_errors

In check_deleted_segments wait_for_segment_removal is per-suffix check 
but uses topo_errors which contains errors from both suffices. Topo 
erros are used to relax the check if topology is disconnected but this 
might relax it too much.

I would change the errors to per-suffix as well, e.g.:
   topo_errors = {}
   for each suffix:
       topo_errors[suffix_name] = (orig_errors, new_errors)
   return topo_errors

Otherwise it looks OK (not tested yet).
-- 
Petr Vobornik