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

[Ludwig Krispenz]

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

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