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 out. It was just easier to stuff it in here than rebasing back, but yeah, its cleaner this way.

No need to check force for clean because it is already required (can't contact the deleted master, it's gone).

rob
>From 0761a3dd30a69c46046c0521b6a59da8840b7f41 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Fri, 14 Sep 2012 15:03:12 -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.

https://fedorahosted.org/freeipa/ticket/2797
---
 install/tools/ipa-replica-manage       | 85 +++++++++++++++++++++++++++++++++-
 install/tools/man/ipa-replica-manage.1 | 14 ++++++
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index acd05271168170ab7403af319062d10a107b28c2..6119db9c4bd6edd68dd48aeead5281f85be2b2b1 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -72,6 +72,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,
@@ -460,9 +462,53 @@ def list_clean_ruv(realm, host, dirman_passwd, verbose):
                 print str(dn)
                 print entry.getValue('nstasklog')
 
+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, hostname, options):
 
     force_del = False
+    delrepl = None
 
     # 1. Connect to the local server
     try:
@@ -475,7 +521,21 @@ def del_master(realm, hostname, options):
     # 2. Ensure we have an agreement with the master
     agreement = thisrepl.get_replication_agreement(hostname)
     if agreement is None:
-        sys.exit("'%s' has no replication agreement for '%s'" % (options.host, hostname))
+        if options.cleanup:
+            """
+            We have no agreement with the current master, so this is a
+            candidate for cleanup. This is VERY dangerous to do because it
+            removes that master from the list of masters. If the master
+            were to try to come back online it wouldn't work at all.
+            """
+            print "Cleaning a master is irreversible."
+            print "This should not normally be require, so use cautiously."
+            if not ipautil.user_input("Continue to clean master?", False):
+                sys.exit("Cleanup aborted")
+            thisrepl.replica_cleanup(hostname, realm, force=True)
+            sys.exit(0)
+        else:
+            sys.exit("'%s' has no replication agreement for '%s'" % (options.host, hostname))
 
     # 3. If an IPA agreement connect to the master to be removed.
     repltype = thisrepl.get_agreement_type(hostname)
@@ -513,6 +573,29 @@ def del_master(realm, hostname, options):
         if not ipautil.user_input("Continue to delete?", False):
             sys.exit("Deletion aborted")
 
+    # Check for orphans if the remote server is up.
+    if delrepl and not winsync:
+        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 Exception, e:
+            masters = []
+            print "Failed to read masters data from '%s': %s" % (delrepl.hostname, convert_error(e))
+            print "Skipping calculation to determine if one or more masters would be orphaned."
+            if not options.force:
+                sys.exit(1)
+
+        # 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, options.force)
+            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)
+    else:
+        print "Skipping calculation to determine if one or more masters would be orphaned."
+
     # Save the RID value before we start deleting
     if repltype == replication.IPA_REPLICA:
         rid = get_rid_by_host(realm, options.host, hostname, options.dirman_passwd)
diff --git a/install/tools/man/ipa-replica-manage.1 b/install/tools/man/ipa-replica-manage.1
index 2a853317ef005f5bd3fcde384c2994f3d426cffc..95ba5df3a09f9fe9473da1b1700db58b4d134398 100644
--- a/install/tools/man/ipa-replica-manage.1
+++ b/install/tools/man/ipa-replica-manage.1
@@ -65,6 +65,14 @@ 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 ipa\-replica\-manage. If one or more masters was down or unreachable when ipa\-replica\-manage was executed then this replica ID may still exist. The clean\-ruv command may be used to clean up an unused replication ID.
 .TP
 \fBNOTE\fR: clean\-ruv is \fBVERY DANGEROUS\fR. Execution against the wrong replication ID can result in inconsistent data on that master. The master should be re\-initialized from another if this happens.
+.TP
+The replication topology is examined when a master is deleted and will attempt to prevent a master from being orphaned. For example, if your topology is A <\-> B <\-> C and you attempt to delete master B it will fail because that would leave masters and A and C orphaned.
+.TP
+The list of masters is stored in cn=masters,cn=ipa,cn=etc,dc=example,dc=com. This should be cleaned up automatically when a master is deleted. If it occurs that you have deleted the master and all the agreements but these entries still exist then you will not be able to re\-install IPA on it, the installation will fail with:
+.TP
+An IPA master host cannot be deleted or disabled using standard commands (host\-del, for example).
+.TP
+An orphaned master may be cleaned up using the del directive with the \-\-cleanup option. This will remove the entries from cn=masters,cn=ipa,cn=etc that otherwise prevent host\-del from working, its dna profile, s4u2proxy configuration, service principals and remove it from the default DUA profile defaultServerList.
 .SH "OPTIONS"
 .TP
 \fB\-H\fR \fIHOST\fR, \fB\-\-host\fR=\fIHOST\fR
@@ -81,6 +89,9 @@ Provide additional information
 \fB\-f\fR, \fB\-\-force\fR
 Ignore some types of errors, don't prompt when deleting a master
 .TP
+\fB\-c\fR, \fB\-\-cleanup\fR
+When deleting a master with the --force flag, remove leftover references to an already deleted master.
+.TP
 \fB\-\-binddn\fR=\fIADMIN_DN\fR
 Bind DN to use with remote server (default is cn=Directory Manager) \- Be careful to quote this value on the command line
 .TP
@@ -135,6 +146,9 @@ List the replication IDs in use:
  # ipa\-replica\-manage list\-ruv
  srv1.example.com:389: 7
  srv2.example.com:389: 4
+.TP
+Remove references to an orphaned and deleted master:
+ # ipa\-replica\-manage del \-\-force \-\-cleanup master.example.com
 .SH "WINSYNC"
 Creating a Windows AD Synchronization agreement is similar to creating an IPA replication agreement, there are just a couple of extra steps.
 
-- 
1.7.11.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to