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

[Petr Vobornik]

On 06/15/2015 02:44 PM, Martin Babinsky wrote:
> 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.
>
pushed to master

* d58bdf29a514a7868c63b767f4954891b10a574d server: add "del" command
* e9e4509b10e5064556f0aa9a6f0124f38f14b31b ipa-replica-manage: adjust 
del to work with managed topology
-- 
Petr Vobornik