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

[Martin Babinsky]

On 04/21/2016 09:21 AM, Jan Cholasta wrote:
> 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?
>

Well most of the code can be run in pre-callback if all the checks are 
passed/forced.

However there is a check for deleted segments and this one should be run 
after the removal of master entry to see if topology plugin removed all 
dangling segments pointing to master. I am not quite sure if it make 
sense to run this check in the master which is being removed.

-- 
Martin^3 Babinsky