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.
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
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
