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

[Ludwig Krispenz]

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 ?


-- 
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