Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
On 09/14/2012 09:16 PM, Rob Crittenden wrote: Martin Kosek wrote: On 09/10/2012 08:34 PM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2012-09-06 at 17:22 -0400, Rob Crittenden wrote: Martin Kosek wrote: On 08/31/2012 07:40 PM, Rob Crittenden wrote: Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob 1) When I run ipa-replica-manage del --force to an already uninstalled host, the new code will prevent me the deletation because it cannot connect to it. It also crashes with UnboundLocalError: # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal Traceback (most recent call last): File /sbin/ipa-replica-manage, line 708, in module main() File /sbin/ipa-replica-manage, line 677, in main del_master(realm, args[1], options) File /sbin/ipa-replica-manage, line 476, in del_master sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e))) UnboundLocalError: local variable 'delrepl' referenced before assignment Fixed. I also hit this error when removing a winsync replica. Fixed. 2) As I wrote before, I think having --force option override the user inquiries would benefit test automation: +if not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) Fixed. 3) I don't think this code won't cover this topology: A - B - C - D - E It would allow you deleting a replica C even though it would separate A-B and D-E. Though we may not want to cover this situation now, what you got is definitely helping. I think you may be right. I only tested with 4 servers. With this B and D would both still have 2 agreements so wouldn't be covered by the last link test. Everything looks good now, so ACK. We just need to push it along with CLEANALLRUV patch. Martin Not to look a gift ACK In the mouth but here is a revised patch. I've added a cleanup routine to remove an orphaned master properly. If you had tried the mechanism I outlined in the man page it would have worked but was less-than-complete. This way is better, just don't try it on a live master. I also added a cleanruv abort command, in case you want to kill an existing task. rob 1) CLEANRUV stuff should be in your patch 1031 and not here (but I will comment on the code in this mail since it is in this patch) 2) In new command defitinion: +abort-clean-ruv:(1, 1, Replica ID of to abort cleaning, ), I miss error message in case REPLICA_ID is not passed, this way error message when I omit the parameter is confusing: # ipa-replica-manage abort-clean-ruv Usage: ipa-replica-manage [options] ipa-replica-manage: error: must provide a command [force-sync | clean-ruv | disconnect | connect | list-ruv | del | re-initialize | list | abort-clean-ruv] On another note, I am thinking about the new command(s). Since we now have abort-clean-ruv command, we may want to also implement list-clean-ruv commands in the future to see all CLEANALLRUV commands in process... Or we may enhance list-ruv to write a flag like [CLEANALLRUV in process] for RUV's for which CLEANALLRUV task is in process. 3) Will clean-ruv - abort-clean-ruv - clean-ruv sequence work? If the aborted CLEANALLRUV task stays in DIT, we may not be able to enter new CLEANALLRUV task because we always use the same DN... Btw. did abort CLEANALLRUV command worked for you? Mine seemed to be stuck on replicas that are down just like CLEANALLRUV command. I had then paralelly running CLEANALLRUV and ABORT CLEANALLRUV running for the same RUV ID. Then, it is unclear to me what this command is actually good for. 4) Man page now contains non-existent command: --- a/install/tools/man/ipa-replica-manage.1 +++ b/install/tools/man/ipa-replica-manage.1 @@ -42,12 +42,18 @@ Manages the replication agreements of an IPA server. \fBforce\-sync\fR \- Immediately flush any data to be replicated from a server specified with the \-\-from option .TP +\fBcleanup\fR +\- Remove leftover references to a deleted master. +.TP The cleanup procedure was implemented rather as an option to the del command than a separate command. 5) My favorite - new cleanup procedure user prompt miss the --force option useful for test automation +if not
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
On 09/17/2012 04:06 PM, Martin Kosek wrote: On 09/14/2012 09:16 PM, Rob Crittenden wrote: Martin Kosek wrote: On 09/10/2012 08:34 PM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2012-09-06 at 17:22 -0400, Rob Crittenden wrote: Martin Kosek wrote: On 08/31/2012 07:40 PM, Rob Crittenden wrote: Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob 1) When I run ipa-replica-manage del --force to an already uninstalled host, the new code will prevent me the deletation because it cannot connect to it. It also crashes with UnboundLocalError: # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal Traceback (most recent call last): File /sbin/ipa-replica-manage, line 708, in module main() File /sbin/ipa-replica-manage, line 677, in main del_master(realm, args[1], options) File /sbin/ipa-replica-manage, line 476, in del_master sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e))) UnboundLocalError: local variable 'delrepl' referenced before assignment Fixed. I also hit this error when removing a winsync replica. Fixed. 2) As I wrote before, I think having --force option override the user inquiries would benefit test automation: +if not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) Fixed. 3) I don't think this code won't cover this topology: A - B - C - D - E It would allow you deleting a replica C even though it would separate A-B and D-E. Though we may not want to cover this situation now, what you got is definitely helping. I think you may be right. I only tested with 4 servers. With this B and D would both still have 2 agreements so wouldn't be covered by the last link test. Everything looks good now, so ACK. We just need to push it along with CLEANALLRUV patch. Martin Not to look a gift ACK In the mouth but here is a revised patch. I've added a cleanup routine to remove an orphaned master properly. If you had tried the mechanism I outlined in the man page it would have worked but was less-than-complete. This way is better, just don't try it on a live master. I also added a cleanruv abort command, in case you want to kill an existing task. rob 1) CLEANRUV stuff should be in your patch 1031 and not here (but I will comment on the code in this mail since it is in this patch) 2) In new command defitinion: +abort-clean-ruv:(1, 1, Replica ID of to abort cleaning, ), I miss error message in case REPLICA_ID is not passed, this way error message when I omit the parameter is confusing: # ipa-replica-manage abort-clean-ruv Usage: ipa-replica-manage [options] ipa-replica-manage: error: must provide a command [force-sync | clean-ruv | disconnect | connect | list-ruv | del | re-initialize | list | abort-clean-ruv] On another note, I am thinking about the new command(s). Since we now have abort-clean-ruv command, we may want to also implement list-clean-ruv commands in the future to see all CLEANALLRUV commands in process... Or we may enhance list-ruv to write a flag like [CLEANALLRUV in process] for RUV's for which CLEANALLRUV task is in process. 3) Will clean-ruv - abort-clean-ruv - clean-ruv sequence work? If the aborted CLEANALLRUV task stays in DIT, we may not be able to enter new CLEANALLRUV task because we always use the same DN... Btw. did abort CLEANALLRUV command worked for you? Mine seemed to be stuck on replicas that are down just like CLEANALLRUV command. I had then paralelly running CLEANALLRUV and ABORT CLEANALLRUV running for the same RUV ID. Then, it is unclear to me what this command is actually good for. 4) Man page now contains non-existent command: --- a/install/tools/man/ipa-replica-manage.1 +++ b/install/tools/man/ipa-replica-manage.1 @@ -42,12 +42,18 @@ Manages the replication agreements of an IPA server. \fBforce\-sync\fR \- Immediately flush any data to be replicated from a server specified with the \-\-from option .TP +\fBcleanup\fR +\- Remove leftover references to a deleted master. +.TP The cleanup procedure was implemented rather as an option to the del command than a separate command. 5) My favorite - new cleanup procedure user prompt miss the --force option useful for test
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
On 09/10/2012 08:34 PM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2012-09-06 at 17:22 -0400, Rob Crittenden wrote: Martin Kosek wrote: On 08/31/2012 07:40 PM, Rob Crittenden wrote: Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob 1) When I run ipa-replica-manage del --force to an already uninstalled host, the new code will prevent me the deletation because it cannot connect to it. It also crashes with UnboundLocalError: # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal Traceback (most recent call last): File /sbin/ipa-replica-manage, line 708, in module main() File /sbin/ipa-replica-manage, line 677, in main del_master(realm, args[1], options) File /sbin/ipa-replica-manage, line 476, in del_master sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e))) UnboundLocalError: local variable 'delrepl' referenced before assignment Fixed. I also hit this error when removing a winsync replica. Fixed. 2) As I wrote before, I think having --force option override the user inquiries would benefit test automation: +if not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) Fixed. 3) I don't think this code won't cover this topology: A - B - C - D - E It would allow you deleting a replica C even though it would separate A-B and D-E. Though we may not want to cover this situation now, what you got is definitely helping. I think you may be right. I only tested with 4 servers. With this B and D would both still have 2 agreements so wouldn't be covered by the last link test. Everything looks good now, so ACK. We just need to push it along with CLEANALLRUV patch. Martin Not to look a gift ACK In the mouth but here is a revised patch. I've added a cleanup routine to remove an orphaned master properly. If you had tried the mechanism I outlined in the man page it would have worked but was less-than-complete. This way is better, just don't try it on a live master. I also added a cleanruv abort command, in case you want to kill an existing task. rob 1) CLEANRUV stuff should be in your patch 1031 and not here (but I will comment on the code in this mail since it is in this patch) 2) In new command defitinion: +abort-clean-ruv:(1, 1, Replica ID of to abort cleaning, ), I miss error message in case REPLICA_ID is not passed, this way error message when I omit the parameter is confusing: # ipa-replica-manage abort-clean-ruv Usage: ipa-replica-manage [options] ipa-replica-manage: error: must provide a command [force-sync | clean-ruv | disconnect | connect | list-ruv | del | re-initialize | list | abort-clean-ruv] On another note, I am thinking about the new command(s). Since we now have abort-clean-ruv command, we may want to also implement list-clean-ruv commands in the future to see all CLEANALLRUV commands in process... Or we may enhance list-ruv to write a flag like [CLEANALLRUV in process] for RUV's for which CLEANALLRUV task is in process. 3) Will clean-ruv - abort-clean-ruv - clean-ruv sequence work? If the aborted CLEANALLRUV task stays in DIT, we may not be able to enter new CLEANALLRUV task because we always use the same DN... Btw. did abort CLEANALLRUV command worked for you? Mine seemed to be stuck on replicas that are down just like CLEANALLRUV command. I had then paralelly running CLEANALLRUV and ABORT CLEANALLRUV running for the same RUV ID. Then, it is unclear to me what this command is actually good for. 4) Man page now contains non-existent command: --- a/install/tools/man/ipa-replica-manage.1 +++ b/install/tools/man/ipa-replica-manage.1 @@ -42,12 +42,18 @@ Manages the replication agreements of an IPA server. \fBforce\-sync\fR \- Immediately flush any data to be replicated from a server specified with the \-\-from option .TP +\fBcleanup\fR +\- Remove leftover references to a deleted master. +.TP The cleanup procedure was implemented rather as an option to the del command than a separate command. 5) My favorite - new cleanup procedure user prompt miss the --force option useful for test automation +if not ipautil.user_input(Continue to clean master?, False): +sys.exit(Cleanup aborted) Otherwise the patch looks good. Martin
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
Martin Kosek wrote: On 09/10/2012 08:34 PM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2012-09-06 at 17:22 -0400, Rob Crittenden wrote: Martin Kosek wrote: On 08/31/2012 07:40 PM, Rob Crittenden wrote: Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob 1) When I run ipa-replica-manage del --force to an already uninstalled host, the new code will prevent me the deletation because it cannot connect to it. It also crashes with UnboundLocalError: # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal Traceback (most recent call last): File /sbin/ipa-replica-manage, line 708, in module main() File /sbin/ipa-replica-manage, line 677, in main del_master(realm, args[1], options) File /sbin/ipa-replica-manage, line 476, in del_master sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e))) UnboundLocalError: local variable 'delrepl' referenced before assignment Fixed. I also hit this error when removing a winsync replica. Fixed. 2) As I wrote before, I think having --force option override the user inquiries would benefit test automation: +if not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) Fixed. 3) I don't think this code won't cover this topology: A - B - C - D - E It would allow you deleting a replica C even though it would separate A-B and D-E. Though we may not want to cover this situation now, what you got is definitely helping. I think you may be right. I only tested with 4 servers. With this B and D would both still have 2 agreements so wouldn't be covered by the last link test. Everything looks good now, so ACK. We just need to push it along with CLEANALLRUV patch. Martin Not to look a gift ACK In the mouth but here is a revised patch. I've added a cleanup routine to remove an orphaned master properly. If you had tried the mechanism I outlined in the man page it would have worked but was less-than-complete. This way is better, just don't try it on a live master. I also added a cleanruv abort command, in case you want to kill an existing task. rob 1) CLEANRUV stuff should be in your patch 1031 and not here (but I will comment on the code in this mail since it is in this patch) 2) In new command defitinion: +abort-clean-ruv:(1, 1, Replica ID of to abort cleaning, ), I miss error message in case REPLICA_ID is not passed, this way error message when I omit the parameter is confusing: # ipa-replica-manage abort-clean-ruv Usage: ipa-replica-manage [options] ipa-replica-manage: error: must provide a command [force-sync | clean-ruv | disconnect | connect | list-ruv | del | re-initialize | list | abort-clean-ruv] On another note, I am thinking about the new command(s). Since we now have abort-clean-ruv command, we may want to also implement list-clean-ruv commands in the future to see all CLEANALLRUV commands in process... Or we may enhance list-ruv to write a flag like [CLEANALLRUV in process] for RUV's for which CLEANALLRUV task is in process. 3) Will clean-ruv - abort-clean-ruv - clean-ruv sequence work? If the aborted CLEANALLRUV task stays in DIT, we may not be able to enter new CLEANALLRUV task because we always use the same DN... Btw. did abort CLEANALLRUV command worked for you? Mine seemed to be stuck on replicas that are down just like CLEANALLRUV command. I had then paralelly running CLEANALLRUV and ABORT CLEANALLRUV running for the same RUV ID. Then, it is unclear to me what this command is actually good for. 4) Man page now contains non-existent command: --- a/install/tools/man/ipa-replica-manage.1 +++ b/install/tools/man/ipa-replica-manage.1 @@ -42,12 +42,18 @@ Manages the replication agreements of an IPA server. \fBforce\-sync\fR \- Immediately flush any data to be replicated from a server specified with the \-\-from option .TP +\fBcleanup\fR +\- Remove leftover references to a deleted master. +.TP The cleanup procedure was implemented rather as an option to the del command than a separate command. 5) My favorite - new cleanup procedure user prompt miss the --force option useful for test automation +if not ipautil.user_input(Continue to clean master?, False): +sys.exit(Cleanup aborted) Otherwise the patch looks good. Martin I pulled the abort-ruv stuff
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
Martin Kosek wrote:
On Thu, 2012-09-06 at 17:22 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On 08/31/2012 07:40 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
It was possible use ipa-replica-manage connect/disconnect/del to end up
orphaning or or more IPA masters. This is an attempt to catch and
prevent that case.
I tested with this topology, trying to delete B.
A - B - C
I got here by creating B and C from A, connecting B to C then deleting
the link from A to B, so it went from A - B and A - C to the above.
What I do is look up the servers that the delete candidate host has
connections to and see if we're the last link.
I added an escape clause if there are only two masters.
rob
Oh, this relies on my cleanruv patch 1031.
rob
1) When I run ipa-replica-manage del --force to an already uninstalled host,
the new code will prevent me the deletation because it cannot connect to it. It
also crashes with UnboundLocalError:
# ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force
Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal
Traceback (most recent call last):
File /sbin/ipa-replica-manage, line 708, in module
main()
File /sbin/ipa-replica-manage, line 677, in main
del_master(realm, args[1], options)
File /sbin/ipa-replica-manage, line 476, in del_master
sys.exit(Failed read master data from '%s': %s % (delrepl.hostname,
str(e)))
UnboundLocalError: local variable 'delrepl' referenced before assignment
Fixed.
I also hit this error when removing a winsync replica.
Fixed.
2) As I wrote before, I think having --force option override the user inquiries
would benefit test automation:
+if not ipautil.user_input(Continue to delete?, False):
+sys.exit(Aborted)
Fixed.
3) I don't think this code won't cover this topology:
A - B - C - D - E
It would allow you deleting a replica C even though it would separate A-B and
D-E. Though we may not want to cover this situation now, what you got is
definitely helping.
I think you may be right. I only tested with 4 servers. With this B and
D would both still have 2 agreements so wouldn't be covered by the last
link test.
Everything looks good now, so ACK. We just need to push it along with
CLEANALLRUV patch.
Martin
Not to look a gift ACK In the mouth but here is a revised patch. I've
added a cleanup routine to remove an orphaned master properly. If you
had tried the mechanism I outlined in the man page it would have worked
but was less-than-complete. This way is better, just don't try it on a
live master.
I also added a cleanruv abort command, in case you want to kill an
existing task.
rob
From a8718439ac1f51d857a03ff27776850324c43cda Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 31 Aug 2012 11:56:58 -0400
Subject: [PATCH] When deleting a master, try to prevent orphaning other
servers.
If you have a replication topology like A - B - C and you try
to delete server B that will leave A and C orphaned. It may also
prevent re-installation of a new master on B because the cn=masters
entry for it probably still exists on at least one of the other masters.
Check on each master that it connects to to ensure that it isn't the
last link, and fail if it is. If any of the masters are not up then
warn that this could be a bad thing but let the user continue if
they want.
Add a new option to the del command, --cleanup, which runs the
replica_cleanup() routine to completely clean up references to a master.
Add a command to run the abort-clean-ruv task.
https://fedorahosted.org/freeipa/ticket/2797
---
install/tools/ipa-replica-manage | 115 -
install/tools/man/ipa-replica-manage.1 | 17 +
ipaserver/install/replication.py | 22 +++
3 files changed, 153 insertions(+), 1 deletion(-)
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index c6ef51b7215164c9538afae942e3d42285ca860b..60d8ecbc22bfde66042b9ff4ea1068ad79566ed1 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -49,6 +49,7 @@ commands = {
re-initialize:(0, 0, , ),
force-sync:(0, 0, , ),
clean-ruv:(1, 1, Replica ID of to clean, ),
+abort-clean-ruv:(1, 1, Replica ID of to abort cleaning, ),
}
def convert_error(exc):
@@ -70,6 +71,8 @@ def parse_options():
help=provide additional information)
parser.add_option(-f, --force, dest=force, action=store_true, default=False,
help=ignore some types of errors)
+parser.add_option(-c, --cleanup, dest=cleanup, action=store_true, default=False,
+ help=DANGER: clean up references to a ghost master)
parser.add_option(--binddn, dest=binddn, default=None, type=dn,
help=Bind DN to use with remote server)
parser.add_option(--bindpw, dest=bindpw, default=None,
@@ -398,9 +401,80
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
On Thu, 2012-09-06 at 17:22 -0400, Rob Crittenden wrote: Martin Kosek wrote: On 08/31/2012 07:40 PM, Rob Crittenden wrote: Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob 1) When I run ipa-replica-manage del --force to an already uninstalled host, the new code will prevent me the deletation because it cannot connect to it. It also crashes with UnboundLocalError: # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal Traceback (most recent call last): File /sbin/ipa-replica-manage, line 708, in module main() File /sbin/ipa-replica-manage, line 677, in main del_master(realm, args[1], options) File /sbin/ipa-replica-manage, line 476, in del_master sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e))) UnboundLocalError: local variable 'delrepl' referenced before assignment Fixed. I also hit this error when removing a winsync replica. Fixed. 2) As I wrote before, I think having --force option override the user inquiries would benefit test automation: +if not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) Fixed. 3) I don't think this code won't cover this topology: A - B - C - D - E It would allow you deleting a replica C even though it would separate A-B and D-E. Though we may not want to cover this situation now, what you got is definitely helping. I think you may be right. I only tested with 4 servers. With this B and D would both still have 2 agreements so wouldn't be covered by the last link test. Everything looks good now, so ACK. We just need to push it along with CLEANALLRUV patch. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
On 08/31/2012 07:40 PM, Rob Crittenden wrote: Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob 1) When I run ipa-replica-manage del --force to an already uninstalled host, the new code will prevent me the deletation because it cannot connect to it. It also crashes with UnboundLocalError: # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal Traceback (most recent call last): File /sbin/ipa-replica-manage, line 708, in module main() File /sbin/ipa-replica-manage, line 677, in main del_master(realm, args[1], options) File /sbin/ipa-replica-manage, line 476, in del_master sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e))) UnboundLocalError: local variable 'delrepl' referenced before assignment I also hit this error when removing a winsync replica. 2) As I wrote before, I think having --force option override the user inquiries would benefit test automation: +if not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) 3) I don't think this code won't cover this topology: A - B - C - D - E It would allow you deleting a replica C even though it would separate A-B and D-E. Though we may not want to cover this situation now, what you got is definitely helping. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
Martin Kosek wrote: On 08/31/2012 07:40 PM, Rob Crittenden wrote: Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob 1) When I run ipa-replica-manage del --force to an already uninstalled host, the new code will prevent me the deletation because it cannot connect to it. It also crashes with UnboundLocalError: # ipa-replica-manage del vm-055.idm.lab.bos.redhat.com --force Unable to connect to replica vm-055.idm.lab.bos.redhat.com, forcing removal Traceback (most recent call last): File /sbin/ipa-replica-manage, line 708, in module main() File /sbin/ipa-replica-manage, line 677, in main del_master(realm, args[1], options) File /sbin/ipa-replica-manage, line 476, in del_master sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e))) UnboundLocalError: local variable 'delrepl' referenced before assignment Fixed. I also hit this error when removing a winsync replica. Fixed. 2) As I wrote before, I think having --force option override the user inquiries would benefit test automation: +if not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) Fixed. 3) I don't think this code won't cover this topology: A - B - C - D - E It would allow you deleting a replica C even though it would separate A-B and D-E. Though we may not want to cover this situation now, what you got is definitely helping. I think you may be right. I only tested with 4 servers. With this B and D would both still have 2 agreements so wouldn't be covered by the last link test. rob From 66217dd61b8271d6282eaad729c92e6bf961123a Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Fri, 31 Aug 2012 11:56:58 -0400 Subject: [PATCH] When deleting a master, try to prevent orphaning other servers. If you have a replication topology like A - B - C and you try to delete server B that will leave A and C orphaned. It may also prevent re-installation of a new master on B because the cn=masters entry for it probably still exists on at least one of the other masters. Check on each master that it connects to to ensure that it isn't the last link, and fail if it is. If any of the masters are not up then warn that this could be a bad thing but let the user continue if they want. Document how to remove a cn=masters entry in the man page. https://fedorahosted.org/freeipa/ticket/2797 --- install/tools/ipa-replica-manage | 66 ++ install/tools/man/ipa-replica-manage.1 | 12 +++ 2 files changed, 78 insertions(+) diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage index c6ef51b7215164c9538afae942e3d42285ca860b..24a33bfb5ea51035eb12eaf7944a7e566640c2ff 100755 --- a/install/tools/ipa-replica-manage +++ b/install/tools/ipa-replica-manage @@ -398,9 +398,52 @@ def clean_ruv(realm, ruv, options): thisrepl.cleanallruv(ruv) print Cleanup task created +def check_last_link(delrepl, realm, dirman_passwd, force): + +We don't want to orphan a server when deleting another one. If you have +A topology that looks like this: + + A B + | | + | | + | | + C D + +If we try to delete host D it will orphan host B. + +What we need to do is if the master being deleted has only a single +agreement, connect to that master and make sure it has agreements with +more than just this master. + +@delrepl: a ReplicationManager object of the master being deleted + +returns: hostname of orphaned server or None + +replica_names = delrepl.find_ipa_replication_agreements() + +orphaned = [] +# Connect to each remote server and see what agreements it has +for replica in replica_names: +try: +repl = replication.ReplicationManager(realm, replica, dirman_passwd) +except ldap.SERVER_DOWN, e: +print Unable to validate that '%s' will not be orphaned. % replica +if not force and not ipautil.user_input(Continue to delete?, False): +sys.exit(Aborted) +continue +names = repl.find_ipa_replication_agreements() +if len(names) == 1 and names[0] == delrepl.hostname: +orphaned.append(replica) + +if len(orphaned): +return ', '.join(orphaned) +else: +return None + def del_master(realm,
[Freeipa-devel] [PATCH] 1050 prevent replica orphans
It was possible use ipa-replica-manage connect/disconnect/del to end up
orphaning or or more IPA masters. This is an attempt to catch and
prevent that case.
I tested with this topology, trying to delete B.
A - B - C
I got here by creating B and C from A, connecting B to C then deleting
the link from A to B, so it went from A - B and A - C to the above.
What I do is look up the servers that the delete candidate host has
connections to and see if we're the last link.
I added an escape clause if there are only two masters.
rob
From 9260967f3ef9df090df86e9705596b7cb4d322b6 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 31 Aug 2012 11:56:58 -0400
Subject: [PATCH] When deleting a master, try to prevent orphaning other
servers.
If you have a replication topology like A - B - C and you try
to delete server B that will leave A and C orphaned. It may also
prevent re-installation of a new master on B because the cn=masters
entry for it probably still exists on at least one of the other masters.
Check on each master that it connects to to ensure that it isn't the
last link, and fail if it is. If any of the masters are not up then
warn that this could be a bad thing but let the user continue if
they want.
Document how to remove a cn=masters entry in the man page.
https://fedorahosted.org/freeipa/ticket/2797
---
install/tools/ipa-replica-manage | 57 ++
install/tools/man/ipa-replica-manage.1 | 12 +++
2 files changed, 69 insertions(+)
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 5023d9a8d761943515c8753237db0ed5c42ab813..21e2868042c9adfbce5ff9ca5ebf8690ab5a76a6 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -373,6 +373,48 @@ def clean_ruv(realm, ruv, options):
thisrepl.cleanallruv(ruv)
print Cleanup task created
+def check_last_link(delrepl, realm, dirman_passwd):
+
+We don't want to orphan a server when deleting another one. If you have
+A topology that looks like this:
+
+ A B
+ | |
+ | |
+ | |
+ C D
+
+If we try to delete host D it will orphan host B.
+
+What we need to do is if the master being deleted has only a single
+agreement, connect to that master and make sure it has agreements with
+more than just this master.
+
+@delrepl: a ReplicationManager object of the master being deleted
+
+returns: hostname of orphaned server or None
+
+replica_names = delrepl.find_ipa_replication_agreements()
+
+orphaned = []
+# Connect to each remote server and see what agreements it has
+for replica in replica_names:
+try:
+repl = replication.ReplicationManager(realm, replica, dirman_passwd)
+except ldap.SERVER_DOWN, e:
+print Unable to validate that '%s' will not be orphaned. % replica
+if not ipautil.user_input(Continue to delete?, False):
+sys.exit(Aborted)
+continue
+names = repl.find_ipa_replication_agreements()
+if len(names) == 1 and names[0] == delrepl.hostname:
+orphaned.append(replica)
+
+if len(orphaned):
+return ', '.join(orphaned)
+else:
+return None
+
def del_master(realm, hostname, options):
force_del = False
@@ -426,6 +468,21 @@ def del_master(realm, hostname, options):
if not ipautil.user_input(Continue to delete?, False):
sys.exit(Deletion aborted)
+# Check for orphans
+masters_dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ipautil.realm_to_suffix(realm))
+try:
+masters = delrepl.conn.getList(masters_dn, ldap.SCOPE_ONELEVEL)
+except:
+sys.exit(Failed read master data from '%s': %s % (delrepl.hostname, str(e)))
+# This only applies if we have more than 2 IPA servers, otherwise there is
+# no chance of an orphan.
+if len(masters) 2:
+orphaned_server = check_last_link(delrepl, realm, options.dirman_passwd)
+if orphaned_server is not None:
+print Deleting this server will orphan '%s'. % orphaned_server
+print You will need to reconfigure your replication topology to delete this server.
+sys.exit(1)
+
# 4. Remove each agreement
for r in replica_names:
try:
diff --git a/install/tools/man/ipa-replica-manage.1 b/install/tools/man/ipa-replica-manage.1
index 4a1c489f33591ff6ac98fe7f9a16ebb6a52ee28a..3eeadd8d6f5af61d9890994f7cadf3acfdc2f3e0 100644
--- a/install/tools/man/ipa-replica-manage.1
+++ b/install/tools/man/ipa-replica-manage.1
@@ -59,6 +59,18 @@ Each IPA master server has a unique replication ID. This ID is used by 389\-ds\-
When a master is removed, all other masters need to remove its replication ID from the list of masters. Normally this occurs automatically when a master is deleted with
Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans
Rob Crittenden wrote: It was possible use ipa-replica-manage connect/disconnect/del to end up orphaning or or more IPA masters. This is an attempt to catch and prevent that case. I tested with this topology, trying to delete B. A - B - C I got here by creating B and C from A, connecting B to C then deleting the link from A to B, so it went from A - B and A - C to the above. What I do is look up the servers that the delete candidate host has connections to and see if we're the last link. I added an escape clause if there are only two masters. rob Oh, this relies on my cleanruv patch 1031. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
