[Freeipa-devel] [WIP PATCH] server-del: perform full master removal in managed topology

[Jan Cholasta]

On 19.4.2016 12:42, Martin Babinsky wrote:
> On 04/14/2016 11:46 AM, Ludwig Krispenz wrote:
>>
>> On 04/14/2016 10:59 AM, Martin Babinsky wrote:
>>> On 04/14/2016 08:24 AM, Jan Cholasta wrote:
>>>> On 13.4.2016 17:10, Rob Crittenden wrote:
>>>>> Martin Babinsky wrote:
>>>>>> This is a WIP patch which moves the `ipa-replica-manage del`
>>>>>> subcommand
>>>>>> to the 'server-del' API method and exposes it as CLI command[1]. A CI
>>>>>> test suite is also included.
>>>>>>
>>>>>> There are some issues with the patch I would like to discuss in more
>>>>>> detail on the list:
>>>>>>
>>>>>> 1.) In the original subcommand there was a lot of output (mostly
>>>>>> print
>>>>>> statements) during all stages of master removal. I have tried to port
>>>>>> these as messages to the command which results in quite voluminous
>>>>>> response sent back to the frontend. Should we try to reduce the
>>>>>> output?
>>>>>
>>>>> I don't think it applies anymore. These messages were there so the
>>>>> user
>>>>> would know something was happening. If it is an API command there
>>>>> isn't
>>>>> much we can do other than add something to the cli to print "This
>>>>> could
>>>>> take a bit" before making the call.
>>>>
>>>> +1
>>>>
>>> This is already implemented in PoC. So I guess we may reduce the
>>> output only to the following:
>>>
>>>
>>> In CLI print:
>>> "Removing {server} from replication topology, "
>>> "please wait...
>>>
>>> The adding info messages:
>>>
>>> "checking topology connectivity" | "skipping topology connectivity
>>> check"
>>> "checking remaining services" | "skipping check for remaining services"
>>> "performing cleanup"
>>> "Deleted server {server}"
>>>
>>>
>>>>>
>>>>>> 2.) In the original discussion[2] we assumed that the cleanup part
>>>>>> would
>>>>>> me a separate API method called during server_del postcallback.
>>>>>> However
>>>>>> since the two objects ended up sharing a lot of state (e.g. topology
>>>>>> state from pre-callback, messages) i have merged it to server-del.
>>>>>> That
>>>>>> makes the code rather unwieldy but I found it difficult to keep the
>>>>>> two
>>>>>> entities separate without some hacking around framework capabilities
>>>>>
>>>>> I haven't looked at the code but as a general principal having
>>>>> separate
>>>>> operations has saved our bacon on more than one occasion.
>>>>
>>>> The patch adds a force option, which allows you to re-run server-del
>>>> even if the master entry does not exist anymore, so I think we are
>>>> safe.
>>>>
>>>>>
>>>>>> 3.) since actions in post-callback require a knowledge about topology
>>>>>> state gathered in pre-callback, I had to store some information in
>>>>>> the
>>>>>> command's context. Sorry about that, if you know about some way to
>>>>>> circumvent me, let me know.
>>>>>
>>>>> Looks like it is the only way since you are extending server_del.
>>>>> Another option would be to drop pre/post and add all this topology
>>>>> stuff
>>>>> directly to server_del execute.
>>>>>
>>>>>> 4.) The master can not remove itself. I have implemented an ad-hoc
>>>>>> forwarding of the request to other master that can do the job. Is
>>>>>> this
>>>>>> okay?
>>>>
>>>> Why can't the master remove itself?
>>>>
>>> Because it removes its own replication agreements hence any changes in
>>> DIT (like removed principals, s4u2 proxy targets etc.) won't replicate
>>> to other masters.
>> It shouldn't remove replication agreements, in fact this should be
>> prevented by the topology plugin.
>> The removal of the agreements will be triggered by removing the master
>> entry
>
> That is true, but there is a plenty of cleanup code that is executed
> *after* the master entry is removed and these changes would not
> replicate if the agreements were removed by topology plugin in the
> meanwhile.

What kind of cleanup is it? Can it be done before instead?

-- 
Jan Cholasta