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: http://www.freeipa.org/page/Contribute/Code
