On 06/15/2016 10:30 AM, Jan Cholasta wrote:
Hi,

On 12.6.2016 17:31, Martin Babinsky wrote:
On 06/09/2016 08:12 PM, Martin Babinsky wrote:
These patches expand `server_del` to a full fledged IPA master killer in
domain level 1.

Due to 'server uninstallation removed master from topology' use case,
the individual steps are not in the same order as in the original code
to facilitate self-removal from topology without introducing an array of
permissions for master to remove itself.

I had no opportunity to test out the CI test suite because of technical
problems so it would be nice if our upstream QE could give it a spin and
report errors.

http://www.freeipa.org/page/V4/Manage_replication_topology_4_4
https://fedorahosted.org/freeipa/ticket/5181



Attaching rebased patches and bumping for review.

Please note that they depend on 'Server Roles v2' patchset.

Patch 0153:

Should be an ipaserver module, unless it is required on clients as well,
in which case it should be an ipalib module.


Patch 0154: LGTM


Patch 0155:

In LDAPDelete subclasses, the primary key argument is multivalue, so I'm
guessing your post_callback won't work correctly.

Also, since this is *server*-del, s/master/server/ where applicable.


Patch 0156: LGTM


Patch 0157:

This looks suspicious:

+    result = server_del_cmd(hostname, version=api_version, **options)

Version is automatically filled in in Command.__call__(), why do you add
it manually here?


Patch 0158: LGTM


Honza


Attaching updated patches.

--
Martin^3 Babinsky
From 0313e3a6b245d3ffabddb11875f21b8cb889442f Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Wed, 8 Jun 2016 18:16:24 +0200
Subject: [PATCH 1/6] ipaserver module for working with managed topology

This module should aggregate common functionality utilized in the commands
managing domain-level 1 topology.

https://fedorahosted.org/freeipa/ticket/5588
---
 ipalib/util.py                   |  50 ----------
 ipaserver/install/replication.py |   3 +-
 ipaserver/plugins/topology.py    |   3 +-
 ipaserver/topology.py            | 193 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 197 insertions(+), 52 deletions(-)
 create mode 100644 ipaserver/topology.py

diff --git a/ipalib/util.py b/ipalib/util.py
index 4b5f115093b93641942cd1caf7c72ae31b0989dd..62a9c6aa6be3977a93d713538a7d8ba0b2f16426 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -45,7 +45,6 @@ from ipapython.ssh import SSHPublicKey
 from ipapython.dn import DN, RDN
 from ipapython.dnsutil import DNSName
 from ipapython.dnsutil import resolve_ip_addresses
-from ipapython.graph import Graph
 
 if six.PY3:
     unicode = str
@@ -765,55 +764,6 @@ def validate_idna_domain(value):
         raise ValueError(error)
 
 
-def create_topology_graph(masters, segments):
-    """
-    Create an oriented graph from topology defined by masters and segments.
-
-    :param masters
-    :param segments
-    :returns: Graph
-    """
-    graph = Graph()
-
-    for m in masters:
-        graph.add_vertex(m['cn'][0])
-
-    for s in segments:
-        direction = s['iparepltoposegmentdirection'][0]
-        left = s['iparepltoposegmentleftnode'][0]
-        right = s['iparepltoposegmentrightnode'][0]
-        try:
-            if direction == u'both':
-                graph.add_edge(left, right)
-                graph.add_edge(right, left)
-            elif direction == u'left-right':
-                graph.add_edge(left, right)
-            elif direction == u'right-left':
-                graph.add_edge(right, left)
-        except ValueError:  # ignore segments with deleted master
-            pass
-
-    return graph
-
-
-def get_topology_connection_errors(graph):
-    """
-    Traverse graph from each master and find out which masters are not
-    reachable.
-
-    :param graph: topology graph where vertices are masters
-    :returns: list of errors, error is: (master, visited, not_visited)
-    """
-    connect_errors = []
-    master_cns = list(graph.vertices)
-    master_cns.sort()
-    for m in master_cns:
-        visited = graph.bfs(m)
-        not_visited = graph.vertices - visited
-        if not_visited:
-            connect_errors.append((m, list(visited), list(not_visited)))
-    return connect_errors
-
 def detect_dns_zone_realm_type(api, domain):
     """
     Detects the type of the realm that the given DNS zone belongs to.
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index a4fd97def8150382880f1cf7f66fec1735d32e82..f656f1e97db01f394ac7649fd3a84aa5fbc7631b 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -29,7 +29,8 @@ import ldap
 
 from ipalib import api, errors
 from ipalib.constants import CACERT
-from ipalib.util import create_topology_graph, get_topology_connection_errors
+from ipaserver.topology import (
+    create_topology_graph, get_topology_connection_errors)
 from ipapython.ipa_log_manager import root_logger
 from ipapython import ipautil, ipaldap
 from ipapython.dn import DN
diff --git a/ipaserver/plugins/topology.py b/ipaserver/plugins/topology.py
index a6e638479ab5f80d673e8afa5b5f5ff7ab3d3066..c1848f0cc699f84b40be3623e956780d65de8619 100644
--- a/ipaserver/plugins/topology.py
+++ b/ipaserver/plugins/topology.py
@@ -13,7 +13,8 @@ from .baseldap import (
 from ipalib import _, ngettext
 from ipalib import output
 from ipalib.constants import DOMAIN_LEVEL_1
-from ipalib.util import create_topology_graph, get_topology_connection_errors
+from ipaserver.topology import (
+    create_topology_graph, get_topology_connection_errors)
 from ipapython.dn import DN
 
 if six.PY3:
diff --git a/ipaserver/topology.py b/ipaserver/topology.py
new file mode 100644
index 0000000000000000000000000000000000000000..245a2b254353a28e8d567bbbaf68e043c9305f97
--- /dev/null
+++ b/ipaserver/topology.py
@@ -0,0 +1,193 @@
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+
+"""
+set of functions and classes useful for management of domain level 1 topology
+"""
+
+from copy import deepcopy
+
+from ipapython.graph import Graph
+
+CURR_TOPOLOGY_DISCONNECTED = """
+Replication topology in suffix '{suffix}' is disconnected:
+{errors}"""
+
+REMOVAL_DISCONNECTS_TOPOLOGY = """
+Removal of '{hostname}' leads to disconnected topology in suffix '{suffix}':
+{errors}"""
+
+
+def create_topology_graph(masters, segments):
+    """
+    Create an oriented graph from topology defined by masters and segments.
+
+    :param masters
+    :param segments
+    :returns: Graph
+    """
+    graph = Graph()
+
+    for m in masters:
+        graph.add_vertex(m['cn'][0])
+
+    for s in segments:
+        direction = s['iparepltoposegmentdirection'][0]
+        left = s['iparepltoposegmentleftnode'][0]
+        right = s['iparepltoposegmentrightnode'][0]
+        try:
+            if direction == u'both':
+                graph.add_edge(left, right)
+                graph.add_edge(right, left)
+            elif direction == u'left-right':
+                graph.add_edge(left, right)
+            elif direction == u'right-left':
+                graph.add_edge(right, left)
+        except ValueError:  # ignore segments with deleted master
+            pass
+
+    return graph
+
+
+def get_topology_connection_errors(graph):
+    """
+    Traverse graph from each master and find out which masters are not
+    reachable.
+
+    :param graph: topology graph where vertices are masters
+    :returns: list of errors, error is: (master, visited, not_visited)
+    """
+    connect_errors = []
+    master_cns = list(graph.vertices)
+    master_cns.sort()
+    for m in master_cns:
+        visited = graph.bfs(m)
+        not_visited = graph.vertices - visited
+        if not_visited:
+            connect_errors.append((m, list(visited), list(not_visited)))
+    return connect_errors
+
+
+def _map_masters_to_suffixes(masters):
+    masters_to_suffix = {}
+
+    for master in masters:
+        try:
+            managed_suffixes = master.get(
+                'iparepltopomanagedsuffix_topologysuffix')
+        except KeyError:
+            continue
+
+        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 _create_topology_graphs(api_instance):
+    """
+    Construct a topology graph for each topology suffix
+    :param api_instance: instance of IPA API
+    """
+    masters = api_instance.Command.server_find(
+        u'', sizelimit=0, no_members=False)['result']
+
+    suffix_to_masters = _map_masters_to_suffixes(masters)
+
+    topology_graphs = {}
+
+    for suffix_name in suffix_to_masters:
+        segments = api_instance.Command.topologysegment_find(
+            suffix_name, sizelimit=0).get('result')
+
+        topology_graphs[suffix_name] = create_topology_graph(
+            suffix_to_masters[suffix_name], segments)
+
+    return topology_graphs
+
+
+def _format_topology_errors(topo_errors):
+    msg_lines = []
+    for error in topo_errors:
+        msg_lines.append(
+            "Topology does not allow server %s to replicate with servers:"
+            % error[0]
+        )
+        for srv in error[2]:
+            msg_lines.append("    %s" % srv)
+
+    return "\n".join(msg_lines)
+
+
+class TopologyConnectivity(object):
+    """
+    a simple class abstracting the replication connectivity in managed topology
+    """
+
+    def __init__(self, api_instance):
+        self.api = api_instance
+
+        self.graphs = _create_topology_graphs(self.api)
+
+    @property
+    def errors(self):
+        errors_by_suffix = {}
+        for suffix in self.graphs:
+            errors_by_suffix[suffix] = get_topology_connection_errors(
+                self.graphs[suffix]
+            )
+
+        return errors_by_suffix
+
+    def errors_after_master_removal(self, master_cn):
+        graphs_before = deepcopy(self.graphs)
+
+        for s in self.graphs:
+            try:
+                self.graphs[s].remove_vertex(master_cn)
+            except ValueError:
+                pass
+
+        errors_after_removal = self.errors
+
+        self.graphs = graphs_before
+
+        return errors_after_removal
+
+    def check_current_state(self):
+        err_msg = ""
+        for suffix in self.errors:
+            errors = self.errors[suffix]
+            if errors:
+                err_msg = "\n".join([
+                    err_msg,
+                    CURR_TOPOLOGY_DISCONNECTED.format(
+                        suffix=suffix,
+                        errors=_format_topology_errors(errors)
+                    )])
+
+            if err_msg:
+                raise ValueError(err_msg)
+
+    def check_state_after_removal(self, master_cn):
+        err_msg = ""
+        errors_after_removal = self.errors_after_master_removal(master_cn)
+
+        for suffix in errors_after_removal:
+            errors = errors_after_removal[suffix]
+            if errors:
+                err_msg = "\n".join([
+                    err_msg,
+                    REMOVAL_DISCONNECTS_TOPOLOGY.format(
+                        hostname=master_cn,
+                        suffix=suffix,
+                        errors=_format_topology_errors(errors)
+                    )
+                ])
+
+        if err_msg:
+            raise ValueError(err_msg)
-- 
2.5.5

From fde10ff6b26692b1fcb2b4cb461b39174b3d6523 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Wed, 8 Jun 2016 18:22:57 +0200
Subject: [PATCH 2/6] delegate removal of master DNS record and replica keys to
 separate functions

https://fedorahosted.org/freeipa/ticket/5588
---
 install/tools/ipa-replica-manage        | 9 ++-------
 ipaserver/install/bindinstance.py       | 7 +++++++
 ipaserver/install/dnskeysyncinstance.py | 6 ++++++
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 095cca68890cd0f6c4cba9faa2395ffa3b980fc0..186eb106948f65703e01078c1d3b12a6ba567d75 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -896,13 +896,8 @@ def cleanup_server_dns_entries(realm, hostname, suffix, options):
     try:
         if bindinstance.dns_container_exists(options.host, suffix,
                                              dm_password=options.dirman_passwd):
-            bind = bindinstance.BindInstance()
-            bind.remove_master_dns_records(hostname, realm, realm.lower())
-            bind.remove_ipa_ca_dns_records(hostname, realm.lower())
-            bind.remove_server_ns_records(hostname)
-
-            keysyncd = dnskeysyncinstance.DNSKeySyncInstance()
-            keysyncd.remove_replica_public_keys(hostname)
+            bindinstance.remove_master_dns_records(hostname, realm)
+            dnskeysyncinstance.remove_replica_public_keys(hostname)
     except Exception as e:
         print("Failed to cleanup %s DNS entries: %s" % (hostname, e))
         print("You may need to manually remove them from the tree")
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 78e75359266bbefe7954242b98922272fb0c9194..ed1dc7692026ae97b94052589ae67c6bd70ed6b6 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -533,6 +533,13 @@ def check_forwarders(dns_forwarders, logger):
     return forwarders_dnssec_valid
 
 
+def remove_master_dns_records(hostname, realm):
+    bind = BindInstance()
+    bind.remove_master_dns_records(hostname, realm, realm.lower())
+    bind.remove_ipa_ca_dns_records(hostname, realm.lower())
+    bind.remove_server_ns_records(hostname)
+
+
 class DnsBackup(object):
     def __init__(self, service):
         self.service = service
diff --git a/ipaserver/install/dnskeysyncinstance.py b/ipaserver/install/dnskeysyncinstance.py
index 4888d83f845bfe611160209d9e829cdfc56956a7..fadaf216eb16f028334423f341dae05beda1dfa3 100644
--- a/ipaserver/install/dnskeysyncinstance.py
+++ b/ipaserver/install/dnskeysyncinstance.py
@@ -56,6 +56,12 @@ def dnssec_container_exists(fqdn, suffix, dm_password=None, ldapi=False,
 
     return ret
 
+
+def remove_replica_public_keys(hostname):
+    keysyncd = DNSKeySyncInstance()
+    keysyncd.remove_replica_public_keys(hostname)
+
+
 class DNSKeySyncInstance(service.Service):
     def __init__(self, fstore=None, dm_password=None, logger=root_logger,
                  ldapi=False, start_tls=False):
-- 
2.5.5

From 3d3e8fd9a4bb6abfb581d383f763b7082fd78db8 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Wed, 8 Jun 2016 18:25:55 +0200
Subject: [PATCH 3/6] server-del: perform full master removal in managed
 topology

This patch implements most of the del_master_managed() functionality as a part
of `server-del` command.

`server-del` nows performs these actions:
  * check topology connectivity
  * check that at least one CA/DNS server and DNSSec masters are left
    after removal
  * cleanup all LDAP entries/attributes exposing information about the master
  * cleanup master DNS records
  * remove master and service principals
  * remove master entry from LDAP
  * check that all segments pointing to the master were removed

  `server-del` now accepts the following options:
  * `--force`: force master removal even if it doesn't exist
  * `--ignore-topology-disconnect`: ignore errors arising from disconnected
    topology before and after master removal
  * `--ignore-last-of-role`: remove master even if it is last DNS server,
    and DNSSec key master. The last CA will *not* be removed regardless of
    this option.

https://fedorahosted.org/freeipa/ticket/5588
---
 API.txt                     |   5 +-
 VERSION                     |   4 +-
 ipalib/errors.py            |  18 +++
 ipalib/messages.py          |  15 ++
 ipaserver/plugins/server.py | 366 +++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 403 insertions(+), 5 deletions(-)

diff --git a/API.txt b/API.txt
index 93b009b943902d5c3e2f06d70983e7c4fe534760..75d86e2947683f8544943c41e62a9752191391d4 100644
--- a/API.txt
+++ b/API.txt
@@ -4093,9 +4093,12 @@ output: Output('result', type=[<type 'bool'>])
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
 output: PrimaryKey('value')
 command: server_del
-args: 1,2,3
+args: 1,5,3
 arg: Str('cn+', cli_name='name')
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Flag('force?', autofill=True, default=False)
+option: Flag('ignore_last_of_role?', autofill=True, default=False)
+option: Flag('ignore_topology_disconnect?', autofill=True, default=False)
 option: Str('version?')
 output: Output('result', type=[<type 'dict'>])
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
diff --git a/VERSION b/VERSION
index 1f8e8ed14e50f77aae404fa98085ad135d7354b5..729d6a0db9f2890796a8e295f37780fc6546e511 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=184
-# Last change: ftweedal - add issuer options to cert-show and cert-find
+IPA_API_VERSION_MINOR=185
+# Last change: mbabinsk - extend server-del to perform full master removal
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 406a940e58505e59d971756af6ac5e6eda2df7f1..71c12f9d325f9d8159aa829b3b697f87d8f3a0ca 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1379,6 +1379,24 @@ class InvalidDomainLevelError(ExecutionError):
     errno = 4032
     format = _('%(reason)s')
 
+
+class ServerRemovalError(ExecutionError):
+    """
+    **4033** Raised when a removal of IPA server from managed topology fails
+
+    For example:
+
+    >>> raise ServerRemovalError(reason='Removal disconnects topology')
+    Traceback (most recent call last):
+      ...
+    ServerRemovalError: Server removal aborted: Removal disconnects topology
+
+    """
+
+    errno = 4033
+    format = _('Server removal aborted: %(reason)s.')
+
+
 class BuiltinError(ExecutionError):
     """
     **4100** Base class for builtin execution errors (*4100 - 4199*).
diff --git a/ipalib/messages.py b/ipalib/messages.py
index e863bdd495b55921c9e487794f5c9573a6166038..5247ac76109646401e84a0a6c92014b1213c695b 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -364,6 +364,21 @@ class ResultFormattingError(PublicMessage):
     **13019** Unable to correctly format some part of the result
     """
     errno = 13019
+
+
+class ServerRemovalInfo(PublicMessage):
+    """
+    **13020** Informative message printed during removal of IPA server
+    """
+    errno = 13020
+    type = "info"
+
+
+class ServerRemovalWarning(PublicMessage):
+    """
+    **13021** Warning raised during removal of IPA server
+    """
+    errno = 13021
     type = "warning"
 
 
diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py
index a2c2752d94913eda0636cd5a360921eb002282d3..c55a5df64f740e369235bd632c05d7ed6cbfd3cd 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -4,9 +4,11 @@
 
 import dbus
 import dbus.mainloop.glib
+import ldap
+import time
 
 from ipalib import api, crud, errors, messages
-from ipalib import Int, Str, DNSNameParam
+from ipalib import Int, Flag, Str, DNSNameParam
 from ipalib.plugable import Registry
 from .baseldap import (
     LDAPSearch,
@@ -20,7 +22,9 @@ from ipalib import _, ngettext
 from ipalib import output
 from ipapython.dn import DN
 from ipapython.dnsutil import DNSName
+from ipaserver import topology
 from ipaserver.servroles import ENABLED
+from ipaserver.install import bindinstance, dnskeysyncinstance
 
 __doc__ = _("""
 IPA servers
@@ -371,9 +375,367 @@ class server_show(LDAPRetrieve):
 @register()
 class server_del(LDAPDelete):
     __doc__ = _('Delete IPA server.')
-    NO_CLI = True
     msg_summary = _('Deleted IPA server "%(value)s"')
 
+    takes_options = LDAPDelete.takes_options + (
+        Flag(
+            'ignore_topology_disconnect?',
+            label=_('Ignore topology errors'),
+            doc=_('Ignore topology connectivity problems after removal'),
+            default=False,
+        ),
+        Flag(
+            'ignore_last_of_role?',
+            label=_('Ignore check for last remaining CA or DNS server'),
+            doc=_('Skip a check whether the last CA master or DNS server is '
+                  'removed'),
+            default=False,
+        ),
+        Flag(
+            'force?',
+            label=_('Force server removal'),
+            doc=_('Force server removal even if it does not exist'),
+            default=False,
+        ),
+    )
+
+    def _ensure_last_of_role(self, hostname, ignore_last_of_role=False):
+        """
+        1. When deleting server, check if there will be at least one remaining
+           DNS and CA server.
+        2. Pick CA renewal master
+        """
+        def handler(msg, ignore_last_of_role):
+            if ignore_last_of_role:
+                self.add_message(
+                    messages.ServerRemovalWarning(
+                        message=_(msg)
+                    )
+                )
+            else:
+                raise errors.ServerRemovalError(reason=_(msg))
+
+        ipa_config = self.api.Command.config_show()['result']
+        dns_config = self.api.Command.dnsconfig_show()['result']
+
+        ca_servers = ipa_config['ca_server_server']
+        ca_renewal_master = ipa_config['ca_renewal_master_server']
+        dns_servers = dns_config['dns_server_server']
+        dnssec_keymaster = dns_config['dnssec_key_master_server']
+
+        if ca_servers == [hostname]:
+            raise errors.ServerRemovalError(
+                reason=_("Deleting this server is not allowed as it would "
+                         "leave your installation without a CA."))
+
+        if dnssec_keymaster == hostname:
+            handler(
+                "Replica is active DNSSEC key master. Uninstall "
+                "could break your DNS system. Please disable or "
+                "replace DNSSEC key master first.", ignore_last_of_role)
+
+        if dns_servers == [hostname]:
+            handler(
+                "Deleting this server will leave your installation "
+                "without a DNS.", ignore_last_of_role)
+
+        if ca_renewal_master == hostname:
+            other_cas = [ca for ca in ca_servers if ca != hostname]
+
+            self.api.Command.config_mod(ipacarenewalmaster=other_cas[0])
+
+    def _check_topology_connectivity(self, topology_connectivity, master_cn):
+        try:
+            topology_connectivity.check_current_state()
+        except ValueError as e:
+            raise errors.ServerRemovalError(reason=_(str(e)))
+
+        try:
+            topology_connectivity.check_state_after_removal(master_cn)
+        except ValueError as e:
+            raise errors.ServerRemovalError(reason=_(str(e)))
+
+    def _remove_server_principal_references(self, master):
+        """
+        This method removes information about the replica in parts
+        of the shared tree that expose it, so clients stop trying to
+        use this replica.
+        """
+
+        master_principal = "{}@{}".format(master, self.env.realm)
+        conn = self.Backend.ldap2
+        env = self.api.env
+
+        # remove replica memberPrincipal from s4u2proxy configuration
+        s4u2proxy_subtree = DN(env.container_s4u2proxy,
+                               env.basedn)
+        dn1 = DN(('cn', 'ipa-http-delegation'), s4u2proxy_subtree)
+        member_principal1 = "HTTP/{}".format(master_principal)
+
+        dn2 = DN(('cn', 'ipa-ldap-delegation-targets'), s4u2proxy_subtree)
+        member_principal2 = "ldap/{}".format(master_principal)
+
+        dn3 = DN(('cn', 'ipa-cifs-delegation-targets'), s4u2proxy_subtree)
+        member_principal3 = "cifs/{}".format(master_principal)
+
+        for (dn, member_principal) in ((dn1, member_principal1),
+                                       (dn2, member_principal2),
+                                       (dn3, member_principal3)):
+            try:
+                mod = [(ldap.MOD_DELETE, 'memberPrincipal', member_principal)]
+                conn.conn.modify_s(str(dn), mod)
+            except (ldap.NO_SUCH_OBJECT, ldap.NO_SUCH_ATTRIBUTE):
+                self.log.debug(
+                    "Replica (%s) memberPrincipal (%s) not found in %s" %
+                    (master, member_principal, dn))
+            except Exception as e:
+                self.add_message(
+                    messages.ServerRemovalWarning(
+                        message=_("Failed to clean memberPrincipal {principal}"
+                                  " from s4u2proxy entry {dn}: {err}".format(
+                                      principal=member_principal,
+                                      dn=dn,
+                                      err=e))))
+
+        try:
+            etc_basedn = DN(('cn', 'etc'), env.basedn)
+            filter = '(dnaHostname=%s)' % master
+            entries = conn.get_entries(
+                etc_basedn, ldap.SCOPE_SUBTREE, filter=filter)
+            if len(entries) != 0:
+                for entry in entries:
+                    conn.delete_entry(entry)
+        except errors.NotFound:
+            pass
+        except Exception as e:
+            self.add_message(
+                messages.ServerRemovalWarning(
+                    message=_("Failed to clean up DNA hostname entries for "
+                              "{master}: {err}".format(
+                                  master=master, err=e))))
+
+        try:
+            dn = DN(('cn', 'default'), ('ou', 'profile'), env.basedn)
+            ret = conn.get_entry(dn)
+            srvlist = ret.single_value.get('defaultServerList', '')
+            srvlist = srvlist[0].split()
+            if master in srvlist:
+                srvlist.remove(master)
+                attr = ' '.join(srvlist)
+                mod = [(ldap.MOD_REPLACE, 'defaultServerList', attr)]
+                conn.conn.modify_s(str(dn), mod)
+        except (errors.NotFound, ldap.NO_SUCH_ATTRIBUTE,
+                ldap.TYPE_OR_VALUE_EXISTS):
+            pass
+        except Exception as e:
+            self.add_message(
+                messages.ServerRemovalWarning(
+                    message=_("Failed to remove server {master} from server "
+                              "list: {err}".format(master=master, err=e))))
+
+    def _remove_server_host_services(self, ldap, master):
+        """
+        delete server kerberos key and all its svc principals
+        """
+        try:
+            entries = ldap.get_entries(
+                self.api.env.basedn, ldap.SCOPE_SUBTREE,
+                filter='(krbprincipalname=*/{}@{})'.format(
+                    master, self.api.env.realm))
+
+            if entries:
+                entries.sort(key=lambda x: len(x.dn), reverse=True)
+                for entry in entries:
+                    ldap.delete_entry(entry)
+        except errors.NotFound:
+            pass
+        except Exception as e:
+            self.add_message(
+                messages.ServerRemovalWarning(
+                    message=_("Failed to cleanup server principals/keys: "
+                              "{err}".format(err=e))))
+
+    def _cleanup_server_dns_records(self, hostname, **options):
+        if not self.api.Command.dns_is_enabled(
+                **options):
+            return
+
+        try:
+            bindinstance.remove_master_dns_records(
+                hostname, self.api.env.realm)
+            dnskeysyncinstance.remove_replica_public_keys(hostname)
+        except Exception as e:
+            self.add_message(
+                messages.ServerRemovalWarning(
+                    message=_(
+                        "Failed to cleanup {hostname} DNS entries: "
+                        "{err}".format(hostname=hostname, err=e))))
+            self.add_message(
+                messages.ServerRemovalWarning(
+                    message=_("You may need to manually remove them from the "
+                              "tree")))
+
+    def pre_callback(self, ldap, dn, *keys, **options):
+        pkey = self.obj.get_primary_key_from_dn(dn)
+
+        if options.get('force', False):
+            self.add_message(
+                messages.ServerRemovalWarning(
+                    message=_("Forcing removal of {hostname}".format(
+                        hostname=pkey))))
+
+        # check the topology errors before and after removal
+        self.context.topology_connectivity = topology.TopologyConnectivity(
+            self.api)
+
+        if options.get('ignore_topology_disconnect', False):
+            self.add_message(
+                messages.ServerRemovalWarning(
+                    message=_("Ignoring topology connectivity errors.")))
+        else:
+            self._check_topology_connectivity(
+                self.context.topology_connectivity, pkey)
+
+        # ensure that we are not removing last CA/DNS server, DNSSec master and
+        # CA renewal master
+        self._ensure_last_of_role(
+            pkey, ignore_last_of_role=options.get('ignore_last_of_role', False)
+        )
+
+        # remove the references to master's ldap/http principals
+        self._remove_server_principal_references(pkey)
+
+        # try to clean up the leftover DNS entries
+        self._cleanup_server_dns_records(pkey)
+
+        # finally destroy all Kerberos principals
+        self._remove_server_host_services(ldap, pkey)
+
+        return dn
+
+    def exc_callback(self, keys, options, exc, call_func, *call_args,
+                     **call_kwargs):
+        if (options.get('force', False) and isinstance(exc, errors.NotFound)
+                and call_func.__name__ == 'delete_entry'):
+            self.add_message(
+                message=messages.ServerRemovalWarning(
+                    message=_("Server has already been deleted")))
+            return
+
+        raise exc
+
+    def interactive_prompt_callback(self, kw):
+        self.api.Backend.textui.print_plain(
+            _("Removing {server} from replication topology, "
+              "please wait...".format(server=', '.join(kw['cn']))))
+
+    def _check_deleted_segments(self, hostname, topology_connectivity,
+                                starting_host):
+
+        def wait_for_segment_removal(hostname, master_cns, suffix_name,
+                                     orig_errors, new_errors):
+            i = 0
+            while True:
+                left = self.api.Command.topologysegment_find(
+                    suffix_name,
+                    iparepltoposegmentleftnode=hostname,
+                    sizelimit=0
+                )['result']
+                right = self.api.Command.topologysegment_find(
+                    suffix_name,
+                    iparepltoposegmentrightnode=hostname,
+                    sizelimit=0
+                )['result']
+
+                # Relax check if topology was or is disconnected. Disconnected
+                # topology can contain segments with already deleted servers
+                # Check only if segments of servers, which can contact this
+                # server, and the deleted server were removed.
+                # This code should handle a case where there was a topology
+                # with a central node(B):  A <-> B <-> C, where A is current
+                # server. After removal of B, topology will be disconnected and
+                # removal of segment B <-> C won't be replicated back to server
+                # A, therefore presence of the segment has to be ignored.
+                if orig_errors or new_errors:
+                    # use errors after deletion because we don't care if some
+                    # server can't contact the deleted one
+                    cant_contact_me = [e[0] for e in new_errors
+                                       if starting_host in e[2]]
+                    can_contact_me = set(master_cns) - set(cant_contact_me)
+                    left = [
+                        s for s in left if s['iparepltoposegmentrightnode'][0]
+                        in can_contact_me
+                    ]
+                    right = [
+                        s for s in right if s['iparepltoposegmentleftnode'][0]
+                        in can_contact_me
+                    ]
+
+                if not left and not right:
+                    self.add_message(
+                        messages.ServerRemovalInfo(
+                            message=_("Agreements deleted")
+                        ))
+                    return
+                time.sleep(2)
+                if i == 2:  # taking too long, something is wrong, report
+                    self.log.info(
+                        "Waiting for removal of replication agreements")
+                if i > 90:
+                    self.log.info("Taking too long, skipping")
+                    self.log.info("Following segments were not deleted:")
+                    self.add_message(messages.ServerRemovalWarning(
+                        message=_("Following segments were not deleted:")))
+                    for s in left:
+                        self.add_message(messages.ServerRemovalWarning(
+                            message=u"  %s" % s['cn'][0]))
+                    for s in right:
+                        self.add_message(messages.ServerRemovalWarning(
+                            message=u"  %s" % s['cn'][0]))
+                    return
+                i += 1
+
+        topology_graphs = topology_connectivity.graphs
+
+        orig_errors = topology_connectivity.errors
+        new_errors = topology_connectivity.errors_after_master_removal(
+            hostname
+        )
+
+        for suffix_name in topology_graphs:
+            suffix_members = topology_graphs[suffix_name].vertices
+
+            if hostname not in suffix_members:
+                # If the server was already deleted, we can expect that all
+                # removals had been done in previous run and dangling segments
+                # were not deleted.
+                self.log.info(
+                    "Skipping replication agreement deletion check for "
+                    "suffix '{0}'".format(suffix_name))
+                continue
+
+            self.log.info(
+                "Checking for deleted segments in suffix '{0}'".format(
+                    suffix_name))
+
+            wait_for_segment_removal(
+                hostname,
+                list(suffix_members),
+                suffix_name,
+                orig_errors[suffix_name],
+                new_errors[suffix_name])
+
+    def post_callback(self, ldap, dn, *keys, **options):
+        # there is no point in checking deleted segment on local host
+        # we should do this only when removing other masters
+        if self.api.env.host != keys[-1]:
+            self._check_deleted_segments(
+                keys[-1], self.context.topology_connectivity,
+                self.api.env.host)
+
+        return super(server_del, self).post_callback(
+            ldap, dn, *keys, **options)
+
 
 @register()
 class server_conncheck(crud.PKQuery):
-- 
2.5.5

From 97ec07a98e8c615006caad548ce523e2eda970bd Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Wed, 8 Jun 2016 18:31:48 +0200
Subject: [PATCH 4/6] CI test suite for `server-del`

these tests cover various scenarios such as:
* trying to remove master that would disconnect topology in one of the
  suffixes
* forcing master removal regardless of topology state before/after removal
* trying to remove last CA/DNS server/DNSSec key master
* forcing removal of the last DNSSec key master

https://fedorahosted.org/freeipa/ticket/5588
---
 ipatests/test_integration/tasks.py           |  36 +++-
 ipatests/test_integration/test_caless.py     |  11 +-
 ipatests/test_integration/test_server_del.py | 300 +++++++++++++++++++++++++++
 3 files changed, 332 insertions(+), 15 deletions(-)
 create mode 100644 ipatests/test_integration/test_server_del.py

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index aebd907ebfdd0c513c8c96eece220f009a0e8953..4b9f03137b252ac5ba24011ba28c174f752c5575 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -57,10 +57,10 @@ def check_arguments_are(slice, instanceof):
     and third arguments are integers
     """
     def wrapper(func):
-        def wrapped(*args):
+        def wrapped(*args, **kwargs):
             for i in args[slice[0]:slice[1]]:
                 assert isinstance(i, instanceof), "Wrong type: %s: %s" % (i, type(i))
-            return func(*args)
+            return func(*args, **kwargs)
         return wrapped
     return wrapper
 
@@ -721,7 +721,7 @@ def clean_replication_agreement(master, replica):
 
 
 @check_arguments_are((0, 3), Host)
-def create_segment(master, leftnode, rightnode):
+def create_segment(master, leftnode, rightnode, suffix=DOMAIN_SUFFIX_NAME):
     """
     creates a topology segment. The first argument is a node to run the command
     :returns: a hash object containing segment's name, leftnode, rightnode
@@ -731,7 +731,7 @@ def create_segment(master, leftnode, rightnode):
     lefthost = leftnode.hostname
     righthost = rightnode.hostname
     segment_name = "%s-to-%s" % (lefthost, righthost)
-    result = master.run_command(["ipa", "topologysegment-add", DOMAIN_SUFFIX_NAME,
+    result = master.run_command(["ipa", "topologysegment-add", suffix,
                                  segment_name,
                                  "--leftnode=%s" % lefthost,
                                  "--rightnode=%s" % righthost], raiseonerr=False)
@@ -743,7 +743,7 @@ def create_segment(master, leftnode, rightnode):
         return {}, result.stderr_text
 
 
-def destroy_segment(master, segment_name):
+def destroy_segment(master, segment_name, suffix=DOMAIN_SUFFIX_NAME):
     """
     Destroys topology segment.
     :param master: reference to master object of class Host
@@ -753,7 +753,7 @@ def destroy_segment(master, segment_name):
     kinit_admin(master)
     command = ["ipa",
                "topologysegment-del",
-               DOMAIN_SUFFIX_NAME,
+               suffix,
                segment_name]
     result = master.run_command(command, raiseonerr=False)
     return result.returncode, result.stderr_text
@@ -1181,3 +1181,27 @@ def replicas_cleanup(func):
                                             "host-del",
                                             host.hostname], raiseonerr=False)
     return wrapped
+
+
+def run_server_del(host, server_to_delete, force=False,
+                   ignore_topology_disconnect=False,
+                   ignore_last_of_role=False):
+    kinit_admin(host)
+    args = ['ipa', 'server-del', server_to_delete]
+    if force:
+        args.append('--force')
+    if ignore_topology_disconnect:
+        args.append('--ignore-topology-disconnect')
+    if ignore_last_of_role:
+        args.append('--ignore-last-of-role')
+
+    return host.run_command(args, raiseonerr=False)
+
+
+def assert_error(result, stderr_text, returncode=None):
+    "Assert that `result` command failed and its stderr contains `stderr_text`"
+    assert stderr_text in result.stderr_text, result.stderr_text
+    if returncode:
+        assert result.returncode == returncode
+    else:
+        assert result.returncode > 0
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index fdc4fc8efe73631e9ab03f3b9019444f7d7e09ec..667e2b3b1d91f967b32fabdb7e472886bbdf79d7 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -35,6 +35,8 @@ from ipatests.test_integration import tasks
 
 _DEFAULT = object()
 
+assert_error = tasks.assert_error
+
 
 def get_install_stdin(cert_passwords=()):
     lines = [
@@ -56,15 +58,6 @@ def get_replica_prepare_stdin(cert_passwords=()):
     return '\n'.join(lines + [''])
 
 
-def assert_error(result, stderr_text, returncode=None):
-    "Assert that `result` command failed and its stderr contains `stderr_text`"
-    assert stderr_text in result.stderr_text, result.stderr_text
-    if returncode:
-        assert result.returncode == returncode
-    else:
-        assert result.returncode > 0
-
-
 class CALessBase(IntegrationTest):
     @classmethod
     def install(cls, mh):
diff --git a/ipatests/test_integration/test_server_del.py b/ipatests/test_integration/test_server_del.py
new file mode 100644
index 0000000000000000000000000000000000000000..628afb8e547a80e4c74094f839f1dc1b6940c117
--- /dev/null
+++ b/ipatests/test_integration/test_server_del.py
@@ -0,0 +1,300 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+from itertools import permutations
+
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+from ipalib.constants import DOMAIN_LEVEL_1, DOMAIN_SUFFIX_NAME, CA_SUFFIX_NAME
+
+REMOVAL_ERR_TEMPLATE = ("Removal of '{hostname}' leads to disconnected "
+                        "topology in suffix '{suffix}'")
+
+
+def check_master_removal(host, hostname_to_remove,
+                         force=False,
+                         ignore_topology_disconnect=False,
+                         ignore_last_of_role=False):
+    result = tasks.run_server_del(
+        host,
+        hostname_to_remove,
+        force=force,
+        ignore_topology_disconnect=ignore_topology_disconnect,
+        ignore_last_of_role=ignore_last_of_role)
+
+    assert result.returncode == 0
+    if force:
+        assert ("Forcing removal of {hostname}".format(
+            hostname=hostname_to_remove) in result.stderr_text)
+
+    if ignore_topology_disconnect:
+        assert "Ignoring topology connectivity errors." in result.stderr_text
+
+    if ignore_last_of_role:
+        assert ("Ignoring these warnings and proceeding with removal" in
+                result.stderr_text)
+
+    tasks.assert_error(
+        host.run_command(
+            ['ipa', 'server-show', hostname_to_remove], raiseonerr=False
+        ),
+        "{}: server not found".format(hostname_to_remove),
+        returncode=2
+    )
+
+
+def check_removal_disconnects_topology(
+        host, hostname_to_remove,
+        affected_suffixes=(DOMAIN_SUFFIX_NAME,)):
+    result = tasks.run_server_del(host, hostname_to_remove)
+    assert len(affected_suffixes) <= 2
+
+    err_messages_by_suffix = {
+        CA_SUFFIX_NAME: REMOVAL_ERR_TEMPLATE.format(
+            hostname=hostname_to_remove,
+            suffix=CA_SUFFIX_NAME
+        ),
+        DOMAIN_SUFFIX_NAME: REMOVAL_ERR_TEMPLATE.format(
+            hostname=hostname_to_remove,
+            suffix=DOMAIN_SUFFIX_NAME
+        )
+    }
+
+    for suffix in err_messages_by_suffix:
+        if suffix in affected_suffixes:
+            tasks.assert_error(
+                result, err_messages_by_suffix[suffix], returncode=1)
+        else:
+            assert err_messages_by_suffix[suffix] not in result.stderr_text
+
+
+class ServerDelBase(IntegrationTest):
+    num_replicas = 2
+    num_clients = 1
+    domain_level = DOMAIN_LEVEL_1
+    topology = 'star'
+
+    @classmethod
+    def install(cls, mh):
+        super(ServerDelBase, cls).install(mh)
+
+        cls.client = cls.clients[0]
+        cls.replica1 = cls.replicas[0]
+        cls.replica2 = cls.replicas[1]
+
+
+class TestServerDel(ServerDelBase):
+
+    @classmethod
+    def install(cls, mh):
+        super(TestServerDel, cls).install(mh)
+        # prepare topologysegments for negative test cases
+        # it should look like this for DOMAIN_SUFFIX_NAME:
+        #             master
+        #            /
+        #           /
+        #          /
+        #   replica1------- replica2
+        # and like this for CA_SUFFIX_NAME
+        #             master
+        #                  \
+        #                   \
+        #                    \
+        #   replica1------- replica2
+
+        tasks.create_segment(cls.client, cls.replica1, cls.replica2)
+        tasks.create_segment(cls.client, cls.replica1, cls.replica2,
+                             suffix=CA_SUFFIX_NAME)
+
+        # try to delete all relevant segment connecting master and replica1/2
+        segment_name_fmt = '{p[0].hostname}-to-{p[1].hostname}'
+        for domain_pair in permutations((cls.master, cls.replica2)):
+            tasks.destroy_segment(
+                cls.client, segment_name_fmt.format(p=domain_pair))
+
+        for ca_pair in permutations((cls.master, cls.replica1)):
+            tasks.destroy_segment(
+                cls.client, segment_name_fmt.format(p=ca_pair),
+                suffix=CA_SUFFIX_NAME)
+
+    def test_removal_of_nonexistent_master_raises_error(self):
+        """
+        tests that removal of non-existent master raises an error
+        """
+        hostname = u'bogus-master.bogus.domain'
+        err_message = "{}: server not found".format(hostname)
+        tasks.assert_error(
+            tasks.run_server_del(self.client, hostname),
+            err_message,
+            returncode=2
+        )
+
+    def test_forced_removal_of_nonexistent_master(self):
+        """
+        tests that removal of non-existent master with '--force' does not raise
+        an error
+        """
+        hostname = u'bogus-master.bogus.domain'
+        result = tasks.run_server_del(self.client, hostname, force=True)
+        assert result.returncode == 0
+        assert ('Deleted IPA server "{}"'.format(hostname) in
+                result.stderr_text)
+
+    def test_removal_of_replica1_disconnects_domain_topology(self):
+        """
+        tests that given the used topology, attempted removal of replica1 fails
+        with disconnected DOMAIN topology but not CA
+        """
+
+        check_removal_disconnects_topology(
+            self.client,
+            self.replica1.hostname,
+            affected_suffixes=(DOMAIN_SUFFIX_NAME,)
+        )
+
+    def test_removal_of_replica2_disconnects_ca_topology(self):
+        """
+        tests that given the used topology, attempted removal of replica2 fails
+        with disconnected CA topology but not DOMAIN
+        """
+
+        check_removal_disconnects_topology(
+            self.client,
+            self.replica2.hostname,
+            affected_suffixes=(CA_SUFFIX_NAME,)
+        )
+
+    def test_ignore_topology_disconnect_replica1(self):
+        """
+        tests that removal of replica1 with '--ignore-topology-disconnect'
+        destroys master for good
+        """
+        check_master_removal(
+            self.client,
+            self.replica1.hostname,
+            ignore_topology_disconnect=True
+        )
+
+        # reinstall the replica
+        tasks.uninstall_master(self.replica1)
+        tasks.install_replica(self.master, self.replica1, setup_ca=True)
+
+    def test_ignore_topology_disconnect_replica2(self):
+        """
+        tests that removal of replica2 with '--ignore-topology-disconnect'
+        destroys master for good
+        """
+        check_master_removal(
+            self.client,
+            self.replica2.hostname,
+            ignore_topology_disconnect=True
+        )
+
+        # reinstall the replica
+        tasks.uninstall_master(self.replica2)
+        tasks.install_replica(self.master, self.replica2, setup_ca=True)
+
+    def test_removal_of_master_disconnects_both_topologies(self):
+        """
+        tests that master removal will now raise errors in both suffixes.
+        """
+        check_removal_disconnects_topology(
+            self.client,
+            self.master.hostname,
+            affected_suffixes=(CA_SUFFIX_NAME, DOMAIN_SUFFIX_NAME)
+        )
+
+    def test_removal_of_replica1(self):
+        """
+        tests the removal of replica1 which should now pass without errors
+        """
+        check_master_removal(
+            self.client,
+            self.replica1.hostname
+        )
+
+    def test_removal_of_replica2(self):
+        """
+        tests the removal of replica2 which should now pass without errors
+        """
+        check_master_removal(
+            self.client,
+            self.replica2.hostname
+        )
+
+
+class TestLastServices(ServerDelBase):
+    """
+    Test the checks for last services during server-del and their bypassing
+    using when forcing the removal
+    """
+    num_replicas = 1
+    domain_level = DOMAIN_LEVEL_1
+    topology = 'line'
+
+    @classmethod
+    def install(cls, mh):
+        tasks.install_topo(
+            cls.topology, cls.master, cls.replicas, [],
+            domain_level=cls.domain_level, setup_replica_cas=False)
+
+    def test_removal_of_master_raises_error_about_last_ca(self):
+        """
+        test that removal of master fails on the last
+        """
+        tasks.assert_error(
+            tasks.run_server_del(self.replicas[0], self.master.hostname),
+            "Deleting this server is not allowed as it would leave your "
+            "installation without a CA.",
+            1
+        )
+
+    def test_install_ca_on_replica1(self):
+        """
+        Install CA on replica so that we can test DNS-related checks
+        """
+        tasks.install_ca(self.replicas[0], domain_level=self.domain_level)
+
+    def test_removal_of_master_raises_error_about_last_dns(self):
+        """
+        Now server-del should complain about the removal of last DNS server
+        """
+        tasks.assert_error(
+            tasks.run_server_del(self.replicas[0], self.master.hostname),
+            "Deleting this server will leave your installation "
+            "without a DNS.",
+            1
+        )
+
+    def test_install_dns_on_replica1_and_dnssec_on_master(self):
+        """
+        install DNS server on replica and DNSSec on master
+        """
+        tasks.install_dns(self.replicas[0])
+        args = [
+            "ipa-dns-install",
+            "--dnssec-master",
+            "--forwarder", self.master.config.dns_forwarder,
+            "-U",
+        ]
+        self.master.run_command(args)
+
+    def test_removal_of_master_raises_error_about_dnssec(self):
+        tasks.assert_error(
+            tasks.run_server_del(self.replicas[0], self.master.hostname),
+            "Replica is active DNSSEC key master. Uninstall "
+            "could break your DNS system. Please disable or replace "
+            "DNSSEC key master first.",
+            1
+        )
+
+    def test_forced_removal_of_master(self):
+        """
+        Tests that we can still force remove the master using
+        '--ignore-last-of-role'
+        """
+        check_master_removal(
+            self.replicas[0], self.master.hostname,
+            ignore_last_of_role=True
+        )
-- 
2.5.5

From 445fc19f172dce931efb7863cdfd779919550e57 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Wed, 8 Jun 2016 18:34:37 +0200
Subject: [PATCH 5/6] ipa-replica-manage: use `server_del` when removing domain
 level 1 replica

`ipa-replica-manage del` will now call `server_del` behind the scenes when a
removal of replica from managed topology is requested. The existing removal
options were mapped on the server_del options to maintain backwards
compatibility with earlier versions.

https://fedorahosted.org/freeipa/ticket/5588
---
 install/tools/ipa-replica-manage | 139 +++------------------------------------
 ipaserver/install/replication.py | 122 ++++------------------------------
 2 files changed, 21 insertions(+), 240 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 186eb106948f65703e01078c1d3b12a6ba567d75..7641727c5bf45aa451c2995efeba28a611961ee4 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -26,7 +26,6 @@ import os
 import re
 import ldap
 import socket
-import time
 import traceback
 
 from six.moves.urllib.parse import urlparse
@@ -920,139 +919,17 @@ def del_master_managed(realm, hostname, options):
         print("Can't remove itself: %s" % (options.host))
         sys.exit(1)
 
-    try:
-        api.Command.server_show(hostname_u)
-    except errors.NotFound:
-        if not options.cleanup:
-            print("{hostname} is not listed among IPA masters.".format(
-                hostname=hostname))
-            print("Please specify an actual server or add the --cleanup "
-                  "option to force clean up.")
-            sys.exit(1)
-
-    # 1. Connect to the local server
-    try:
-        thisrepl = replication.ReplicationManager(realm, options.host,
-                                                  options.dirman_passwd)
-    except Exception as e:
-        print("Failed to connect to server %s: %s" % (options.host, e))
-        sys.exit(1)
-
-    # 2. Get all masters
-    masters = api.Command.server_find(
-        '', sizelimit=0, no_members=False)['result']
-
-    # 3. Check topology connectivity in all suffixes
-    topo_errors = replication.check_last_link_managed(api, hostname, masters)
+    server_del_options = dict(
+        force=options.cleanup,
+        ignore_topology_disconnect=options.force,
+        ignore_last_of_role=options.force
+    )
 
-    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.
-    ensure_last_services(api.Backend.ldap2, hostname, masters, options)
-
-    # 5. Remove master entry. Topology plugin will remove replication agreements.
-    try:
-        api.Command.server_del(hostname_u)
-    except errors.NotFound:
-        print("Server entry already deleted: %s" % (hostname))
-
-    # 6. Cleanup
     try:
-        thisrepl.replica_cleanup(hostname, realm, force=True)
+        replication.run_server_del_as_cli(
+            api, hostname_u, **server_del_options)
     except Exception as e:
-        print("Failed to cleanup %s entries: %s" % (hostname, e))
-        print("You may need to manually remove them from the tree")
-
-    # 7. Clean RUV for the deleted master
-    # Wait for topology plugin to delete segments
-    check_deleted_segments(hostname_u, masters, topo_errors, options.host)
-
-    # Clean RUV is handled by the topolgy plugin
-
-    # 8. And clean up the removed replica DNS entries if any.
-    cleanup_server_dns_entries(realm, hostname, thisrepl.suffix, options)
-
-
-def check_deleted_segments(hostname, masters, topo_errors, starting_host):
-
-    def wait_for_segment_removal(hostname, master_cns, suffix_name,
-                                 topo_errors):
-        i = 0
-        while True:
-            left = api.Command.topologysegment_find(
-                suffix_name, iparepltoposegmentleftnode=hostname, sizelimit=0
-            )['result']
-            right = api.Command.topologysegment_find(
-                suffix_name, iparepltoposegmentrightnode=hostname, sizelimit=0
-            )['result']
-
-            # Relax check if topology was or is disconnected. Disconnected
-            # topology can contain segments with already deleted servers.
-            # Check only if segments of servers, which can contact this server,
-            # and the deleted server were removed.
-            # This code should handle a case where there was a topology with
-            # a central node(B):  A <-> B <-> C, where A is current server.
-            # After removal of B, topology will be disconnected and removal of
-            # segment B <-> C won't be replicated back to server A, therefore
-            # presence of the segment has to be ignored.
-            if topo_errors[0] or topo_errors[1]:
-                # use errors after deletion because we don't care if some
-                # server can't contact the deleted one
-                cant_contact_me = [e[0] for e in topo_errors[1]
-                                   if starting_host in e[2]]
-                can_contact_me = set(master_cns) - set(cant_contact_me)
-                left = [s for s in left if s['iparepltoposegmentrightnode'][0]
-                        in can_contact_me]
-                right = [s for s in right if s['iparepltoposegmentleftnode'][0]
-                         in can_contact_me]
-
-            if not left and not right:
-                print("Agreements deleted")
-                return
-            time.sleep(2)
-            if i == 2: # taking too long, something is wrong, report
-                print("Waiting for removal of replication agreements")
-            if i > 90:
-                print("Taking too long, skipping")
-                print("Following segments were not deleted:")
-                for s in left:
-                    print("  %s" % s['cn'][0])
-                for s in right:
-                    print("  %s" % s['cn'][0])
-                return
-            i += 1
-
-    if not replication.check_hostname_in_masters(hostname, masters):
-        print("{0} not in masters, skipping agreement deletion check".format(
-            hostname))
-        return
-
-    suffix_to_masters = replication.map_masters_to_suffixes(masters)
-
-    for suffix_name in suffix_to_masters:
-        suffix_member_cns = [
-            m['cn'][0] for m in suffix_to_masters[suffix_name]
-        ]
-
-        if hostname not in suffix_member_cns:
-            # If the server was already deleted, we can expect that all
-            # removals had been done in previous run and dangling segments
-            # were not deleted.
-            print("Skipping replication agreement deletion check for "
-                  "suffix '{0}'".format(suffix_name))
-            continue
-
-        print("Checking for deleted segments in suffix '{0}'".format(
-            suffix_name))
-        wait_for_segment_removal(hostname, suffix_member_cns, suffix_name,
-                                 topo_errors[suffix_name])
+        sys.exit(e)
 
 
 def del_master_direct(realm, hostname, options):
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index f656f1e97db01f394ac7649fd3a84aa5fbc7631b..9f87f4b36743325d6b913b74c06f88c2ed21d031 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -28,9 +28,8 @@ from random import randint
 import ldap
 
 from ipalib import api, errors
+from ipalib.cli import textui
 from ipalib.constants import CACERT
-from ipaserver.topology import (
-    create_topology_graph, get_topology_connection_errors)
 from ipapython.ipa_log_manager import root_logger
 from ipapython import ipautil, ipaldap
 from ipapython.dn import DN
@@ -1753,116 +1752,21 @@ class CAReplicationManager(ReplicationManager):
             raise RuntimeError("Failed to start replication")
 
 
-def map_masters_to_suffixes(masters):
-    masters_to_suffix = {}
-
-    for master in masters:
-        try:
-            managed_suffixes = master['iparepltopomanagedsuffix_topologysuffix']
-        except KeyError:
-            print("IPA master {0} does not manage any suffix")
-            continue
-
-        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 get_orphaned_suffixes(masters):
-    """
-    :param masters: result of server_find command
-    :return a set consisting of suffix names which are not managed by any
-    master
+def run_server_del_as_cli(api_instance, hostname, **options):
     """
-    all_suffixes = api.Command.topologysuffix_find(
-        sizelimit=0)['result']
-    all_suffix_names = set(s['cn'][0] for s in all_suffixes)
-    managed_suffixes = set(map_masters_to_suffixes(masters))
-
-    return all_suffix_names ^ managed_suffixes
-
+    run server_del API command and print the result to stdout/stderr using
+    textui backend.
 
-def check_last_link_managed(api, hostname, masters):
+    :params api_instance: API instance
+    :params hostname: server FQDN
+    :params options: options for server_del command
     """
-    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>)}
-    """
-    suffix_to_masters = map_masters_to_suffixes(masters)
-    topo_errors_by_suffix = {}
-
-    # sanity check for orphaned suffixes
-    orphaned_suffixes = get_orphaned_suffixes(masters)
-    if orphaned_suffixes:
-        print("The following suffixes are not managed by any IPA master:")
-        print("  {0}".format(
-                ', '.join(sorted(orphaned_suffixes))
-            )
-        )
-
-    for suffix_name in suffix_to_masters:
-        print("Checking connectivity in topology suffix '{0}'".format(
-            suffix_name))
-        if not check_hostname_in_masters(hostname,
-                                         suffix_to_masters[suffix_name]):
-            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)
+    server_del_cmd = api_instance.Command.server_del
 
-    return topo_errors_by_suffix
+    if 'version' not in options:
+        options['version'] = api_instance.env.api_version
 
+    result = server_del_cmd(hostname, **options)
 
-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)
+    textui_backend = textui(api_instance)
+    server_del_cmd.output_for_cli(textui_backend, result, hostname, **options)
-- 
2.5.5

From bbb1133768deff6c7ad192269735a56fca304693 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Wed, 8 Jun 2016 18:36:45 +0200
Subject: [PATCH 6/6] remove the master from managed topology during
 uninstallation

In managed topology, calling `ipa-server-install --uninstall` will cause the
master to remove itself from the topology by calling `server_del` behind the
scenes.

https://fedorahosted.org/freeipa/ticket/5588
---
 ipaserver/install/server/install.py | 184 ++++++------------------------------
 1 file changed, 28 insertions(+), 156 deletions(-)

diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 46b7190dc899dc85daa109af4850206cf5343d5c..930cca7b31ca06c04ab92deff49b6a4f198c2b6e 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -4,7 +4,6 @@
 
 from __future__ import print_function
 
-import gssapi
 import os
 import pickle
 import pwd
@@ -27,15 +26,15 @@ from ipapython.ipautil import (
 from ipaplatform import services
 from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
-from ipalib import api, create_api, constants, errors, x509
-from ipalib.krb_utils import KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN
+from ipalib import api, constants, errors, x509
 from ipalib.constants import CACERT
 from ipalib.util import validate_domain_name
 import ipaclient.ntpconf
 from ipaserver.install import (
-    bindinstance, ca, cainstance, certs, dns, dsinstance, httpinstance,
-    installutils, kra, krbinstance, memcacheinstance, ntpinstance,
-    otpdinstance, custodiainstance, replication, service, sysupgrade)
+    bindinstance, ca, cainstance, certs, dns, dsinstance,
+    httpinstance, installutils, kra, krbinstance, memcacheinstance,
+    ntpinstance, otpdinstance, custodiainstance, replication, service,
+    sysupgrade)
 from ipaserver.install.installutils import (
     IPA_MODULES, BadHostError, get_fqdn, get_server_ip_address,
     is_ipa_configured, load_pkcs12, read_password, verify_fqdn,
@@ -290,136 +289,21 @@ def common_cleanup(func):
     return decorated
 
 
-def check_master_deleted(api, masters, interactive):
-    """
-    Determine whether the IPA master was removed from the domain level 1
-    topology. The function first tries to locally lookup the master host entry
-    and fetches host prinicipal from DS. Then we attempt to acquire host TGT,
-    contact the other masters one at a time and query for the existence of the
-    host entry for our IPA master.
-
-    :param api: instance of API object
-    :param masters: list of masters to contact
-    :param interactive: whether run in interactive mode. The user will be
-        prompted for action if the removal status cannot be determined
-    :return: True if the master is not part of the topology anymore as
-        determined by the following conditions:
-            * the host entry does not exist in local DS
-            * request for host TGT fails due to missing/invalid/revoked creds
-            * GSSAPI connection to remote DS fails on invalid authentication
-            * if we are the only master
-        False otherwise
-    """
+def remove_master_from_managed_topology(api_instance, options):
     try:
-        host_princ = api.Command.host_show(
-            api.env.host)['result']['krbprincipalname'][0]
-    except errors.NotFound:
-        root_logger.debug(
-            "Host entry for {} already deleted".format(api.env.host)
+        # we may force the removal
+        # if the master was already deleted we will just get a warning
+        server_del_options = dict(
+            force=True,
+            ignore_topology_disconnect=options.ignore_topology_disconnect,
+            ignore_last_of_role=options.ignore_last_of_role
         )
-        return True
+
+        replication.run_server_del_as_cli(
+            api_instance, api_instance.env.host, **server_del_options)
+
     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):
-        # attempt to get host TGT. This can fail if the master contacts remote
-        # KDCs on other masters that have already cleared our master's
-        # principal. In that case return True
-        try:
-            ipautil.kinit_keytab(host_princ, paths.KRB5_KEYTAB, ccache_path)
-        except gssapi.exceptions.GSSError as e:
-            min_code = e.min_code  # pylint: disable=no-member
-            if min_code == KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN:
-                root_logger.debug("Host principal not found, assuming that "
-                                  "master is removed from topology")
-                return True
-
-            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)
+        root_logger.warning("Failed to delete master: {}".format(e))
 
 
 @common_cleanup
@@ -1151,26 +1035,7 @@ def uninstall_check(installer):
                     print("Aborting uninstall operation.")
                     sys.exit(1)
         else:
-            masters = api.Command.server_find(
-                sizelimit=0, no_members=False)['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")
-
+            remove_master_from_managed_topology(api, options)
 
     installer._fstore = fstore
     installer._sstore = sstore
@@ -1421,6 +1286,11 @@ class Server(BaseServer):
         description="do not check whether server uninstall disconnects the "
                     "topology (domain level 1+)",
     )
+    ignore_last_of_role = Knob(
+        bool, False,
+        description="do not check whether server uninstall removes last "
+                    "CA/DNS server or DNSSec master (domain level 1+)",
+    )
 
     # dns
     dnssec_master = None
@@ -1465,10 +1335,12 @@ 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:
+        any_ignore_option_true = any(
+            [self.ignore_topology_disconnect, self.ignore_last_of_role])
+        if any_ignore_option_true and not self.uninstalling:
             raise RuntimeError(
-                "'--ignore-topology-disconnect' can be used only during "
-                "uninstallation")
+                "'--ignore-topology-disconnect/--ignore-last-of-role' options "
+                "can be used only during uninstallation")
 
         if self.idmax < self.idstart:
             raise RuntimeError(
-- 
2.5.5

-- 
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