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

[Martin Babinsky]

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.
>>
>> Assisted suicide?
>>
>> What does this effectively do? Potentially disconnect it from the
>> topology, perhaps to run as standalone for upgrade, integration or other
>> testing (e.g. there might be valid reasons to do this)? If so that seems
>> ok to me, or if too hacky you could just spit out a message directing
>> them to disconnect from another host, but that would be strange in the
>> UI I think.
>>
>>> 5.) Since the original behavior of 'chekc_deleted_segments' was kept,
>>> the code can sometimes hang waiting for removal of some segments,
>>> especially during forced removal in wonky topologies. This can cause
>>> gateway timeout in JSON-RPC call and report false positive error back to
>>> the user. This makes a large part of 'TestServerDel' suite to fail. How
>>> should we handle this? Should we keep the original behavior?
>>
>> Oh to have async calls...
>>
>> I wonder if "I'm still here" messages could be sent. I'm not entirely
>> sure this is possible with the framework but it might be one way to keep
>> long-lived connections alive.
>
> Impossible, everything is synchronous.
>
>>
>>> 6.) There are some in-place imports of server-side code in some places.
>>> I'm not very happy about it and would be glad if we can agree on a way
>>> to fix this.
>>
>> Is this to prevent imports on non-servers? You might look at trust for
>> inspiration, it does this.
>
> I wouldn't worry about this, I'm going to move most plugin code to
> ipaserver as part of the thin client feature anyway ;-)
>
> Honza
>


-- 
Martin^3 Babinsky