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

[Martin Babinsky]

On 04/21/2016 12:22 PM, Ludwig Krispenz wrote:
>
> 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

Yes, also if you run `ipa server del $server` from a client that talks 
to $server, what should we do? current code handle this by forwarding 
the request to other master as Ludwig pointed out.
>>
>> 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.
>


-- 
Martin^3 Babinsky