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

[Ludwig Krispenz]

On 04/21/2016 12:12 PM, Petr Vobornik wrote:
> 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?
I don't think that there is a use case except uninstall, but this 
doesen't prevent users to run the command. And then there are a few 
options how to handle this.
- accept it and try to do the best
- reject it and say it should be run on any other master
- automatically execute this on another master - I think this is 
Martin's current plan
>
> 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.

-- 
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Paul Argiry, Charles Cachera, Michael Cunningham, Michael O'Neill