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

[Petr Vobornik]

On 04/21/2016 10:41 AM, Ludwig Krispenz wrote:
> 
> On 04/21/2016 10:11 AM, Martin Babinsky wrote:
>> 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.
> no, it is not guranteed that the information on the removed master will
> be correct. If the del is applied to the to be removed master the
> topology plugin will only on a master which is remaining start the
> removal of segments, these will alos be replicated back to the removed
> master, but the repl agreements to this master will also be removed, so
> no gurantee which mods will be available on the removed master, and you
> should also be able to remove a master if it is down - so applying the
> full removal on a remaining server makes sense.
> 
> What is the behaviour if the removal of a server would disconnect
> topology ?
> 
> 

What would be the use-case for master to remove itself?

The only one I see, which was proposed in design page is that in
`ipa-server-install --uninstall` the installer would call `ipa
server-del $to_be_removed` on different replica so that uninstallation
would be done in single step. But effectively this is not removing
itself from API point of view.

Calling `ipa server-del $me` without subsequent uninstallation seems
pointless to me. In `ipa-replica-manage` a replica can't remove itself
as well.
-- 
Petr Vobornik