Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

On 11/12/2015 04:24 PM, Martin Babinsky wrote:

On 11/12/2015 02:04 PM, Petr Vobornik wrote:

On 11/10/2015 05:43 PM, Martin Babinsky wrote:

On 11/04/2015 06:50 PM, Petr Vobornik wrote:

On 11/04/2015 01:30 PM, Martin Babinsky wrote:

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology
suffixes,
but
there is a code (install/tools/ipa-replica-manage starting at line
788
after applying my patch) which monitors whether the segments
pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.




Hmm, I'm thinking whether the 'check_last_link_managed' and
'check_deleted_segments' should not be called per-suffix, but should
themselves check all suffixes available. This could make the fix for
https://fedorahosted.org/freeipa/ticket/5409 also easier.



Depends if the output is reusable. If so then why not.
check_last_link_managed basically adds text to several
get_topology_connection_errors calls.


Attaching updated patch.



I'm not sure about (pseudo code):

 topo_errors = ([], [])
 for each suffix:
 topo_errors[0].extend(orig_errors)
 topo_errors[1].extend(new_errors)
 return topo_errors

In check_deleted_segments wait_for_segment_removal is per-suffix check
but uses topo_errors which contains errors from both suffices. Topo
erros are used to relax the check if topology is disconnected but this
might relax it too much.

I would change the errors to per-suffix as well, e.g.:
   topo_errors = {}
   for each suffix:
   topo_errors[suffix_name] = (orig_errors, new_errors)
   return topo_errors

Otherwise it looks OK (not tested yet).


I didn't realize that. I have modified the patch accordingly.



Attaching updated patch with changed docstring of 
'check_last_link_managed()'


--
Martin^3 Babinsky
From 3d8c659effc0eb7f5c000ae98195be509072b99e Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 30 Oct 2015 13:59:03 +0100
Subject: [PATCH 1/3] check for disconnected topology and deleted agreements
 for all suffices

The code in ipa-replica-manage which checks for disconnected topology and
deleted agreements during node removal was generalized so that it now performs
these checks for all suffixes to which the node belongs.

https://fedorahosted.org/freeipa/ticket/5309
---
 install/tools/ipa-replica-manage | 251 ++-
 1 file changed, 168 insertions(+), 83 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 2de6fd7993be290fefa5c2c7d07733c39d457ed6..ebbdf5c3301675d39d957f54d954a440ad5cbbc2 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -570,46 +570,97 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 else:
 return None
 
-def check_last_link_managed(api, masters, hostname, force):
+
+def map_masters_to_suffices(masters, suffices):
+masters_to_suffix = {}
+suffix_name_to_root = {
+s['iparepltopoconfroot'][0]: s['cn'][0] for s in suffices
+}
+
+for master in masters:
+managed_suffices = master['iparepltopomanagedsuffix']
+for suffix in managed_suffices:
+suffix_name = suffix_name_to_root[suffix]
+try:
+masters_to_suffix[suffix_name].append(master)
+except KeyError:
+masters_to_suffix[suffix_name] = [master]
+
+return masters_to_suffix
+
+
+def check_hostname_in_masters(hostname, masters):
+master_cns = {m['cn'][0] for m in masters}
+return hostname in master_cns
+
+
+def check_last_link_managed(api, hostname, masters, force):
 """
 Check if 'hostname' is safe to delete.
 
-:returns: tuple with lists of current and future errors in topology
-  (current_errors, new_errors)
+:returns: a dictionary of topology errors across all suffices in the form
+  {: (,
+  )}
 """
+suffices = api.Command.topologysuffix_find(u'')['result']
+suffix_to_masters = map_masters_to_suffices(masters, suffices)
+topo_errors_by_suffix = {}
+
+for suffix in suffices:
+suffix_name = suffix['cn'][0]
+suffix_members = suffix_to_masters[suffix_name]
+print("Checking connectivity in topology suffix '{0}'".format(
+suffix_name))
+if not check_hostname_in_masters(hostname, suffix_members):
+print(
+"'{0}' is not a part of topology suffix '{1}'".format(
+hostname, suffix_name
+)
+

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Basti]

On 13.11.2015 14:33, Petr Vobornik wrote:

On 11/13/2015 10:46 AM, Martin Babinsky wrote:


Otherwise it looks OK (not tested yet).


I didn't realize that. I have modified the patch accordingly.




Attaching updated patch with changed docstring of
'check_last_link_managed()'



ACK


Pushed to master: a6cdafd37451c99c1a7492ce5901195405881bb2

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

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Petr Vobornik]

On 11/13/2015 10:46 AM, Martin Babinsky wrote:


Otherwise it looks OK (not tested yet).


I didn't realize that. I have modified the patch accordingly.




Attaching updated patch with changed docstring of
'check_last_link_managed()'



ACK

--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Petr Vobornik]

On 11/10/2015 05:43 PM, Martin Babinsky wrote:

On 11/04/2015 06:50 PM, Petr Vobornik wrote:

On 11/04/2015 01:30 PM, Martin Babinsky wrote:

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes,
but
there is a code (install/tools/ipa-replica-manage starting at line
788
after applying my patch) which monitors whether the segments pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.




Hmm, I'm thinking whether the 'check_last_link_managed' and
'check_deleted_segments' should not be called per-suffix, but should
themselves check all suffixes available. This could make the fix for
https://fedorahosted.org/freeipa/ticket/5409 also easier.



Depends if the output is reusable. If so then why not.
check_last_link_managed basically adds text to several
get_topology_connection_errors calls.


Attaching updated patch.



I'm not sure about (pseudo code):

topo_errors = ([], [])
for each suffix:
topo_errors[0].extend(orig_errors)
topo_errors[1].extend(new_errors)
return topo_errors

In check_deleted_segments wait_for_segment_removal is per-suffix check 
but uses topo_errors which contains errors from both suffices. Topo 
erros are used to relax the check if topology is disconnected but this 
might relax it too much.


I would change the errors to per-suffix as well, e.g.:
  topo_errors = {}
  for each suffix:
  topo_errors[suffix_name] = (orig_errors, new_errors)
  return topo_errors

Otherwise it looks OK (not tested yet).
--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

On 11/12/2015 02:04 PM, Petr Vobornik wrote:

On 11/10/2015 05:43 PM, Martin Babinsky wrote:

On 11/04/2015 06:50 PM, Petr Vobornik wrote:

On 11/04/2015 01:30 PM, Martin Babinsky wrote:

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes,
but
there is a code (install/tools/ipa-replica-manage starting at line
788
after applying my patch) which monitors whether the segments
pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.




Hmm, I'm thinking whether the 'check_last_link_managed' and
'check_deleted_segments' should not be called per-suffix, but should
themselves check all suffixes available. This could make the fix for
https://fedorahosted.org/freeipa/ticket/5409 also easier.



Depends if the output is reusable. If so then why not.
check_last_link_managed basically adds text to several
get_topology_connection_errors calls.


Attaching updated patch.



I'm not sure about (pseudo code):

 topo_errors = ([], [])
 for each suffix:
 topo_errors[0].extend(orig_errors)
 topo_errors[1].extend(new_errors)
 return topo_errors

In check_deleted_segments wait_for_segment_removal is per-suffix check
but uses topo_errors which contains errors from both suffices. Topo
erros are used to relax the check if topology is disconnected but this
might relax it too much.

I would change the errors to per-suffix as well, e.g.:
   topo_errors = {}
   for each suffix:
   topo_errors[suffix_name] = (orig_errors, new_errors)
   return topo_errors

Otherwise it looks OK (not tested yet).


I didn't realize that. I have modified the patch accordingly.

--
Martin^3 Babinsky
From f163d3f45e4d82cfd046f80d45cfccaa8aac0776 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 30 Oct 2015 13:59:03 +0100
Subject: [PATCH] check for disconnected topology and deleted agreements for
 all suffices

The code in ipa-replica-manage which checks for disconnected topology and
deleted agreements during node removal was generalized so that it now performs
these checks for all suffixes to which the node belongs.

https://fedorahosted.org/freeipa/ticket/5309
---
 install/tools/ipa-replica-manage | 246 ++-
 1 file changed, 165 insertions(+), 81 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index b9998da44dcc1f01c5eb342ee713634de0ee84ee..93c1014a49ed2c32bed22dbf80c6d925e6f3d825 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -570,46 +570,96 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 else:
 return None
 
-def check_last_link_managed(api, masters, hostname, force):
+
+def map_masters_to_suffices(masters, suffices):
+masters_to_suffix = {}
+suffix_name_to_root = {
+s['iparepltopoconfroot'][0]: s['cn'][0] for s in suffices
+}
+
+for master in masters:
+managed_suffices = master['iparepltopomanagedsuffix']
+for suffix in managed_suffices:
+suffix_name = suffix_name_to_root[suffix]
+try:
+masters_to_suffix[suffix_name].append(master)
+except KeyError:
+masters_to_suffix[suffix_name] = [master]
+
+return masters_to_suffix
+
+
+def check_hostname_in_masters(hostname, masters):
+master_cns = {m['cn'][0] for m in masters}
+return hostname in master_cns
+
+
+def check_last_link_managed(api, hostname, masters, force):
 """
 Check if 'hostname' is safe to delete.
 
 :returns: tuple with lists of current and future errors in topology
   (current_errors, new_errors)
 """
+suffices = api.Command.topologysuffix_find(u'')['result']
+suffix_to_masters = map_masters_to_suffices(masters, suffices)
+topo_errors_by_suffix = {}
+
+for suffix in suffices:
+suffix_name = suffix['cn'][0]
+suffix_members = suffix_to_masters[suffix_name]
+print("Checking connectivity in topology suffix '{}'".format(
+suffix_name))
+if not check_hostname_in_masters(hostname, suffix_members):
+print(
+"'{}' is not a part of topology suffix '{}'".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], 

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

On 11/04/2015 06:50 PM, Petr Vobornik wrote:

On 11/04/2015 01:30 PM, Martin Babinsky wrote:

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes,
but
there is a code (install/tools/ipa-replica-manage starting at line 788
after applying my patch) which monitors whether the segments pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.




Hmm, I'm thinking whether the 'check_last_link_managed' and
'check_deleted_segments' should not be called per-suffix, but should
themselves check all suffixes available. This could make the fix for
https://fedorahosted.org/freeipa/ticket/5409 also easier.



Depends if the output is reusable. If so then why not.
check_last_link_managed basically adds text to several
get_topology_connection_errors calls.


Attaching updated patch.

--
Martin^3 Babinsky
From bb0506c3f79d34bf70441eddb36e0c150229e2f0 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 30 Oct 2015 13:59:03 +0100
Subject: [PATCH] check for disconnected topology and deleted agreements for
 all suffices

The code in ipa-replica-manage which checks for disconnected topology and
deleted agreements during node removal was generalized so that it now performs
these checks for all suffixes to which the node belongs.

https://fedorahosted.org/freeipa/ticket/5309
---
 install/tools/ipa-replica-manage | 247 ++-
 1 file changed, 166 insertions(+), 81 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index b9998da44dcc1f01c5eb342ee713634de0ee84ee..8a44895c17e7fdc4c5160a77f55119fd3cc7f58d 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -570,46 +570,97 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 else:
 return None
 
-def check_last_link_managed(api, masters, hostname, force):
+
+def map_masters_to_suffices(masters, suffices):
+masters_to_suffix = {}
+suffix_name_to_root = {
+s['iparepltopoconfroot'][0]: s['cn'][0] for s in suffices
+}
+
+for master in masters:
+managed_suffices = master['iparepltopomanagedsuffix']
+for suffix in managed_suffices:
+suffix_name = suffix_name_to_root[suffix]
+try:
+masters_to_suffix[suffix_name].append(master)
+except KeyError:
+masters_to_suffix[suffix_name] = [master]
+
+return masters_to_suffix
+
+
+def check_hostname_in_masters(hostname, masters):
+master_cns = {m['cn'][0] for m in masters}
+return hostname in master_cns
+
+
+def check_last_link_managed(api, hostname, masters, force):
 """
 Check if 'hostname' is safe to delete.
 
 :returns: tuple with lists of current and future errors in topology
   (current_errors, new_errors)
 """
+suffices = api.Command.topologysuffix_find(u'')['result']
+suffix_to_masters = map_masters_to_suffices(masters, suffices)
+topo_errors = ([], [])
+
+for suffix in suffices:
+suffix_name = suffix['cn'][0]
+suffix_members = suffix_to_masters[suffix_name]
+print("Checking connectivity in topology suffix '{}'".format(
+suffix_name))
+if not check_hostname_in_masters(hostname, suffix_members):
+print(
+"'{}' is not a part of topology suffix '{}'".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 '{}' 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 '{}' will lead to 

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes, but
there is a code (install/tools/ipa-replica-manage starting at line 788
after applying my patch) which monitors whether the segments pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.



Hmm, I'm thinking whether the 'check_last_link_managed' and 
'check_deleted_segments' should not be called per-suffix, but should 
themselves check all suffixes available. This could make the fix for 
https://fedorahosted.org/freeipa/ticket/5409 also easier.


--
Martin^3 Babinsky

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

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Petr Vobornik]

On 11/04/2015 01:30 PM, Martin Babinsky wrote:

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes,
but
there is a code (install/tools/ipa-replica-manage starting at line 788
after applying my patch) which monitors whether the segments pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.




Hmm, I'm thinking whether the 'check_last_link_managed' and
'check_deleted_segments' should not be called per-suffix, but should
themselves check all suffixes available. This could make the fix for
https://fedorahosted.org/freeipa/ticket/5409 also easier.



Depends if the output is reusable. If so then why not. 
check_last_link_managed basically adds text to several 
get_topology_connection_errors calls.

--
Petr Vobornik

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

[Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes, but 
there is a code (install/tools/ipa-replica-manage starting at line 788 
after applying my patch) which monitors whether the segments pointing 
to/from the deleted host are already deleted.


These checks are currently hardcoded for 'realm' prefix, should we 
generalize them as well or is it a part of other effort?


--
Martin^3 Babinsky
From 7ef87f07500b361d84e18ac3784c7f9ba9596b1f Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 30 Oct 2015 13:59:03 +0100
Subject: [PATCH] perform connectivity checks for all topology suffixes during
 node deletion

The code in ipa-replica-manage which checks for disconnected topology before
and after deletion of a node in a topology plugin-managed domain was
generalized so that it now performs these checks for all suffixes to which the
node belongs.

https://fedorahosted.org/freeipa/ticket/5309
---
 install/tools/ipa-replica-manage | 47 ++--
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 1350590b625e5dcab36abbcef75fe5eafc5f7123..05ac28cec4036676994942ad7150c9a6ae82a528 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -569,7 +569,7 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 else:
 return None
 
-def check_last_link_managed(api, masters, hostname, force):
+def check_last_link_managed(api, hostname, masters, suffix_name, force):
 """
 Check if 'hostname' is safe to delete.
 
@@ -577,13 +577,31 @@ def check_last_link_managed(api, masters, hostname, force):
   (current_errors, new_errors)
 """
 
-segments = api.Command.topologysegment_find(u'realm', sizelimit=0).get('result')
-graph = create_topology_graph(masters, segments)
+suffix = api.Command.topologysuffix_show(suffix_name)['result']
+suffix_members = []
+for m in masters:
+if suffix['iparepltopoconfroot'][0] in m['iparepltopomanagedsuffix']:
+suffix_members.append(m)
+
+member_cns = {member['cn'][0] for member in suffix_members}
+
+if hostname not in member_cns:
+print(
+"'{}' is not a part of topology suffix '{}'".format(
+hostname, suffix_name
+)
+)
+print("Not checking connectivity")
+return [], []
+
+segments = api.Command.topologysegment_find(suffix_name, sizelimit=0).get('result')
+graph = create_topology_graph(suffix_members, segments)
 
 # check topology before removal
 orig_errors = get_topology_connection_errors(graph)
 if orig_errors:
-print("Current topology is disconnected:")
+print("Current topology in suffix '{}' 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)
@@ -596,7 +614,8 @@ def check_last_link_managed(api, masters, hostname, force):
 
 new_errors = get_topology_connection_errors(graph)
 if new_errors:
-print("WARNING: Topology after removal of %s will be disconnected." % hostname)
+print("WARNING: Removal of '{}' will lead to disconnected topology "
+  "in suffix '{}'".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:")
@@ -724,8 +743,22 @@ def del_master_managed(realm, hostname, options):
 # 2. Get all masters
 masters = api.Command.server_find('', sizelimit=0)['result']
 
-# 3. Check topology
-topo_errors = check_last_link_managed(api, masters, hostname, options.force)
+# 3. Check topology connectivity in all suffices
+suffices = api.Command.topologysuffix_find('', sizelimit=0)['result']
+# initialize the error tuple here and extend it by errors found in each
+# suffix
+topo_errors = ([], [])
+
+for suffix in suffices:
+suffix_name = suffix['cn'][0]
+print("Checking connectivity in topology suffix '{}'".format(
+suffix_name))
+
+suffix_errors = check_last_link_managed(
+api, hostname, masters, suffix_name, options.force)
+
+topo_errors[0].extend(suffix_errors[0])
+topo_errors[1].extend(suffix_errors[1])
 
 # 4. Check that we are not leaving the installation without CA and/or DNS
 #And pick new CA master.
-- 
2.4.3

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

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Petr Vobornik]

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes, but
there is a code (install/tools/ipa-replica-manage starting at line 788
after applying my patch) which monitors whether the segments pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.
--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes, but
there is a code (install/tools/ipa-replica-manage starting at line 788
after applying my patch) which monitors whether the segments pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.
Ok I have included it in the attached patch so that both of these 
operations are performed for all suffixes.


--
Martin^3 Babinsky
From 356fbe7c3f542938b87f50c864c28de8b65a9b36 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 30 Oct 2015 13:59:03 +0100
Subject: [PATCH] check for disconnected topology and deleted agreements for
 all suffices

The code in ipa-replica-manage which checks for disconnected topology and
deleted agreements during node removal was generalized so that it now performs
these checks for all suffixes to which the node belongs.

https://fedorahosted.org/freeipa/ticket/5309
---
 install/tools/ipa-replica-manage | 87 ++--
 1 file changed, 66 insertions(+), 21 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 1350590b625e5dcab36abbcef75fe5eafc5f7123..f754d699e89785666dd35386a2fbb1a6017f5d1f 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -569,7 +569,7 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 else:
 return None
 
-def check_last_link_managed(api, masters, hostname, force):
+def check_last_link_managed(api, hostname, masters, suffix_name, force):
 """
 Check if 'hostname' is safe to delete.
 
@@ -577,13 +577,31 @@ def check_last_link_managed(api, masters, hostname, force):
   (current_errors, new_errors)
 """
 
-segments = api.Command.topologysegment_find(u'realm', sizelimit=0).get('result')
-graph = create_topology_graph(masters, segments)
+suffix = api.Command.topologysuffix_show(suffix_name)['result']
+suffix_members = []
+for m in masters:
+if suffix['iparepltopoconfroot'][0] in m['iparepltopomanagedsuffix']:
+suffix_members.append(m)
+
+member_cns = {member['cn'][0] for member in suffix_members}
+
+if hostname not in member_cns:
+print(
+"'{}' is not a part of topology suffix '{}'".format(
+hostname, suffix_name
+)
+)
+print("Not checking connectivity")
+return [], []
+
+segments = api.Command.topologysegment_find(suffix_name, sizelimit=0).get('result')
+graph = create_topology_graph(suffix_members, segments)
 
 # check topology before removal
 orig_errors = get_topology_connection_errors(graph)
 if orig_errors:
-print("Current topology is disconnected:")
+print("Current topology in suffix '{}' 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)
@@ -596,7 +614,8 @@ def check_last_link_managed(api, masters, hostname, force):
 
 new_errors = get_topology_connection_errors(graph)
 if new_errors:
-print("WARNING: Topology after removal of %s will be disconnected." % hostname)
+print("WARNING: Removal of '{}' will lead to disconnected topology "
+  "in suffix '{}'".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:")
@@ -724,8 +743,22 @@ def del_master_managed(realm, hostname, options):
 # 2. Get all masters
 masters = api.Command.server_find('', sizelimit=0)['result']
 
-# 3. Check topology
-topo_errors = check_last_link_managed(api, masters, hostname, options.force)
+# 3. Check topology connectivity in all suffices
+suffices = api.Command.topologysuffix_find('', sizelimit=0)['result']
+suffix_names = [s['cn'][0] for s in suffices]
+# initialize the error tuple here and extend it by errors found in each
+# suffix
+topo_errors = ([], [])
+
+for suffix_name in suffix_names:
+print("Checking connectivity in topology suffix '{}'".format(
+suffix_name))
+
+suffix_errors = check_last_link_managed(
+api, hostname, masters, suffix_name, options.force)
+
+topo_errors[0].extend(suffix_errors[0])
+topo_errors[1].extend(suffix_errors[1])
 
 # 4. Check that we are not leaving the installation without CA and/or DNS
 #