On 11/30/2015 12:31 PM, Jan Cholasta wrote:
Hi,

On 27.11.2015 14:58, Martin Babinsky wrote:
On 11/19/2015 06:19 PM, Martin Babinsky wrote:
These two patches fix the following tickets:

https://fedorahosted.org/freeipa/ticket/5377
https://fedorahosted.org/freeipa/ticket/5409

I have added a new option '--ignore-disconnected-topology' which forces
IPA master uninstall despite reported errors in topology. I'm not quite
sure if we want to flood ipa-server-install with uninstall-specific
options, maybe it is better to skip the check in unattended mode and
just print a warning about disconnected topology and what to do about
it.

I would like to hear your opinions about this.



Attaching rebased and updated patches.

Patch 0098: LGTM


Patch 0099:

a) This check should be done in Server.__init__() rather than
install_check():

+    if options.ignore_disconnected_topology:
+        print("'--ignore-disconnected-topology' is used only during "
+              "uninstallation")
+        sys.exit(1)


b) s/--ignore-disconnected-topology/--ignore-topology-disconnect/, for
consistency with other options, e.g. --no-ui-redirect.

Maybe even shorten it to --ignore-topology? But we probably don't want
people to use this option much, so it might be better to keep it long?

I would rather leave it with the long option name, it is more apparent what this switch should be around.

c) I'm fine with uninstall options, you can remove the TODO:

+    # TODO: ask jcholast about uninstallation options


Honza


Attaching updated patches.

--
Martin^3 Babinsky
From 9e997fbfa14e66740538f50eace51039226b51c0 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Thu, 19 Nov 2015 17:58:44 +0100
Subject: [PATCH 2/2] implement domain level 1 specific topology checks into
 IPA server uninstaller

When uninstalling domain level 1 master its removal from topology is checked
on remote masters. The uninstaller also checks whether the uninstallation
disconnects the topology and if yes aborts the procedure. The
'--ignore-disconnected-topology' options skips this check.

https://fedorahosted.org/freeipa/ticket/5377
https://fedorahosted.org/freeipa/ticket/5409
---
 install/tools/man/ipa-server-install.1 |   3 +
 ipaserver/install/server/install.py    | 193 ++++++++++++++++++++++++++++-----
 2 files changed, 169 insertions(+), 27 deletions(-)

diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1
index 5c601b123385a30a1bd6962663f8f97b528e805e..b11c9503982ece60b6fa862d2bebf201f49c3b86 100644
--- a/install/tools/man/ipa-server-install.1
+++ b/install/tools/man/ipa-server-install.1
@@ -61,6 +61,9 @@ The maximum user and group id number (default: idstart+199999). If set to zero,
 \fB\-\-no_hbac_allow\fR
 Don't install allow_all HBAC rule. This rule lets any user from any host access any service on any other host. It is expected that users will remove this rule before moving to production.
 .TP
+\fB\-\-ignore-topology-disconnect\fR
+Ignore errors reported when uninstallation of IPA server would lead to disconnected domain level 1 topology.
+.TP
 \fB\-\-no\-ui\-redirect\fR
 Do not automatically redirect to the Web UI.
 .TP
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index bd07793bb295db9d3b03614f44b54ddd2ea20cb5..333697139a4d67cd89daa3713670a38657884570 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -26,7 +26,7 @@ from ipapython.ipautil import (
 from ipaplatform import services
 from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
-from ipalib import api, constants, errors, x509
+from ipalib import api, create_api, constants, errors, x509
 from ipalib.constants import CACERT
 from ipalib.util import validate_domain_name
 import ipaclient.ntpconf
@@ -290,6 +290,110 @@ def common_cleanup(func):
     return decorated
 
 
+def check_master_deleted(api, masters, interactive):
+    try:
+        host_princ = api.Command.host_show(api.env.host)['result']['krbprincipalname'][0]
+    except Exception as e:
+        root_logger.warning(
+            "Failed to get host principal name: {0}".format(e)
+        )
+        return False
+
+    ccache_path = os.path.join('/', 'tmp', 'krb5cc_host')
+    with ipautil.private_ccache(ccache_path):
+        try:
+            ipautil.kinit_keytab(host_princ, paths.KRB5_KEYTAB, ccache_path)
+        except Exception as e:
+            root_logger.error(
+                "Kerberos authentication as '{0}' failed: {1}".format(
+                    host_princ, e
+                )
+            )
+            return False
+
+        last_server = True
+        for master in masters:
+            master_cn = master['cn'][0]
+            if api.env.host == master_cn:
+                continue
+
+            last_server = False
+            master_ldap_uri = u'ldap://{0}'.format(master_cn)
+
+            # initialize remote api
+            remote_api = create_api(mode=None)
+            remote_api.bootstrap(ldap_uri=master_ldap_uri, in_server=True)
+            remote_api.finalize()
+
+            root_logger.debug("Connecting to '{0}'...".format(master_ldap_uri))
+            try:
+                remote_api.Backend.ldap2.connect(ccache=ccache_path)
+                remote_api.Command.server_show(api.env.host)
+                root_logger.debug(
+                    "Server entry '{0}' present on '{1}'".format(
+                        api.env.host, master_cn
+                    )
+                )
+                return False
+            except (errors.NotFound, errors.ACIError):
+                # this may occur because the node was already deleted from the
+                # topology and the host principal doesn't exist
+                root_logger.debug(
+                    "'{0}' was removed from topology".format(
+                        api.env.host
+                    )
+                )
+                return True
+            except errors.NetworkError:
+                # try the next master
+                root_logger.debug(
+                    "Connection to remote master '{0}' failed".format(
+                        master_cn
+                    )
+                )
+            except Exception as e:
+                root_logger.debug(
+                    "Unexpected error when connecting to remote master '{0}': "
+                    "{1}".format(
+                        master_cn, e
+                    )
+                )
+            finally:
+                root_logger.debug("Disconnecting from {0}".format(master_cn))
+
+                if remote_api.Backend.ldap2.isconnected():
+                    remote_api.Backend.ldap2.disconnect()
+
+    # prompt the user if we are not able to determine whether the IPA master
+    # was removed from topology
+    if not last_server:
+        print("WARNING: Failed to determine whether the IPA master was "
+              "already removed from topology.")
+        if (interactive and not user_input("Proceed with uninstallation?", False)):
+            print("Aborted")
+            sys.exit(1)
+
+        return False
+
+    return True
+
+
+def check_topology_connectivity(api, masters, ignore_errors):
+    topo_errors = replication.check_last_link_managed(
+                    api, api.env.host, masters)
+    any_topo_error = any(topo_errors[t][0] or topo_errors[t][1]
+                         for t in topo_errors)
+    if any_topo_error:
+        if ignore_errors:
+            print("Ignoring topology errors and forcing uninstall")
+            return
+
+        print("Uninstallation leads to disconnected topology")
+        print("Use '--ignore-disconnected-topology' to skip this check")
+        print("Aborting uninstallation")
+        sys.exit(1)
+
+
 @common_cleanup
 def install_check(installer):
     options = installer
@@ -1000,35 +1104,59 @@ def uninstall_check(installer):
     else:
         api.Backend.ldap2.connect(autobind=True)
         dns.uninstall_check(options)
+        domain_level = dsinstance.get_domain_level(api)
 
-        rm = replication.ReplicationManager(
-            realm=api.env.realm,
-            hostname=api.env.host,
-            dirman_passwd=None,
-            conn=conn
-        )
-        agreements = rm.find_ipa_replication_agreements()
-
-        if agreements:
-            other_masters = [a.get('cn')[0][4:] for a in agreements]
-            msg = (
-                "\nReplication agreements with the following IPA masters "
-                "found: %s. Removing any replication agreements before "
-                "uninstalling the server is strongly recommended. You can "
-                "remove replication agreements by running the following "
-                "command on any other IPA master:\n" % ", ".join(
-                    other_masters)
-            )
-            cmd = "$ ipa-replica-manage del %s\n" % api.env.host
-            print(textwrap.fill(msg, width=80, replace_whitespace=False))
-            print(cmd)
-            if (installer.interactive and
-                not user_input("Are you sure you want to continue with the "
-                               "uninstall procedure?", False)):
-                print("")
-                print("Aborting uninstall operation.")
+        if domain_level == constants.DOMAIN_LEVEL_0:
+            if options.ignore_disconnected_topology:
+                print("'--ignore-disconnected-topology' option works only in "
+                      "domain level {0}".format(constants.DOMAIN_LEVEL_1))
                 sys.exit(1)
 
+            rm = replication.ReplicationManager(
+                realm=api.env.realm,
+                hostname=api.env.host,
+                dirman_passwd=None,
+                conn=conn
+            )
+            agreements = rm.find_ipa_replication_agreements()
+
+            if agreements:
+                other_masters = [a.get('cn')[0][4:] for a in agreements]
+                msg = (
+                    "\nReplication agreements with the following IPA masters "
+                    "found: %s. Removing any replication agreements before "
+                    "uninstalling the server is strongly recommended. You can "
+                    "remove replication agreements by running the following "
+                    "command on any other IPA master:\n" % ", ".join(
+                        other_masters)
+                )
+                cmd = "$ ipa-replica-manage del %s\n" % api.env.host
+                print(textwrap.fill(msg, width=80, replace_whitespace=False))
+                print(cmd)
+                if (installer.interactive and
+                        not user_input("Are you sure you want to continue with"
+                                       " the uninstall procedure?", False)):
+                    print("")
+                    print("Aborting uninstall operation.")
+                    sys.exit(1)
+        else:
+            masters = api.Command.server_find(u'', sizelimit=0)['result']
+
+            if not check_master_deleted(api, masters,
+                                        not options.unattended):
+                print("WARNING: This IPA master is still a part of the "
+                      "replication topology.")
+                print("To properly remove the master entry and clean "
+                      "up related segments, run:")
+                print("  $ ipa-replica-manage del {0}".format(api.env.host))
+                if (not options.unattended and not user_input(
+                        "Do you want to continue uninstallation?", False)):
+                    print("Aborted")
+                    sys.exit(1)
+
+                check_topology_connectivity(
+                    api, masters, options.ignore_disconnected_topology)
+
     installer._fstore = fstore
     installer._sstore = sstore
 
@@ -1237,6 +1365,12 @@ class Server(BaseServer):
     kasp_db_file = None
     force = None
 
+    ignore_topology_disconnect = Knob(
+        bool, False,
+        description="(domain level 1) ignore disconnected topology during "
+                    "server uninstall",
+    )
+
     def __init__(self, **kwargs):
         super(Server, self).__init__(**kwargs)
 
@@ -1272,6 +1406,11 @@ class Server(BaseServer):
                         "You must specify at least one --forwarder option or "
                         "--no-forwarders option")
 
+        if self.ignore_topology_disconnect and not self.uninstalling:
+            raise RuntimeError(
+                "'--ignore-topology-disconnect' is used only during "
+                "uninstallation")
+
         if self.idmax < self.idstart:
             raise RuntimeError(
                 "idmax (%s) cannot be smaller than idstart (%s)" %
-- 
2.5.0

From bb6905b7332122ba13609751376f60b7924dff25 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Thu, 19 Nov 2015 17:55:23 +0100
Subject: [PATCH 1/2] extract domain level 1 topology-checking code from
 ipa-replica-manage

This facilitates reusability of this code in other components, e.g. IPA server
uninstallers.

https://fedorahosted.org/freeipa/ticket/5409
---
 install/tools/ipa-replica-manage | 112 ++++-----------------------------------
 ipaserver/install/replication.py |  93 ++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 101 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index d74886a5aec809358814e0bd9953abb8880c8915..f125209f85178f465763c44c60d9a67ba188a3be 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -571,103 +571,6 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
         return None
 
 
-def map_masters_to_suffices(masters, suffices):
-    masters_to_suffix = {}
-    suffix_name_to_root = {
-        s['iparepltopoconfroot'][0]: s['cn'][0] for s in suffices
-    }
-
-    for master in masters:
-        managed_suffices = master['iparepltopomanagedsuffix']
-        for suffix in managed_suffices:
-            suffix_name = suffix_name_to_root[suffix]
-            try:
-                masters_to_suffix[suffix_name].append(master)
-            except KeyError:
-                masters_to_suffix[suffix_name] = [master]
-
-    return masters_to_suffix
-
-
-def check_hostname_in_masters(hostname, masters):
-    master_cns = {m['cn'][0] for m in masters}
-    return hostname in master_cns
-
-
-def check_last_link_managed(api, hostname, masters, force):
-    """
-    Check if 'hostname' is safe to delete.
-
-    :returns: a dictionary of topology errors across all suffices in the form
-              {<suffix name>: (<original errors>,
-              <errors after removing the node>)}
-    """
-    suffices = api.Command.topologysuffix_find(u'')['result']
-    suffix_to_masters = map_masters_to_suffices(masters, suffices)
-    topo_errors_by_suffix = {}
-
-    for suffix in suffices:
-        suffix_name = suffix['cn'][0]
-        suffix_members = suffix_to_masters[suffix_name]
-        print("Checking connectivity in topology suffix '{0}'".format(
-            suffix_name))
-        if not check_hostname_in_masters(hostname, suffix_members):
-            print(
-                "'{0}' is not a part of topology suffix '{1}'".format(
-                    hostname, suffix_name
-                )
-            )
-            print("Not checking connectivity")
-            continue
-
-        segments = api.Command.topologysegment_find(
-            suffix_name, sizelimit=0).get('result')
-        graph = create_topology_graph(suffix_to_masters[suffix_name], segments)
-
-        # check topology before removal
-        orig_errors = get_topology_connection_errors(graph)
-        if orig_errors:
-            print("Current topology in suffix '{0}' is disconnected:".format(
-                suffix_name))
-            print("Changes are not replicated to all servers and data are "
-                  "probably inconsistent.")
-            print("You need to add segments to reconnect the topology.")
-            print_connect_errors(orig_errors)
-
-        # after removal
-        try:
-            graph.remove_vertex(hostname)
-        except ValueError:
-            pass  # ignore already deleted master, continue to clean
-
-        new_errors = get_topology_connection_errors(graph)
-        if new_errors:
-            print("WARNING: Removal of '{0}' will lead to disconnected "
-                  "topology in suffix '{1}'".format(hostname, suffix_name))
-            print("Changes will not be replicated to all servers and data will"
-                  " become inconsistent.")
-            print("You need to add segments to prevent disconnection of the "
-                  "topology.")
-            print("Errors in topology after removal:")
-            print_connect_errors(new_errors)
-
-        if orig_errors or new_errors:
-            if not force:
-                sys.exit("Aborted")
-            else:
-                print("Forcing removal of %s" % hostname)
-
-        topo_errors_by_suffix[suffix_name] = (orig_errors, new_errors)
-
-    return topo_errors_by_suffix
-
-
-def print_connect_errors(errors):
-    for error in errors:
-        print("Topology does not allow server %s to replicate with servers:" % error[0])
-        for srv in error[2]:
-            print("    %s" % srv)
-
 def enforce_host_existence(host, message=None):
     if host is not None and not ipautil.host_exists(host):
         if message is None:
@@ -786,8 +689,15 @@ def del_master_managed(realm, hostname, options):
     masters = api.Command.server_find('', sizelimit=0)['result']
 
     # 3. Check topology connectivity in all suffices
-    topo_errors = check_last_link_managed(
-        api, hostname, masters, options.force)
+    topo_errors = replication.check_last_link_managed(api, hostname, masters)
+
+    any_topo_error = any(topo_errors[t][0] or topo_errors[t][1]
+                         for t in topo_errors)
+    if any_topo_error:
+        if not options.force:
+            sys.exit("Aborted")
+        else:
+            print("Forcing removal of %s" % hostname)
 
     # 4. Check that we are not leaving the installation without CA and/or DNS
     #    And pick new CA master.
@@ -869,13 +779,13 @@ def check_deleted_segments(hostname, masters, topo_errors, starting_host):
                 return
             i += 1
 
-    if not check_hostname_in_masters(hostname, masters):
+    if not replication.check_hostname_in_masters(hostname, masters):
         print("{0} not in masters, skipping agreement deletion check".format(
             hostname))
         return
 
     suffices = api.Command.topologysuffix_find('', sizelimit=0)['result']
-    suffix_to_masters = map_masters_to_suffices(masters, suffices)
+    suffix_to_masters = replication.map_masters_to_suffices(masters, suffices)
 
     for suffix in suffices:
         suffix_name = suffix['cn'][0]
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 576dfd3f00f0418e043ed35a8ce5ced068a83ce1..7747ea7cf7b3da63390c6cc1c4af328e082b8267 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -29,6 +29,7 @@ import ldap
 
 from ipalib import api, errors
 from ipalib.constants import CACERT
+from ipalib.util import create_topology_graph, get_topology_connection_errors
 from ipapython.ipa_log_manager import *
 from ipapython import ipautil, ipaldap
 from ipapython.dn import DN
@@ -1848,3 +1849,95 @@ class CAReplicationManager(ReplicationManager):
         ret = self.start_replication(r_conn, master=False)
         if ret != 0:
             raise RuntimeError("Failed to start replication")
+
+
+def map_masters_to_suffices(masters, suffices):
+    masters_to_suffix = {}
+    suffix_name_to_root = {
+        s['iparepltopoconfroot'][0]: s['cn'][0] for s in suffices
+    }
+
+    for master in masters:
+        managed_suffices = master['iparepltopomanagedsuffix']
+        for suffix in managed_suffices:
+            suffix_name = suffix_name_to_root[suffix]
+            try:
+                masters_to_suffix[suffix_name].append(master)
+            except KeyError:
+                masters_to_suffix[suffix_name] = [master]
+
+    return masters_to_suffix
+
+
+def check_hostname_in_masters(hostname, masters):
+    master_cns = {m['cn'][0] for m in masters}
+    return hostname in master_cns
+
+
+def check_last_link_managed(api, hostname, masters):
+    """
+    Check if 'hostname' is safe to delete.
+
+    :returns: a dictionary of topology errors across all suffices in the form
+              {<suffix name>: (<original errors>,
+              <errors after removing the node>)}
+    """
+    suffices = api.Command.topologysuffix_find(u'')['result']
+    suffix_to_masters = map_masters_to_suffices(masters, suffices)
+    topo_errors_by_suffix = {}
+
+    for suffix in suffices:
+        suffix_name = suffix['cn'][0]
+        suffix_members = suffix_to_masters[suffix_name]
+        print("Checking connectivity in topology suffix '{0}'".format(
+            suffix_name))
+        if not check_hostname_in_masters(hostname, suffix_members):
+            print(
+                "'{0}' is not a part of topology suffix '{1}'".format(
+                    hostname, suffix_name
+                )
+            )
+            print("Not checking connectivity")
+            continue
+
+        segments = api.Command.topologysegment_find(
+            suffix_name, sizelimit=0).get('result')
+        graph = create_topology_graph(suffix_to_masters[suffix_name], segments)
+
+        # check topology before removal
+        orig_errors = get_topology_connection_errors(graph)
+        if orig_errors:
+            print("Current topology in suffix '{0}' is disconnected:".format(
+                suffix_name))
+            print("Changes are not replicated to all servers and data are "
+                  "probably inconsistent.")
+            print("You need to add segments to reconnect the topology.")
+            print_connect_errors(orig_errors)
+
+        # after removal
+        try:
+            graph.remove_vertex(hostname)
+        except ValueError:
+            pass  # ignore already deleted master, continue to clean
+
+        new_errors = get_topology_connection_errors(graph)
+        if new_errors:
+            print("WARNING: Removal of '{0}' will lead to disconnected "
+                  "topology in suffix '{1}'".format(hostname, suffix_name))
+            print("Changes will not be replicated to all servers and data will"
+                  " become inconsistent.")
+            print("You need to add segments to prevent disconnection of the "
+                  "topology.")
+            print("Errors in topology after removal:")
+            print_connect_errors(new_errors)
+
+        topo_errors_by_suffix[suffix_name] = (orig_errors, new_errors)
+
+    return topo_errors_by_suffix
+
+
+def print_connect_errors(errors):
+    for error in errors:
+        print("Topology does not allow server %s to replicate with servers:" % error[0])
+        for srv in error[2]:
+            print("    %s" % srv)
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to