Martin Kosek wrote:
On 10/08/2012 05:12 PM, Jan Cholasta wrote:
Hi,

On 20.9.2012 19:38, Rob Crittenden wrote:
Jan Cholasta wrote:
Hi,

Dne 31.8.2012 19:43, Rob Crittenden napsal(a):
The naming in CS replication agreements is different from IPA
agreements, we have to live with what the create. The master side should
be on the local side, replica1, not the remote. This required reversing
a few master variables.

Pass in the force flag to del_link.

Do a better job of finding the agreements on each side.

This should be ipa-csreplica-manage more in line with
ipa-replica-manage.

rob


Rob, can you please rebase the patch on top of current master? There
were some dogtag 10 related changes to ipa-csreplica-manage since you
posted the patch.

Honza


I re-tested after the merge and found some problems with my initial
approach. The problem stems from the naming convention that dogtag uses
when creating the initial agreements. It is hard to predict how things
were set up later so rather than trying to reconstruct the DN we search
for it and pass it when deleting agreements.

rob

So far I have found this:

   * Deleting a "bridge" link that connects two "islands" of replicas works, but
it should not (I was told that this is expected, as no complex graph algorithms
are engaged to detect this kind of errors).

Exactly, I hit this error when I was a similar Rob's patch for
ipa-replica-manage (ticket 2797). I used topology "A - B - C - D - E" and I was
able to delete C. We may want to log an enhancement ticket for this.

Martin


Right, and ipa-csreplica-manage doesn't even have the basic last_link checking code that ipa-replica-manage has, nor the ruv code. We decided to push that to a future release.

This patch should fix up the basics.

rob
>From 82abfb2f235bd8f01a29fa08f41c464f7efd9349 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Thu, 30 Aug 2012 16:24:10 -0400
Subject: [PATCH] Fix CS replication management.

The master side should be on the local side, replica1, not the
remote. This required reversing a few master variables. This impacts
the naming of the replication agreements.

When deleting an agreement pass in the DN of that agreement
rather than trying to calculate what it is on-the-fly. We cannot
be sure which side is the master/clone and since we search for it
anyway to determine if the agreement exists it is more correct
to use what we find.

The force flag wasn't being passed into del_link so there was no way
to force a deletion.

https://fedorahosted.org/freeipa/ticket/2858
---
 install/tools/ipa-csreplica-manage | 58 ++++++++++++++++++++++++++------------
 ipaserver/install/replication.py   | 22 +++++++++++----
 2 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/install/tools/ipa-csreplica-manage b/install/tools/ipa-csreplica-manage
index 39cfa58511ae552cae64798c7559303fda27866a..5fce48a3759a0de55193a2d025f3af4543ddb32e 100755
--- a/install/tools/ipa-csreplica-manage
+++ b/install/tools/ipa-csreplica-manage
@@ -176,7 +176,7 @@ def list_replicas(realm, host, replica, dirman_passwd, verbose):
                 peers[ent.getValue('cn')] = ['CA not configured', '']
 
     except Exception, e:
-        sys.exit("Failed to get data from '%s': %s" % (host, convert_error(e)))
+        sys.exit("Failed to get data from '%s' while trying to list replicas: %s" % (host, convert_error(e)))
     finally:
         conn.unbind_s()
 
@@ -205,18 +205,21 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
         repl1 = CSReplicationManager(realm, replica1, dirman_passwd, PORT, True)
 
         repl1.hostnames = [replica1, replica2]
-        type1 = repl1.get_agreement_type(replica2)
 
-        repl_list = repl1.find_ipa_replication_agreements()
-        if not force and len(repl_list) <= 1:
-            print "Cannot remove the last replication link of '%s'" % replica1
-            print "Please use the 'del' command to remove it from the domain"
-            sys.exit(1)
+        repl_list1 = repl1.find_replication_agreements()
+
+        # Find the DN of the replication agreement to remove
+        replica1_dn = None
+        for e in repl_list1:
+            if e.getValue('nsDS5ReplicaHost') == replica2:
+                replica1_dn = e.dn
+                break
+
+        if replica1_dn is None:
+            sys.exit("'%s' has no replication agreement for '%s'" % (replica1, replica2))
+
+        repl1.hostnames = [replica1, replica2]
 
-    except ldap.NO_SUCH_OBJECT:
-        sys.exit("'%s' has no replication agreement for '%s'" % (replica1, replica2))
-    except errors.NotFound:
-        sys.exit("'%s' has no replication agreement for '%s'" % (replica1, replica2))
     except ldap.SERVER_DOWN, e:
         sys.exit("Unable to connect to %s: %s" % (ipautil.format_netloc(replica1, PORT), convert_error(e)))
     except Exception, e:
@@ -226,12 +229,31 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
         repl2 = CSReplicationManager(realm, replica2, dirman_passwd, PORT, True)
         repl2.hostnames = [replica1, replica2]
 
-        repl_list = repl1.find_ipa_replication_agreements()
+        repl_list = repl2.find_replication_agreements()
+
+        # Now that we've confirmed that both hostnames are vaild, make sure
+        # that we aren't removing the last link from either side.
         if not force and len(repl_list) <= 1:
             print "Cannot remove the last replication link of '%s'" % replica2
             print "Please use the 'del' command to remove it from the domain"
             sys.exit(1)
 
+        if not force and len(repl_list1) <= 1:
+            print "Cannot remove the last replication link of '%s'" % replica1
+            print "Please use the 'del' command to remove it from the domain"
+            sys.exit(1)
+
+        # Find the DN of the replication agreement to remove
+        replica2_dn = None
+        for e in repl_list:
+            if e.getValue('nsDS5ReplicaHost') == replica1:
+                replica2_dn = e.dn
+                break
+
+        # This should never happen
+        if replica2_dn is None:
+            sys.exit("'%s' has no replication agreement for '%s'" % (replica1, replica2))
+
     except ldap.NO_SUCH_OBJECT:
         print "'%s' has no replication agreement for '%s'" % (replica2, replica1)
         if not force:
@@ -248,7 +270,7 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
     if repl2:
         failed = False
         try:
-            repl2.delete_agreement(replica1)
+            repl2.delete_agreement(replica1, replica2_dn)
             repl2.delete_referral(replica1)
         except Exception, e:
             print "Unable to remove agreement on %s: %s" % (replica2, convert_error(e))
@@ -263,7 +285,7 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
     if not repl2 and force:
         print "Forcing removal on '%s'" % replica1
 
-    repl1.delete_agreement(replica2)
+    repl1.delete_agreement(replica2, replica1_dn)
     repl1.delete_referral(replica2)
 
     print "Deleted replication agreement from '%s' to '%s'" % (replica1, replica2)
@@ -324,13 +346,13 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
     except ldap.SERVER_DOWN, e:
         sys.exit("Unable to connect to %s: %s" % (ipautil.format_netloc(replica2, 636), convert_error(e)))
     except Exception, e:
-        sys.exit("Failed to get data from '%s': %s" % (replica1, convert_error(e)))
+        sys.exit("Failed to get data while trying to bind to '%s': %s" % (replica1, convert_error(e)))
 
     try:
         repl1 = CSReplicationManager(realm, replica1, dirman_passwd, PORT, True)
         entries = repl1.find_replication_agreements()
         for e in entries:
-            if replica1 in e.dn or replica2 in e.dn:
+            if e.getValue('nsDS5ReplicaHost') == replica2:
                 sys.exit('This replication agreement already exists.')
         repl1.hostnames = [replica1, replica2]
 
@@ -339,7 +361,7 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
     except ldap.SERVER_DOWN, e:
         sys.exit("Unable to connect to %s %s" % (ipautil.format_netloc(replica1, PORT), convert_error(e)))
     except Exception, e:
-        sys.exit("Failed to get data from '%s': %s" % (replica1, convert_error(e)))
+        sys.exit("Failed to get data from '%s' while trying to get current agreements: %s" % (replica1, convert_error(e)))
 
     repl1.setup_replication(replica2, PORT, 0, DN(('cn', 'Directory Manager')), dirman_passwd, True, True)
     print "Connected '%s' to '%s'" % (replica1, replica2)
@@ -436,7 +458,7 @@ def main():
         elif len(args) == 2:
             replica1 = host
             replica2 = args[1]
-        del_link(realm, replica1, replica2, dirman_passwd)
+        del_link(realm, replica1, replica2, dirman_passwd, options.force)
 
 try:
     main()
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 564097e7653ac57b6c6862a654442aed55c1d457..260e7bba1911eef5600aa57ac5e1c1c5e82b4ebd 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -648,8 +648,20 @@ class ReplicationManager(object):
         except ldap.NO_SUCH_OBJECT:
             pass
 
-    def delete_agreement(self, hostname):
-        cn, dn = self.agreement_dn(hostname)
+    def delete_agreement(self, hostname, dn=None):
+        """
+        Delete a replication agreement.
+
+        @hostname: the hostname of the agreement to remove
+        @dn: optional dn of the agreement to remove
+
+        For IPA agreements we can easily calculate the DN of the agreement
+        to remove. Dogtag agreements are another matter, its agreement
+        names depend entirely on where it is created. In this case it is
+        better to pass the DN in directly.
+        """
+        if dn is None:
+            cn, dn = self.agreement_dn(hostname)
         return self.conn.deleteEntry(dn)
 
     def delete_referral(self, hostname):
@@ -808,11 +820,11 @@ class ReplicationManager(object):
             self.setup_agreement(r_conn, self.conn.host, port=r_port,
                                  repl_man_dn=self.repl_man_dn,
                                  repl_man_passwd=self.repl_man_passwd,
-                                 master=True)
+                                 master=False)
             self.setup_agreement(self.conn, r_hostname, port=r_port,
                                  repl_man_dn=self.repl_man_dn,
                                  repl_man_passwd=self.repl_man_passwd,
-                                 master=False)
+                                 master=True)
         else:
             self.setup_agreement(r_conn, self.conn.host, port=r_port,
                                  repl_man_dn=self.repl_man_dn,
@@ -822,7 +834,7 @@ class ReplicationManager(object):
                                  repl_man_passwd=self.repl_man_passwd)
 
         #Finally start replication
-        ret = self.start_replication(r_conn, master=True)
+        ret = self.start_replication(r_conn, master=False)
         if ret != 0:
             raise RuntimeError("Failed to start replication")
 
-- 
1.7.11.4

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

Reply via email to