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

[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