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

[Martin Babinsky]

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.

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