[Freeipa-devel] [PATCH 0153-0158] move ipa-replica-manage del functionality into server-del

[Martin Babinsky]

On 06/15/2016 10:30 AM, Jan Cholasta wrote:
> Hi,
>
> On 12.6.2016 17:31, Martin Babinsky wrote:
>> On 06/09/2016 08:12 PM, Martin Babinsky wrote:
>>> These patches expand `server_del` to a full fledged IPA master killer in
>>> domain level 1.
>>>
>>> Due to 'server uninstallation removed master from topology' use case,
>>> the individual steps are not in the same order as in the original code
>>> to facilitate self-removal from topology without introducing an array of
>>> permissions for master to remove itself.
>>>
>>> I had no opportunity to test out the CI test suite because of technical
>>> problems so it would be nice if our upstream QE could give it a spin and
>>> report errors.
>>>
>>> http://www.freeipa.org/page/V4/Manage_replication_topology_4_4
>>> https://fedorahosted.org/freeipa/ticket/5181
>>>
>>>
>>>
>> Attaching rebased patches and bumping for review.
>>
>> Please note that they depend on 'Server Roles v2' patchset.
>
> Patch 0153:
>
> Should be an ipaserver module, unless it is required on clients as well,
> in which case it should be an ipalib module.
>
>
> Patch 0154: LGTM
>
>
> Patch 0155:
>
> In LDAPDelete subclasses, the primary key argument is multivalue, so I'm
> guessing your post_callback won't work correctly.
>
> Also, since this is *server*-del, s/master/server/ where applicable.
>
>
> Patch 0156: LGTM
>
>
> Patch 0157:
>
> This looks suspicious:
>
> +    result = server_del_cmd(hostname, version=api_version, **options)
>
> Version is automatically filled in in Command.__call__(), why do you add
> it manually here?
>

Because this uses server API which calls command's `execute` directly so 
it does not pass version automatically.

So you get version mismatch warning every time this function is called.

>
> Patch 0158: LGTM
>
>
> Honza
>


-- 
Martin^3 Babinsky