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

[Jan Cholasta]

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

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

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

-- 
Jan Cholasta