On 12/01/2015 02:40 PM, Martin Babinsky wrote:
On 11/30/2015 08:34 PM, Martin Basti wrote:



On 30.11.2015 18:41, Martin Babinsky wrote:
On 11/30/2015 06:15 PM, Martin Basti wrote:


On 30.11.2015 16:43, Martin Babinsky wrote:
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.

NACK

ipa-server-install --uninstall

2015-11-30T17:14:30Z DEBUG Destroyed connection
context.ldap2_140081152041808
2015-11-30T17:14:30Z DEBUG Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 91, in _handle_exception
     super(Continuous, self)._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 439, in _handle_exception
     super(ComponentBase, self)._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 355, in __runner
     step()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 352, in <lambda>
     step = lambda: next(self.__gen)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 81, in run_generator_with_yield_from
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 59, in run_generator_with_yield_from
     value = gen.send(prev_value)
   File "/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 71, in _uninstall
     for nothing in self._uninstaller(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 1409, in main
     uninstall_check(self)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 265, in decorated
     func(installer)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 1140, in uninstall_check
     api, masters, options.ignore_disconnected_topology)
AttributeError: 'uninstaller(Server)' object has no attribute
'ignore_disconnected_topology'

2015-11-30T17:14:30Z ERROR 'uninstaller(Server)' object has no
attribute
'ignore_disconnected_topology'
2015-11-30T17:14:30Z INFO The ipa-server-install command was successful


Sorry I have failed horribly during option rename. Attaching patch
that should actually work.

functional ACK
Attaching rebased patches reflecting the recent changes in the handling
of managed topology suffixes handling.





Jan had some more suggestions to the patches. Attaching updated version.

--
Martin^3 Babinsky
From a6dfb5eb910c6bcc4c456a704ba499ba4037e7ca 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 | 108 ++++-----------------------------------
 ipaserver/install/replication.py |  90 ++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 97 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 7bbef3593ef291b4af209d7f59c21e23d3d22944..6d303e6f008325648461287448ef3d2712e23911 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -571,99 +571,6 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
         return None
 
 
-def map_masters_to_suffixes(masters, suffixes):
-    masters_to_suffix = {}
-
-    for master in masters:
-        managed_suffixes = master['iparepltopomanagedsuffix_topologysuffix']
-        for suffix_name in managed_suffixes:
-            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 suffixes in the form
-              {<suffix name>: (<original errors>,
-              <errors after removing the node>)}
-    """
-    suffixes = api.Command.topologysuffix_find(u'')['result']
-    suffix_to_masters = map_masters_to_suffixes(masters, suffixes)
-    topo_errors_by_suffix = {}
-
-    for suffix in suffixes:
-        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:
@@ -773,8 +680,15 @@ def del_master_managed(realm, hostname, options):
     masters = api.Command.server_find('', sizelimit=0)['result']
 
     # 3. Check topology connectivity in all suffixes
-    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.
@@ -862,13 +776,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
 
     suffixes = api.Command.topologysuffix_find('', sizelimit=0)['result']
-    suffix_to_masters = map_masters_to_suffixes(masters, suffixes)
+    suffix_to_masters = replication.map_masters_to_suffixes(masters, suffixes)
 
     for suffix in suffixes:
         suffix_name = suffix['cn'][0]
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 576dfd3f00f0418e043ed35a8ce5ced068a83ce1..13a8b82cc537fdf7c0807b5a0af6e4e65aeec72b 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,92 @@ class CAReplicationManager(ReplicationManager):
         ret = self.start_replication(r_conn, master=False)
         if ret != 0:
             raise RuntimeError("Failed to start replication")
+
+
+def map_masters_to_suffixes(masters, suffixes):
+    masters_to_suffix = {}
+
+    for master in masters:
+        managed_suffixes = master['iparepltopomanagedsuffix_topologysuffix']
+        for suffix_name in managed_suffixes:
+            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 suffixes in the form
+              {<suffix name>: (<original errors>,
+              <errors after removing the node>)}
+    """
+    suffixes = api.Command.topologysuffix_find(sizelimit=0)['result']
+    suffix_to_masters = map_masters_to_suffixes(masters, suffixes)
+    topo_errors_by_suffix = {}
+
+    for suffix in suffixes:
+        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

From 77b9b07f7d7c4dae53e2522750c8ba65a68df5f5 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..8d79755c9f07697eb1533788edbc6027c8d6fbd5 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 IPA server uninstall would lead to disconnected topology. This option can be used only when domain level is 1 or more.
+.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 3c9a527d6d11db345cfed835a89e885860b5608a..c2eb634bdaa2f954d865b74231e94df336d71ed8 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,107 @@ 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):
+    topo_errors = replication.check_last_link_managed(
+                    api, api.env.host, masters)
+    errors_after_uninstall = [topo_errors[i][1] for i in topo_errors]
+
+    if any(errors_after_uninstall):
+        print("Uninstallation leads to disconnected topology")
+        print("Use '--ignore-topology-disconnect' to skip this check")
+        print("Aborting uninstallation")
+        sys.exit(1)
+
+
 @common_cleanup
 def install_check(installer):
     options = installer
@@ -999,35 +1100,62 @@ 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_topology_disconnect:
+                print("'--ignore-topology-disconnect' 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(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)
+
+                if not options.ignore_topology_disconnect:
+                    check_topology_connectivity(api, masters)
+                else:
+                    print("Ignoring topology errors and forcing uninstall")
+
+
     installer._fstore = fstore
     installer._sstore = sstore
 
@@ -1227,6 +1355,12 @@ class Server(BaseServer):
         cli_name='no_hbac_allow',
     )
 
+    ignore_topology_disconnect = Knob(
+        bool, False,
+        description="do not check whether server uninstall disconnects the "
+                    "topology (domain level 1+)",
+    )
+
     # ca
     skip_schema_check = None
 
@@ -1272,6 +1406,11 @@ class Server(BaseServer):
                         "You must specify at least one of --forwarder, "
                         "--auto-forwarders, or --no-forwarders options")
 
+        if self.ignore_topology_disconnect and not self.uninstalling:
+            raise RuntimeError(
+                "'--ignore-topology-disconnect' can be used only during "
+                "uninstallation")
+
         if self.idmax < self.idstart:
             raise RuntimeError(
                 "idmax (%s) cannot be smaller than idstart (%s)" %
-- 
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