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

[Martin Babinsky]

On 11/12/2015 04:24 PM, Martin Babinsky wrote:
> On 11/12/2015 02:04 PM, Petr Vobornik wrote:
>> 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).
>
> I didn't realize that. I have modified the patch accordingly.
>
>
>
Attaching updated patch with changed docstring of 
'check_last_link_managed()'

-- 
Martin^3 Babinsky
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0093.4-check-for-disconnected-topology-and-deleted-agreemen.patch
Type: text/x-patch
Size: 13050 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151113/20abea1d/attachment.bin>