Refactor some replication code

This simplifies or rationalizes some code in order to make it easier to change
it to fix bug #690

3 files changed, 140 insertions, 158 deletions

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index da2c9d4a7..9d8f15107 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -54,18 +54,19 @@ def parse_options():
     parser.add_option("-f", "--force", dest="force", action="store_true", default=False,
                       help="ignore some types of errors")
     parser.add_option("--port", type="int", dest="port",
+                      default=replication.PORT,
                       help="port number of other server")
-    parser.add_option("--binddn", dest="binddn",
+    parser.add_option("--binddn", dest="binddn", default=None,
                       help="Bind DN to use with remote server")
-    parser.add_option("--bindpw", dest="bindpw",
+    parser.add_option("--bindpw", dest="bindpw", default=None,
                       help="Password for Bind DN to use with remote server")
     parser.add_option("--winsync", dest="winsync", action="store_true", default=False,
                       help="This is a Windows Sync Agreement")
-    parser.add_option("--cacert", dest="cacert",
+    parser.add_option("--cacert", dest="cacert", default=None,
                       help="Full path and filename of CA certificate to use with TLS/SSL to the remote server")
-    parser.add_option("--win-subtree", dest="win_subtree",
+    parser.add_option("--win-subtree", dest="win_subtree", default=None,
                       help="DN of Windows subtree containing the users you want to sync (default cn=Users,<domain suffix)")
-    parser.add_option("--passsync", dest="passsync",
+    parser.add_option("--passsync", dest="passsync", default=None,
                       help="Password for the Windows PassSync user")
     parser.add_option("--from", dest="fromhost", help="Host to get data from")
 
@@ -102,15 +103,7 @@ def parse_options():
         # else user has already configured logging externally lower
     return options, args
 
-def get_realm_name():
-    c = krbV.default_context()
-    return c.default_realm
-
-def get_suffix():
-    suffix = util.realm_to_suffix(get_realm_name())
-    return ipaldap.IPAdmin.normalizeDN(suffix)
-
-def test_connection(host):
+def test_connection(realm, host):
     """
     Make a GSSAPI connection to the remote LDAP server to test out credentials.
 
@@ -119,19 +112,18 @@ def test_connection(host):
     returns True if connection successful, False otherwise
     """
     try:
-        replman = replication.ReplicationManager(host, None)
+        replman = replication.ReplicationManager(realm, host, None)
         ents = replman.find_replication_agreements()
         del replman
         return True
     except ldap.LOCAL_ERROR:
         return False
 
-def list_masters(host, replica, dirman_passwd, verbose):
+def list_masters(realm, host, replica, dirman_passwd, verbose):
 
     if replica:
         try:
-            repl = replication.ReplicationManager(replica, dirman_passwd)
-            repl.suffix = get_suffix()
+            repl = replication.ReplicationManager(realm, replica, dirman_passwd)
         except Exception, e:
             print "Failed to get data from '%s': %s" % (replica, str(e))
             return
@@ -154,7 +146,7 @@ def list_masters(host, replica, dirman_passwd, verbose):
             else:
                 conn.sasl_interactive_bind_s('', ipaldap.sasl_auth)
 
-            dn = 'cn=masters,cn=ipa,cn=etc,%s' % get_suffix()
+            dn = 'cn=masters,cn=ipa,cn=etc,%s' % util.realm_to_suffix(realm)
             entries = conn.search_s(dn, ldap.SCOPE_ONELEVEL)
 
             for ent in entries:
@@ -164,13 +156,12 @@ def list_masters(host, replica, dirman_passwd, verbose):
             print "Failed to get data from '%s': %s" % (host, str(e))
             return
 
-def del_link(replica1, replica2, dirman_passwd, force=False):
+def del_link(realm, replica1, replica2, dirman_passwd, force=False):
 
     repl2 = None
 
     try:
-        repl1 = replication.ReplicationManager(replica1, dirman_passwd)
-        repl1.suffix = get_suffix()
+        repl1 = replication.ReplicationManager(realm, replica1, dirman_passwd)
 
         type1 = repl1.get_agreement_type(replica2)
 
@@ -192,8 +183,7 @@ def del_link(replica1, replica2, dirman_passwd, force=False):
 
     if type1 == replication.IPA_REPLICA:
         try:
-            repl2 = replication.ReplicationManager(replica2, dirman_passwd)
-            repl2.suffix = get_suffix()
+            repl2 = replication.ReplicationManager(realm, replica2, dirman_passwd)
 
             repl_list = repl1.find_ipa_replication_agreements()
             if not force and len(repl_list) <= 1:
@@ -240,14 +230,13 @@ def del_link(replica1, replica2, dirman_passwd, force=False):
     repl1.delete_agreement(replica2)
     repl1.delete_referral(replica2)
 
-def del_master(hostname, options):
+def del_master(realm, hostname, options):
 
     force_del = False
 
     # 1. Connect to the master to be removed.
     try:
-        delrepl = replication.ReplicationManager(hostname, options.dirman_passwd)
-        delrepl.suffix = get_suffix()
+        delrepl = replication.ReplicationManager(realm, hostname, options.dirman_passwd)
     except Exception, e:
         if not options.force:
             print "Unable to delete replica %s: %s" % (hostname, str(e))
@@ -258,14 +247,14 @@ def del_master(hostname, options):
 
     # 2. Connect to the local server
     try:
-        thisrepl = replication.ReplicationManager(options.host,
+        thisrepl = replication.ReplicationManager(realm, options.host,
                                                   options.dirman_passwd)
     except Exception, e:
         print "Failed to connect to server %s: %s" % (options.host, str(e))
         sys.exit(1)
 
     if force_del:
-        dn = 'cn=masters,cn=ipa,cn=etc,%s' % get_suffix()
+        dn = 'cn=masters,cn=ipa,cn=etc,%s' % thisrepl.suffix
         res = thisrepl.conn.search_s(dn, ldap.SCOPE_ONELEVEL)
         replica_names = []
         for entry in res:
@@ -277,41 +266,27 @@ def del_master(hostname, options):
     # 3. Remove each agreement
     for r in replica_names:
         try:
-            del_link(r, hostname, options.dirman_passwd, force=True)
+            del_link(realm, r, hostname, options.dirman_passwd, force=True)
         except Exception, e:
             print "There were issues removing a connection: %s" % str(e)
 
     # 4. Finally clean up the removed replica common entries.
     try:
-        thisrepl.replica_cleanup(hostname, get_realm_name(), force=True)
+        thisrepl.replica_cleanup(hostname, realm, force=True)
     except Exception, e:
         print "Failed to cleanup %s entries: %s" % (hostname, str(e))
         print "You may need to manually remove them from the tree"
 
-def add_link(replica1, replica2, dirman_passwd, options):
+def add_link(realm, replica1, replica2, dirman_passwd, options):
 
-    other_args = {}
-    if options.port:
-        other_args['port'] = options.port
-    if options.binddn:
-        other_args['binddn'] = options.binddn
-    if options.bindpw:
-        other_args['bindpw'] = options.bindpw
-    if options.cacert:
-        other_args['cacert'] = options.cacert
-    if options.win_subtree:
-        other_args['win_subtree'] = options.win_subtree
-    if options.passsync:
-        other_args['passsync'] = options.passsync
     if options.winsync:
-        other_args['winsync'] = True
         if not options.binddn or not options.bindpw or not options.cacert or not options.passsync:
             logging.error("The arguments --binddn, --bindpw, --passsync and --cacert are required to create a winsync agreement")
             sys.exit(1)
 
     if options.cacert:
         # have to install the given CA cert before doing anything else
-        ds = dsinstance.DsInstance(realm_name = get_realm_name(),
+        ds = dsinstance.DsInstance(realm_name = realm,
                                    dm_password = dirman_passwd)
         if not ds.add_ca_cert(options.cacert):
             print "Could not load the required CA certificate file [%s]" % options.cacert
@@ -322,8 +297,7 @@ def add_link(replica1, replica2, dirman_passwd, options):
     # need to wait until cacert is installed as that command may restart
     # the directory server and kill the connection
     try:
-        repl1 = replication.ReplicationManager(replica1, dirman_passwd)
-        repl1.suffix = get_suffix()
+        repl1 = replication.ReplicationManager(realm, replica1, dirman_passwd)
 
     except ldap.NO_SUCH_OBJECT:
         print "Cannot find replica '%s'" % replica1
@@ -335,17 +309,22 @@ def add_link(replica1, replica2, dirman_passwd, options):
         print "Failed to get data from '%s': %s" % (replica1, str(e))
         return
 
-    repl1.setup_replication(replica2, get_realm_name(), **other_args)
+    if options.winsync:
+        repl1.setup_winsync_replication(replica2,
+                                        options.binddn, options.bindpw,
+                                        options.passsync, options.win_subtree,
+                                        options.cacert)
+    else:
+        repl1.setup_replication(replica2, "cn=Directory Manager", dirman_passwd)
     print "Connected '%s' to '%s'" % (replica1, replica2)
 
-def re_initialize(options):
+def re_initialize(realm, options):
 
     if not options.fromhost:
         print "re-initialize requires the option --from <host name>"
         sys.exit(1)
 
-    repl = replication.ReplicationManager(options.fromhost, options.dirman_passwd)
-    repl.suffix = get_suffix()
+    repl = replication.ReplicationManager(realm, options.fromhost, options.dirman_passwd)
 
     thishost = installutils.get_fqdn()
 
@@ -360,13 +339,12 @@ def re_initialize(options):
     repl.initialize_replication(entry[0].dn, repl.conn)
     repl.wait_for_repl_init(repl.conn, entry[0].dn)
 
-    ds = dsinstance.DsInstance(realm_name = get_realm_name(), dm_password = options.dirman_passwd)
+    ds = dsinstance.DsInstance(realm_name = realm, dm_password = options.dirman_passwd)
     ds.init_memberof()
 
-def force_sync(thishost, fromhost, dirman_passwd):
+def force_sync(realm, thishost, fromhost, dirman_passwd):
 
-    repl = replication.ReplicationManager(fromhost, dirman_passwd)
-    repl.suffix = get_suffix()
+    repl = replication.ReplicationManager(realm, fromhost, dirman_passwd)
 
     filter = "(&(nsDS5ReplicaHost=%s)(|(objectclass=nsDSWindowsReplicationAgreement)(objectclass=nsds5ReplicationAgreement)))" % thishost
     entry = repl.conn.search_s("cn=config", ldap.SCOPE_SUBTREE, filter)
@@ -381,6 +359,7 @@ def main():
     options, args = parse_options()
 
     dirman_passwd = None
+    realm = krbV.default_context().default_realm
 
     if options.host:
         host = options.host
@@ -392,7 +371,7 @@ def main():
     if options.dirman_passwd:
         dirman_passwd = options.dirman_passwd
     else:
-        if not test_connection(host) or args[0] == "connect":
+        if not test_connection(realm, host) or args[0] == "connect":
             dirman_passwd = getpass.getpass("Directory Manager password: ")
 
     options.dirman_passwd = dirman_passwd
@@ -401,16 +380,16 @@ def main():
         replica = None
         if len(args) == 2:
             replica = args[1]
-        list_masters(host, replica, dirman_passwd, options.verbose)
+        list_masters(realm, host, replica, dirman_passwd, options.verbose)
     elif args[0] == "del":
-        del_master(args[1], options)
+        del_master(realm, args[1], options)
     elif args[0] == "re-initialize":
-        re_initialize(options)
+        re_initialize(realm, options)
     elif args[0] == "force-sync":
         if not options.fromhost:
             print "force-sync requires the option --from <host name>"
             sys.exit(1)
-        force_sync(host, options.fromhost, options.dirman_passwd)
+        force_sync(realm, host, options.fromhost, options.dirman_passwd)
     elif args[0] == "connect":
         if len(args) == 3:
             replica1 = args[1]
@@ -418,7 +397,7 @@ def main():
         elif len(args) == 2:
             replica1 = host
             replica2 = args[1]
-        add_link(replica1, replica2, dirman_passwd, options)
+        add_link(realm, replica1, replica2, dirman_passwd, options)
     elif args[0] == "disconnect":
         if len(args) == 3:
             replica1 = args[1]
@@ -426,7 +405,7 @@ def main():
         elif len(args) == 2:
             replica1 = host
             replica2 = args[1]
-        del_link(replica1, replica2, dirman_passwd)
+        del_link(realm, replica1, replica2, dirman_passwd)
 
 try:
     main()
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 73bc8b0c1..284bf2429 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -287,14 +287,12 @@ class DsInstance(service.Service):
 
 
     def __setup_replica(self):
-        try:
-            repl = replication.ReplicationManager(self.fqdn, self.dm_password)
-            ret = repl.setup_replication(self.master_fqdn, self.realm_name)
-        except Exception, e:
-            logging.debug("Connection error: %s" % e)
-            raise RuntimeError("Unable to connect to LDAP server %s." % self.fqdn)
-        if ret != 0:
-            raise RuntimeError("Failed to start replication")
+        repl = replication.ReplicationManager(self.realm_name,
+                                              self.fqdn,
+                                              self.dm_password)
+        repl.setup_replication(self.master_fqdn,
+                               "cn=Directory Manager",
+                               self.dm_password)
 
     def __enable(self):
         self.backup_state("enabled", self.is_enabled())
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index cf24f5bfd..3e467a9e7 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -56,9 +56,11 @@ def check_replication_plugin():
 class ReplicationManager:
     """Manage replication agreements between DS servers, and sync
     agreements with Windows servers"""
-    def __init__(self, hostname, dirman_passwd):
+    def __init__(self, realm, hostname, dirman_passwd):
         self.hostname = hostname
         self.dirman_passwd = dirman_passwd
+        tmp = util.realm_to_suffix(realm)
+        self.suffix = ipaldap.IPAdmin.normalizeDN(tmp)
 
         # If we are passed a password we'll use it as the DM password
         # otherwise we'll do a GSSAPI bind.
@@ -74,7 +76,6 @@ class ReplicationManager:
         # at runtime if you really want
         self.repl_man_dn = "cn=replication manager,cn=config"
         self.repl_man_cn = "replication manager"
-        self.suffix = ""
 
     def _get_replica_id(self, conn, master_conn):
         """
@@ -157,25 +158,25 @@ class ReplicationManager:
 
         return res
 
-    def add_replication_manager(self, conn, passwd=None):
+    def add_replication_manager(self, conn, dn, pw):
         """
-        Create a pseudo user to use for replication. If no password
-        is provided the directory manager password will be used.
+        Create a pseudo user to use for replication.
         """
 
-        if passwd:
-            self.repl_man_passwd = passwd
+        edn = ldap.dn.str2dn(dn)
+        rdn_attr = edn[0][0][0]
+        rdn_val = edn[0][0][1]
 
-        ent = ipaldap.Entry(self.repl_man_dn)
+        ent = ipaldap.Entry(dn)
         ent.setValues("objectclass", "top", "person")
-        ent.setValues("cn", self.repl_man_cn)
-        ent.setValues("userpassword", self.repl_man_passwd)
+        ent.setValues(rdn_attr, rdn_val)
+        ent.setValues("userpassword", pw)
         ent.setValues("sn", "replication manager pseudo user")
 
         try:
             conn.add_s(ent)
         except ldap.ALREADY_EXISTS:
-            # should we set the password here?
+            conn.modify_s(dn, [(ldap.MOD_REPLACE, "userpassword", pw)])
             pass
 
     def delete_replication_manager(self, conn, dn="cn=replication manager,cn=config"):
@@ -193,7 +194,7 @@ class ReplicationManager:
     def replica_dn(self):
         return 'cn=replica, cn="%s", cn=mapping tree, cn=config' % self.suffix
 
-    def local_replica_config(self, conn, replica_id):
+    def replica_config(self, conn, replica_id, replica_binddn):
         dn = self.replica_dn()
 
         try:
@@ -212,7 +213,7 @@ class ReplicationManager:
         entry.setValues('nsds5replicaid', str(replica_id))
         entry.setValues('nsds5replicatype', replica_type)
         entry.setValues('nsds5flags', "1")
-        entry.setValues('nsds5replicabinddn', [self.repl_man_dn])
+        entry.setValues('nsds5replicabinddn', [replica_binddn])
         entry.setValues('nsds5replicalegacyconsumer', "off")
 
         conn.add_s(entry)
@@ -338,22 +339,18 @@ class ReplicationManager:
         except ldap.TYPE_OR_VALUE_EXISTS:
             logging.debug("passsync aci already exists in suffix %s on %s" % (self.suffix, conn.host))
 
-    def setup_winsync_agmt(self, entry, **kargs):
+    def setup_winsync_agmt(self, entry, win_subtree=None):
+        if win_subtree is None:
+            win_subtree = WIN_USER_CONTAINER + "," + self.suffix
+        ds_subtree = IPA_USER_CONTAINER + "," + self.suffix
+        windomain = '.'.join(ldap.explode_dn(self.suffix, 1))
+
         entry.setValues("objectclass", "nsDSWindowsReplicationAgreement")
-        entry.setValues("nsds7WindowsReplicaSubtree",
-                        kargs.get("win_subtree",
-                                  WIN_USER_CONTAINER + "," + self.suffix))
-        entry.setValues("nsds7DirectoryReplicaSubtree",
-                        kargs.get("ds_subtree",
-                                 IPA_USER_CONTAINER + "," + self.suffix))
+        entry.setValues("nsds7WindowsReplicaSubtree", win_subtree)
+        entry.setValues("nsds7DirectoryReplicaSubtree", ds_subtree)
         # for now, just sync users and ignore groups
-        entry.setValues("nsds7NewWinUserSyncEnabled", kargs.get('newwinusers', 'true'))
-        entry.setValues("nsds7NewWinGroupSyncEnabled", kargs.get('newwingroups', 'false'))
-        windomain = ''
-        if kargs.has_key('windomain'):
-            windomain = kargs['windomain']
-        else:
-            windomain = '.'.join(ldap.explode_dn(self.suffix, 1))
+        entry.setValues("nsds7NewWinUserSyncEnabled", 'true')
+        entry.setValues("nsds7NewWinGroupSyncEnabled", 'false')
         entry.setValues("nsds7WindowsDomain", windomain)
 
     def agreement_dn(self, hostname, port=PORT):
@@ -362,7 +359,9 @@ class ReplicationManager:
 
         return (cn, dn)
 
-    def setup_agreement(self, a, b, **kargs):
+    def setup_agreement(self, a, b,
+                        repl_man_dn=None, repl_man_passwd=None,
+                        iswinsync=False, win_subtree=None):
         cn, dn = self.agreement_dn(b.host)
         try:
             a.getEntry(dn, ldap.SCOPE_BASE)
@@ -370,10 +369,11 @@ class ReplicationManager:
         except errors.NotFound:
             pass
 
-        iswinsync = kargs.get("winsync", False)
-        repl_man_dn = kargs.get("binddn", self.repl_man_dn)
-        repl_man_passwd = kargs.get("bindpw", self.repl_man_passwd)
-        port = kargs.get("port", PORT)
+        port = PORT
+        if repl_man_dn is None:
+            repl_man_dn = self.repl_man_dn
+        if repl_man_passwd is None:
+            repl_man_passwd = self.repl_man_passwd
 
         # List of attributes that need to be excluded from replication.
         excludes = ('memberof', 'entryusn',
@@ -397,7 +397,7 @@ class ReplicationManager:
                         '(objectclass=*) $ EXCLUDE %s' % " ".join(excludes))
         entry.setValues('description', "me to %s%d" % (b.host, port))
         if iswinsync:
-            self.setup_winsync_agmt(entry, **kargs)
+            self.setup_winsync_agmt(entry, win_subtree)
 
         a.add_s(entry)
 
@@ -517,61 +517,70 @@ class ReplicationManager:
 
         return self.wait_for_repl_init(other_conn, dn)
 
-    def basic_replication_setup(self, conn, replica_id):
-        self.add_replication_manager(conn)
-        self.local_replica_config(conn, replica_id)
+    def basic_replication_setup(self, conn, replica_id, repldn, replpw):
+        self.add_replication_manager(conn, repldn, replpw)
+        self.replica_config(conn, replica_id, repldn)
         self.setup_changelog(conn)
 
-    def setup_replication(self, other_hostname, realm_name, **kargs):
-        """
-        NOTES:
-           - the directory manager password needs to be the same on
-             both directories.  Or use the optional binddn and bindpw
-        """
-        iswinsync = kargs.get("winsync", False)
-        oth_port = kargs.get("port", PORT)
-        oth_cacert = kargs.get("cacert", CACERT)
-        oth_binddn = kargs.get("binddn", DIRMAN_CN)
-        oth_bindpw = kargs.get("bindpw", self.dirman_passwd)
+    def setup_replication(self, r_hostname, r_binddn=None, r_bindpw=None):
         # note - there appears to be a bug in python-ldap - it does not
         # allow connections using two different CA certs
-        other_conn = ipaldap.IPAdmin(other_hostname, port=oth_port, cacert=oth_cacert)
+        r_conn = ipaldap.IPAdmin(r_hostname, port=PORT, cacert=CACERT)
+        if r_bindpw:
+            r_conn.do_simple_bind(binddn=r_binddn, bindpw=r_bindpw)
+        else:
+            r_conn.sasl_interactive_bind_s('', SASL_AUTH)
+
+        #Setup the first half
+        l_id = self._get_replica_id(self.conn, r_conn)
+        self.basic_replication_setup(self.conn, l_id,
+                                     self.repl_man_dn, self.repl_man_passwd)
+
+        # Now setup the other half
+        r_id = self._get_replica_id(r_conn, r_conn)
+        self.basic_replication_setup(r_conn, r_id,
+                                     self.repl_man_dn, self.repl_man_passwd)
+
+        self.setup_agreement(r_conn, self.conn)
+        self.setup_agreement(self.conn, r_conn)
+
+        #Finally start replication
+        ret = self.start_replication(r_conn)
+        if ret != 0:
+            raise RuntimeError("Failed to start replication")
+
+    def setup_winsync_replication(self,
+                                  ad_dc_name, ad_binddn, ad_pwd,
+                                  passsync_pw, ad_subtree,
+                                  cacert=CACERT):
         try:
-            if oth_bindpw:
-                other_conn.do_simple_bind(binddn=oth_binddn, bindpw=oth_bindpw)
-            else:
-                other_conn.sasl_interactive_bind_s('', SASL_AUTH)
+            # Validate AD connection
+            ad_conn = ipaldap.IPAdmin(ad_dc_name, port=636, cacert=cacert)
+            ad_conn.do_simple_bind(binddn=ad_binddn, bindpw=ad_pwd)
         except Exception, e:
-            if iswinsync:
-                logging.info("Could not validate connection to remote server %s:%d - continuing" %
-                             (other_hostname, oth_port))
-                logging.info("The error was: %s" % e)
-            else:
-                raise e
-
-        self.suffix = ipaldap.IPAdmin.normalizeDN(util.realm_to_suffix(realm_name))
-
-        if not iswinsync:
-            local_id = self._get_replica_id(self.conn, other_conn)
-        else:
-            # there is no other side to get a replica ID from
-            local_id = self._get_replica_id(self.conn, self.conn)
-        self.basic_replication_setup(self.conn, local_id)
-
-        if not iswinsync:
-            other_id = self._get_replica_id(other_conn, other_conn)
-            self.basic_replication_setup(other_conn, other_id)
-            self.setup_agreement(other_conn, self.conn)
-            self.setup_agreement(self.conn, other_conn)
-            return self.start_replication(other_conn)
-        else:
-            self.add_passsync_user(self.conn, kargs.get("passsync"))
-            self.setup_agreement(self.conn, other_conn, **kargs)
-            logging.info("Added new sync agreement, waiting for it to become ready . . .")
-            cn, dn = self.agreement_dn(other_hostname)
-            self.wait_for_repl_update(self.conn, dn, 30)
-            logging.info("Agreement is ready, starting replication . . .")
-            return self.start_replication(self.conn, other_conn)
+            logging.info("Failed to connect to AD server %s" % ad_dc_name)
+            logging.info("The error was: %s" % e)
+            logging.info("Continuning ...")
+
+        # Setup the only half.
+        # there is no other side to get a replica ID from
+        # So we generate one locally
+        replica_id = self._get_replica_id(self.conn, self.conn)
+        self.basic_replication_setup(self.conn, replica_id)
+
+        #now add a passync user allowed to access the AD server
+        self.add_passsync_user(self.conn, passsync_pw)
+        self.setup_agreement(self.conn, ad_conn,
+                             repl_man_dn=ad_binddn, repl_man_passwd=ad_pwd,
+                             iswinsync=True, win_subtree=ad_subtree)
+        logging.info("Added new sync agreement, waiting for it to become ready . . .")
+        cn, dn = self.agreement_dn(ad_dc_name)
+        self.wait_for_repl_update(self.conn, dn, 30)
+        logging.info("Agreement is ready, starting replication . . .")
+
+        #Finally start replication
+        return self.start_replication(self.conn, ad_conn,
+                                      self.repl_man_dn, self.repl_man_passwd)
 
     def initialize_replication(self, dn, conn):
         mod = [(ldap.MOD_ADD, 'nsds5BeginReplicaRefresh', 'start')]
@@ -618,10 +627,6 @@ class ReplicationManager:
         if replica == self.hostname:
             raise RuntimeError("Can't cleanup self")
 
-        if not self.suffix or self.suffix == "":
-            self.suffix = util.realm_to_suffix(realm)
-            self.suffix = ipaldap.IPAdmin.normalizeDN(self.suffix)
-
         # delete master kerberos key and all its svc principals
         try:
             filter='(krbprincipalname=*/%s@%s)' % (replica, realm)