Re: [Freeipa-devel] [PATCH 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-17 Thread Martin Basti 



On 17.06.2016 16:18, Martin Babinsky wrote:

On 06/17/2016 03:56 PM, Martin Babinsky wrote:

On 06/16/2016 12:45 PM, Martin Basti wrote:



On 15.06.2016 15:29, Martin Babinsky wrote:

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.





Hello, I have a few comments:

1)
you reused ID numbers for the your messages

+class ServerRemovalInfo(PublicMessage):
...
+errno = 13020

+class ServerRemovalWarning(PublicMessage):
...
+errno = 13021


 class FailedToRemoveHostDNSRecords(PublicMessage):
...
 errno = 13020


 class DNSForwardPolicyConflictWithEmptyZone(PublicMessage):
...
 errno = 13021

2)
+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)))

* _(str(e)): gettext cannot be used by this way
* str(e): you dont need to convert exception to string, this is done
automatically in exception


3) gettext again
+self.add_message(
+messages.ServerRemovalWarning(
+message=_(msg)
+)
+)


4)
+messages.ServerRemovalWarning(
+message=_("Failed to clean memberPrincipal
{principal}"
+  " from s4u2proxy entry {dn}:
{err}".format(
+ principal=member_principal,
+  dn=dn,
+  err=e

+messages.ServerRemovalWarning(
+message=_("Failed to clean up DNA hostname entries
for "
+  "{master}: {err}".format(
+  master=master, err=e

several more times

I'm not sure if this will work, for safety I would prefer to change it
to dictionary substitution
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=668226
It looks like it was fixed in gettext 18.3, some distributions still
have the older one

I have to test more how gettext works with the new python format 
strings


5)
+def interactive_prompt_callback(self, kw):
+self.api.Backend.textui.print_plain(
+_("Removing {server} from replication topology, "
+  "please wait...".format(server=', '.join(kw['cn']

Will this work? IMO this should be on client side





Updated and rebased patches attached.



I made an error during rebase of patch 153. Re-sending the whole batch 
with the correct one.



ACK, rebased and pushed
master:
* d8ae2b4055284de8c1baf76819d6611978f83cc6 ipaserver module for working 
with managed topology
* db882ae8d6eba768e08be9317e386f8ab3c8fcf7 delegate removal of master 
DNS record and replica keys to separate functions
* a6eb87bd68295e15ea19f5cb274cffbef5954d04 server-del: perform full 
master removal in managed topology

* 081941a5b9c9ac8832c465b857032e474bb9b09f CI test suite for `server-del`
* 47decc9b843b1b1d292511bcc8a24f8ac85745c0 ipa-replica-manage: use 
`server_del` when removing domain level 1 replica
* 31ffe1a12922b5118c847cbd6ac1ca9ea232ef94 remove the master from 
managed topology during uninstallation


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

Re: [Freeipa-devel] [PATCH 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-17 Thread Martin Babinsky 

On 06/17/2016 03:56 PM, Martin Babinsky wrote:

On 06/16/2016 12:45 PM, Martin Basti wrote:



On 15.06.2016 15:29, Martin Babinsky wrote:

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.





Hello, I have a few comments:

1)
you reused ID numbers for the your messages

+class ServerRemovalInfo(PublicMessage):
...
+errno = 13020

+class ServerRemovalWarning(PublicMessage):
...
+errno = 13021


 class FailedToRemoveHostDNSRecords(PublicMessage):
...
 errno = 13020


 class DNSForwardPolicyConflictWithEmptyZone(PublicMessage):
...
 errno = 13021

2)
+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)))

* _(str(e)): gettext cannot be used by this way
* str(e): you dont need to convert exception to string, this is done
automatically in exception


3) gettext again
+self.add_message(
+messages.ServerRemovalWarning(
+message=_(msg)
+)
+)


4)
+messages.ServerRemovalWarning(
+message=_("Failed to clean memberPrincipal
{principal}"
+  " from s4u2proxy entry {dn}:
{err}".format(
+  principal=member_principal,
+  dn=dn,
+  err=e

+messages.ServerRemovalWarning(
+message=_("Failed to clean up DNA hostname entries
for "
+  "{master}: {err}".format(
+  master=master, err=e

several more times

I'm not sure if this will work, for safety I would prefer to change it
to dictionary substitution
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=668226
It looks like it was fixed in gettext 18.3, some distributions still
have the older one

I have to test more how gettext works with the new python format strings

5)
+def interactive_prompt_callback(self, kw):
+self.api.Backend.textui.print_plain(
+_("Removing {server} from replication topology, "
+  "please wait...".format(server=', '.join(kw['cn']

Will this work? IMO this should be on client side





Updated and rebased patches attached.



I made an error during rebase of patch 153. Re-sending the whole batch 
with the correct one.


--
Martin^3 Babinsky
From 36d72836a6704a0bc666d75c6e84b94067a22256 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 8 Jun 2016 18:16:24 +0200
Subject: [PATCH 1/7] 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| 195 +++
 4 files changed, 199 insertions(+), 52 deletions(-)
 create mode 100644 ipaserver/topology.py

diff --git a/ipalib/util.py b/ipalib/util.py
index

Re: [Freeipa-devel] [PATCH 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-17 Thread Martin Babinsky 

On 06/16/2016 12:45 PM, Martin Basti wrote:



On 15.06.2016 15:29, Martin Babinsky wrote:

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.





Hello, I have a few comments:

1)
you reused ID numbers for the your messages

+class ServerRemovalInfo(PublicMessage):
...
+errno = 13020

+class ServerRemovalWarning(PublicMessage):
...
+errno = 13021


 class FailedToRemoveHostDNSRecords(PublicMessage):
...
 errno = 13020


 class DNSForwardPolicyConflictWithEmptyZone(PublicMessage):
...
 errno = 13021

2)
+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)))

* _(str(e)): gettext cannot be used by this way
* str(e): you dont need to convert exception to string, this is done
automatically in exception


3) gettext again
+self.add_message(
+messages.ServerRemovalWarning(
+message=_(msg)
+)
+)


4)
+messages.ServerRemovalWarning(
+message=_("Failed to clean memberPrincipal
{principal}"
+  " from s4u2proxy entry {dn}:
{err}".format(
+  principal=member_principal,
+  dn=dn,
+  err=e

+messages.ServerRemovalWarning(
+message=_("Failed to clean up DNA hostname entries
for "
+  "{master}: {err}".format(
+  master=master, err=e

several more times

I'm not sure if this will work, for safety I would prefer to change it
to dictionary substitution
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=668226
It looks like it was fixed in gettext 18.3, some distributions still
have the older one

I have to test more how gettext works with the new python format strings

5)
+def interactive_prompt_callback(self, kw):
+self.api.Backend.textui.print_plain(
+_("Removing {server} from replication topology, "
+  "please wait...".format(server=', '.join(kw['cn']

Will this work? IMO this should be on client side





Updated and rebased patches attached.

--
Martin^3 Babinsky
From 03fb6f81c57794b295dcb3f8eeb85202e05df549 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 8 Jun 2016 18:16:24 +0200
Subject: [PATCH 1/7] 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 68d11fc6c8f3689726765aa34759a2d3aabe42ef..8435f7ab6e8fd66caacb1641a4ef5409382637c5 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -45,7 +45,6 @@ from ipapython.ssh import SSHPublicKey
 from

Re: [Freeipa-devel] [PATCH 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-16 Thread Martin Basti 



On 15.06.2016 15:29, Martin Babinsky wrote:

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.





Hello, I have a few comments:

1)
you reused ID numbers for the your messages

+class ServerRemovalInfo(PublicMessage):
...
+errno = 13020

+class ServerRemovalWarning(PublicMessage):
...
+errno = 13021


 class FailedToRemoveHostDNSRecords(PublicMessage):
...
 errno = 13020


 class DNSForwardPolicyConflictWithEmptyZone(PublicMessage):
...
 errno = 13021

2)
+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)))

* _(str(e)): gettext cannot be used by this way
* str(e): you dont need to convert exception to string, this is done 
automatically in exception



3) gettext again
+self.add_message(
+messages.ServerRemovalWarning(
+message=_(msg)
+)
+)


4)
+messages.ServerRemovalWarning(
+message=_("Failed to clean memberPrincipal 
{principal}"
+  " from s4u2proxy entry {dn}: 
{err}".format(

+  principal=member_principal,
+  dn=dn,
+  err=e

+messages.ServerRemovalWarning(
+message=_("Failed to clean up DNA hostname entries 
for "

+  "{master}: {err}".format(
+  master=master, err=e

several more times

I'm not sure if this will work, for safety I would prefer to change it 
to dictionary substitution

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=668226
It looks like it was fixed in gettext 18.3, some distributions still 
have the older one


I have to test more how gettext works with the new python format strings

5)
+def interactive_prompt_callback(self, kw):
+self.api.Backend.textui.print_plain(
+_("Removing {server} from replication topology, "
+  "please wait...".format(server=', '.join(kw['cn']

Will this work? IMO this should be on client side


-- 
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 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-15 Thread Martin Babinsky 

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?



Because this uses server API which calls command's `execute` directly so 
it does not pass version automatically.


So you get version mismatch warning every time this function is called.



Patch 0158: LGTM


Honza




--
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 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-15 Thread Jan Cholasta 

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

--
Jan Cholasta

--
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 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-12 Thread Martin Babinsky 

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.

--
Martin^3 Babinsky
From b6b5a35d4a47395853db77b3e9289bb927f9feba Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 8 Jun 2016 18:16:24 +0200
Subject: [PATCH 01/06] ipapython 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 --
 ipapython/topology.py| 193 +++
 ipaserver/install/replication.py |   3 +-
 ipaserver/plugins/topology.py|   2 +-
 4 files changed, 196 insertions(+), 52 deletions(-)
 create mode 100644 ipapython/topology.py

diff --git a/ipalib/util.py b/ipalib/util.py
index 2c8772e525ddf0c7a3a39c7e682166551b588fcb..0ef5f00f1baed2a1ba766ca0fda78b2b61061cce 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/ipapython/topology.py b/ipapython/topology.py
new file mode 100644
index ..245a2b254353a28e8d567bbbaf68e043c9305f97
--- /dev/null
+++ b/ipapython/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)
+

[Freeipa-devel] [PATCH 0153-0158] move ipa-replica-manage del functionality into server-del

2016-06-09 Thread Martin Babinsky 
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

--
Martin^3 Babinsky
From e5c106a2d004be8086d9a53c98bc50cc226743d4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 8 Jun 2016 18:16:24 +0200
Subject: [PATCH 1/6] ipapython 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 --
 ipapython/topology.py| 193 +++
 ipaserver/install/replication.py |   3 +-
 ipaserver/plugins/topology.py|   2 +-
 4 files changed, 196 insertions(+), 52 deletions(-)
 create mode 100644 ipapython/topology.py

diff --git a/ipalib/util.py b/ipalib/util.py
index 2c8772e525ddf0c7a3a39c7e682166551b588fcb..0ef5f00f1baed2a1ba766ca0fda78b2b61061cce 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/ipapython/topology.py b/ipapython/topology.py
new file mode 100644
index ..245a2b254353a28e8d567bbbaf68e043c9305f97
--- /dev/null
+++ b/ipapython/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':
+