[Freeipa-devel] [PATCH] 873-874 ipa-replica-manage: adjust del to work with managed topology

[Martin Babinsky]

On 06/15/2015 02:15 PM, Petr Vobornik wrote:
> On 06/15/2015 01:46 PM, Martin Babinsky wrote:
>> On 06/15/2015 10:57 AM, Petr Vobornik wrote:
>>> On 06/12/2015 04:18 PM, Petr Vobornik wrote:
>>>> Some notes:
>>>>
>>>> 1. As mentioned in the WIP patch thread: original 'del' worked also
>>>> with
>>>> winsync agreements. I'm not sure why is that. Shouldn't 'disconnect' be
>>>> used for winsync agreements? At least man page says that. This patch
>>>> doesn't support it if domain level > 0. Is it a blocker?
>>>>
>>>> Following should be addressed in beta:
>>>>
>>>> 2. If `ipa-replica-manage del` is run before `ipa-csreplica-manage del`
>>>> then the `ipa-csreplica-manage del` will fail unless run with --force
>>>> options.
>>>>
>>>> 3. Check for orphaned server is missing. I want to use proper graph
>>>> traversing algorithm for that given that we have the whole topology.
>>>>
>>>> 4. Probably a work for topology plugin: I've seen that the removed
>>>> master doesn't remove its segments and agreements even though that it
>>>> knows about its removal (doesn't have its own entry in cn=masters). It
>>>> leads to failed replication connection attempts. Not a big issue, but
>>>> also not wanted.
>>>>
>>>>
>>>
>>> Martin3 found that there is wrong hostname in one error message. Fixed.
>>> Patch 873 rebased.
>>
>> Sorry but NACK.
>>
>> When I try to test the removal of last CA master I get a generic error
>> like this:
>>
>> """
>> unexpected error: no such entry
>>
>> """
>>
>> Traceback leading to this error is here:
>> http://pastebin.test.redhat.com/290131
>>
>> This is caused by the following test which assumes that 'master' is a
>> string, but this is in fact the whole result dictionary returned by
>> api.Command.server_find
>>
>> +        if master == hostname:
>> +            this_services = services_cns
>>
>> the following quick hack fixes this:
>> +        if str(master['dn'][0]['cn']) == hostname:
>> +            this_services = services_cn
>>
>> but there is certainly a more elegant approach, like transforming the
>> results to a list of master FQDNs directly after calling API command on
>> line 679.
>>
>
> ah, had this originally when serverservice object was used instead of
> direct ldap find in the WIP patch. Dict allow us to get dn directly for
> the service search. CN is also in the dict: master['cn'][0] so not need
> to get it from dn.
>
> Thanks for finding it.
>
> Updated patch attached.

Everything seems to work as expected. ACK.

-- 
Martin^3 Babinsky