Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans

2012-09-17 Thread Martin Kosek
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

2012-09-17 Thread Martin Kosek
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

2012-09-14 Thread Martin Kosek
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

2012-09-14 Thread Rob Crittenden

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

2012-09-10 Thread Rob Crittenden

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

2012-09-07 Thread Martin Kosek
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

2012-09-06 Thread Martin Kosek
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

2012-09-06 Thread Rob Crittenden

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, 

Re: [Freeipa-devel] [PATCH] 1050 prevent replica orphans

2012-08-31 Thread Rob Crittenden

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