On 03/24/2016 07:10 PM, Stanislav Laznicka wrote:
On 03/23/2016 08:13 PM, Martin Basti wrote:
[...]
Can you please update design
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4 (mainly
the --suffix option)? Also there are missing clean-ruv and list-ruv
commands in design, and fix usage at the bottom.
1)
I don't understand this expression
+ if dirman_passwd is None or (
+ not dirman_passwd and args[0] in cs_enabled_commands):
You already tested if subcommand belongs to cs_enabled_commands few
lines above, IMO the 'dirman_password is None' expression is enough.
If I understand it well, when empty password is entered, the program
continues and uses Kerberos credentials for some operations. E.g. for
list-ruv, if empty password is entered, the command would only display
RUVs for domain tree but not for the CA tree no matter if CA is set up
or not (in the current state of the patch, after get_ruv refactoring).
This here is one possible way around this, although the check for
non-empty password might probably just as well be in
get_ruv_both_suffixes.
2)
+# tuple of commands that work with ca tree and need Directory Manager
password
+cs_enabled_commands = ("list-ruv", "clean-ruv", "abort-clean-ruv")
this variable is used only toi detect if dirman passwd is needed, I
suggest to rename it to commands_req_dirman_passwd, or something better.
3)
Q: Do we need is_cs_set() function?
A: Yes!
I wanted to give you ultimate NACK, but then I checked how get_ruv code
works and I changed my mind.
Please write a comment where is_cs_set function is used, why we need
extra function instead of catching an exception, possibly you can open a
refactoring ticket.
After the refactoring changes, is_cs_set should not be needed anymore,
removed it.
Shame:
1)
+ if not test_connection(realm, host, options.nolookup) or\
Please use parentheses instead of backslash
2)
+ args[0] in cs_enabled_commands:
+ not dirman_passwd and args[0] in cs_enabled_commands):
Indentation is not multiplication of 4
Shame on me indeed, fixed it.
Nitpicks (I don't insist on fixing these):
1)
+ if servers.get('ca', None):
None is default
2)
+ for (netloc, rid) in servers['ca']:
parentheses are not needed
3)
+ print("\t%s: %s" % (netloc, rid))
Would be nice to use .format() instead of %
Martin^2
I changed my mind, ultimate NACK.
Please fix get_ruv function, is_cs_set will not help. In case there
are no RUVs but CA is installed, sys.exit there prevents us from
removing RUVs (or any else operation) on domain suffix, and vice versa.
I propose to move ticket to 4.4 milestone because I would like to
avoid breaking something in 4.3, as this will hit many places in
ipa-replica-manage.
Please provide the refactoring of get_ruv as separate patch a put
these patches on top of it.
Martin2
Did the refactoring. There also seemed to be duplicit code in
abort_clean_ruv for some reason, removed it (I hope it does not break
anything, but it seemed rather useless). Also had to change the
numbers of the patches so that they would apply.
Self NACK. As I was updating the design today, I found out I omitted the
information that abort-clean-ruv should now be called with --force by
default. Updated the arguments of the abort call + man page in the patchset.
From 349a286574dd73a22e834b4bf0afb928c727e4a7 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Thu, 24 Mar 2016 15:59:31 +0100
Subject: [PATCH 1/3] ipa-replica-manage refactoring
get_ruv does not call sys.exit anymore, instead it raises RuntimeError
for better error handling
Also removed duplicit code from abort_clean_ruv
---
install/tools/ipa-replica-manage | 63 +++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 26 deletions(-)
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 6d1fc93c4c8499dcb4cd0924ff837a0216395821..9ad7cda647ebb7b1bb9ccd4a5c7b3ff9034b6b84 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -361,8 +361,9 @@ def get_ruv(realm, host, dirman_passwd, nolookup=False, ca=False):
else:
thisrepl = replication.ReplicationManager(realm, host, dirman_passwd)
except Exception as e:
- print("Failed to connect to server %s: %s" % (host, e))
- sys.exit(1)
+ root_logger.debug("Failed to connect to server {host}: {err}"
+ .format(host=host, err=e))
+ raise RuntimeError(e)
search_filter = '(&(nsuniqueid=ffffffff-ffffffff-ffffffff-ffffffff)(objectclass=nstombstone))'
try:
@@ -370,8 +371,9 @@ def get_ruv(realm, host, dirman_passwd, nolookup=False, ca=False):
thisrepl.db_suffix, thisrepl.conn.SCOPE_SUBTREE, search_filter,
['nsds50ruv'])
except errors.NotFound:
- print("No RUV records found.")
- sys.exit(0)
+ root_logger.debug("No RUV records found on host {host}."
+ .format(host=host))
+ raise RuntimeError("No RUV records found.")
servers = []
for e in entries:
@@ -384,7 +386,7 @@ def get_ruv(realm, host, dirman_passwd, nolookup=False, ca=False):
(scheme, netloc, path, params, query, fragment) = urlparse(data.group(2))
servers.append((netloc, rid))
else:
- print("unable to decode: %s" % ruv)
+ root_logger.debug("unable to decode: %s" % ruv)
return servers
@@ -393,8 +395,12 @@ def list_ruv(realm, host, dirman_passwd, verbose, nolookup=False):
List the Replica Update Vectors on this host to get the available
replica IDs.
"""
-
- servers = get_ruv(realm, host, dirman_passwd, nolookup)
+ try:
+ servers = get_ruv(realm, host, dirman_passwd, nolookup)
+ except RuntimeError as e:
+ root_logger.debug('list_ruv failed for host {host}: {err}'
+ .format(host=host, err=e))
+ sys.exit(e)
for (netloc, rid) in servers:
print("%s: %s" % (netloc, rid))
@@ -402,7 +408,12 @@ def get_rid_by_host(realm, sourcehost, host, dirman_passwd, nolookup=False):
"""
Try to determine the RID by host name.
"""
- servers = get_ruv(realm, sourcehost, dirman_passwd, nolookup)
+ try:
+ servers = get_ruv(realm, sourcehost, dirman_passwd, nolookup)
+ except RuntimeError as e:
+ root_logger.debug('get_rid_by_host failed for host {host}: {err}'
+ .format(host=sourcehost, err=e))
+ sys.exit(e)
for (netloc, rid) in servers:
if '%s:389' % host == netloc:
return int(rid)
@@ -416,8 +427,14 @@ def clean_ruv(realm, ruv, options, ca=False):
except ValueError:
sys.exit("Replica ID must be an integer: %s" % ruv)
- servers = get_ruv(realm, options.host, options.dirman_passwd,
- options.nolookup, ca=ca)
+ try:
+ servers = get_ruv(realm, options.host, options.dirman_passwd,
+ options.nolookup, ca=ca)
+ except RuntimeError as e:
+ root_logger.debug('clean_ruv failed for host {host}: {err}'
+ .format(host=options.host, err=e))
+ sys.exit(e)
+
found = False
for (netloc, rid) in servers:
if ruv == int(rid):
@@ -461,26 +478,20 @@ def abort_clean_ruv(realm, ruv, options):
except ValueError:
sys.exit("Replica ID must be an integer: %s" % ruv)
- servers = get_ruv(realm, options.host, options.dirman_passwd,
- options.nolookup)
- found = False
- for (netloc, rid) in servers:
- if ruv == int(rid):
- found = True
- hostname = netloc
- break
-
- if not found:
- sys.exit("Replica ID %s not found" % ruv)
+ try:
+ servers = get_ruv(realm, options.host, options.dirman_passwd,
+ options.nolookup)
+ except RuntimeError as e:
+ root_logger.debug('abort_clean_ruv failed for host {host}: {err}'
+ .format(host=options.host, err=e))
+ sys.exit(e)
- servers = get_ruv(realm, options.host, options.dirman_passwd,
- options.nolookup)
found = False
for (netloc, rid) in servers:
if ruv == int(rid):
- found = True
- hostname = netloc
- break
+ found = True
+ hostname = netloc
+ break
if not found:
sys.exit("Replica ID %s not found" % ruv)
--
2.5.0
From 84c53cccee777f32cc5b56457c90ea92e1b71f2e Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 11 Mar 2016 10:15:02 +0100
Subject: [PATCH 2/3] abort-clean/list/clean-ruv now work for both suffixes
The rid passed to abort-clean-ruv and clean-ruv is now searched
for in both ipaca and domain trees as well as list-ruv now
displays both RUVs and CS-RUVs
https://fedorahosted.org/freeipa/ticket/4987
---
install/tools/ipa-replica-manage | 123 +++++++++++++++++++++++----------
install/tools/man/ipa-replica-manage.1 | 10 ++-
2 files changed, 94 insertions(+), 39 deletions(-)
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 9ad7cda647ebb7b1bb9ccd4a5c7b3ff9034b6b84..b02c6924a58155f0a97b66c23431a2146395e48e 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -68,6 +68,8 @@ commands = {
"dnanextrange-set":(2, 2, "<master fqdn> <range>", "must provide a master and ID range"),
}
+# tuple of commands that work with ca tree and need Directory Manager password
+dirman_passwd_req_commands = ("list-ruv", "clean-ruv", "abort-clean-ruv")
def parse_options():
parser = IPAOptionParser(version=version.VERSION)
@@ -390,19 +392,50 @@ def get_ruv(realm, host, dirman_passwd, nolookup=False, ca=False):
return servers
+
+def get_ruv_both_suffixes(realm, host, dirman_passwd, nolookup=False):
+ """
+ Get RUVs for both domain and ipaca suffixes
+ """
+ ruvs = {}
+
+ try:
+ ruvs['ca'] = get_ruv(realm, host, dirman_passwd, nolookup, True)
+ except RuntimeError as e:
+ root_logger.debug("Failed to get CS-RUVs from {host}: {err}"
+ .format(host=host, err=e))
+ try:
+ ruvs['domain'] = get_ruv(realm, host, dirman_passwd, nolookup)
+ except RuntimeError as e:
+ root_logger.debug("Failed to get RUVs from {host}: {err}"
+ .format(host=host, err=e))
+
+ if not ruvs.keys():
+ raise RuntimeError("No RUV records found.")
+
+ return ruvs
+
+
def list_ruv(realm, host, dirman_passwd, verbose, nolookup=False):
"""
List the Replica Update Vectors on this host to get the available
replica IDs.
"""
try:
- servers = get_ruv(realm, host, dirman_passwd, nolookup)
+ servers = get_ruv_both_suffixes(realm, host, dirman_passwd, nolookup)
except RuntimeError as e:
root_logger.debug('list_ruv failed for host {host}: {err}'
.format(host=host, err=e))
sys.exit(e)
- for (netloc, rid) in servers:
- print("%s: %s" % (netloc, rid))
+ if servers.get('domain'):
+ print('Replica Update Vectors:')
+ for netloc, rid in servers['domain']:
+ print("\t{name}: {id}".format(netloc, rid))
+ if servers.get('ca'):
+ print('Certificate Server Replica Update Vectors:')
+ for netloc, rid in servers['ca']:
+ print("\t{name}: {id}".format(netloc, rid))
+
def get_rid_by_host(realm, sourcehost, host, dirman_passwd, nolookup=False):
"""
@@ -418,7 +451,8 @@ def get_rid_by_host(realm, sourcehost, host, dirman_passwd, nolookup=False):
if '%s:389' % host == netloc:
return int(rid)
-def clean_ruv(realm, ruv, options, ca=False):
+
+def clean_ruv(realm, ruv, options):
"""
Given an RID create a CLEANALLRUV task to clean it up.
"""
@@ -428,24 +462,27 @@ def clean_ruv(realm, ruv, options, ca=False):
sys.exit("Replica ID must be an integer: %s" % ruv)
try:
- servers = get_ruv(realm, options.host, options.dirman_passwd,
- options.nolookup, ca=ca)
+ servers = get_ruv_both_suffixes(realm, options.host,
+ options.dirman_passwd,
+ options.nolookup)
except RuntimeError as e:
root_logger.debug('clean_ruv failed for host {host}: {err}'
.format(host=options.host, err=e))
sys.exit(e)
+ tree_found = None
+ for tree, ruvs in servers.items():
+ for netloc, rid in ruvs:
+ if ruv == int(rid):
+ tree_found = tree
+ hostname = netloc
+ break
+ if tree_found:
+ break
- found = False
- for (netloc, rid) in servers:
- if ruv == int(rid):
- found = True
- hostname = netloc
- break
-
- if not found:
+ if not tree_found:
sys.exit("Replica ID %s not found" % ruv)
- if ca:
+ if tree_found == 'ca':
print("Clean the Certificate Server Replication Update Vector for %s"
% hostname)
else:
@@ -460,7 +497,7 @@ def clean_ruv(realm, ruv, options, ca=False):
if not ipautil.user_input("Continue to clean?", False):
sys.exit("Aborted")
- if ca:
+ if tree_found == 'ca':
thisrepl = replication.get_cs_replication_manager(realm, options.host,
options.dirman_passwd)
else:
@@ -469,6 +506,7 @@ def clean_ruv(realm, ruv, options, ca=False):
thisrepl.cleanallruv(ruv)
print("Cleanup task created")
+
def abort_clean_ruv(realm, ruv, options):
"""
Given an RID abort a CLEANALLRUV task.
@@ -479,31 +517,40 @@ def abort_clean_ruv(realm, ruv, options):
sys.exit("Replica ID must be an integer: %s" % ruv)
try:
- servers = get_ruv(realm, options.host, options.dirman_passwd,
- options.nolookup)
+ servers = get_ruv_both_suffixes(realm, options.host,
+ options.dirman_passwd,
+ options.nolookup)
except RuntimeError as e:
root_logger.debug('abort_clean_ruv failed for host {host}: {err}'
.format(host=options.host, err=e))
sys.exit(e)
- found = False
- for (netloc, rid) in servers:
- if ruv == int(rid):
- found = True
- hostname = netloc
+ tree_found = None
+ for tree, ruvs in servers.items():
+ for netloc, rid in ruvs:
+ if ruv == int(rid):
+ tree_found = tree
+ hostname = netloc
+ break
+ if tree_found:
break
- if not found:
+ if not tree_found:
sys.exit("Replica ID %s not found" % ruv)
print("Aborting the clean Replication Update Vector task for %s" % hostname)
print()
- thisrepl = replication.ReplicationManager(realm, options.host,
- options.dirman_passwd)
- thisrepl.abortcleanallruv(ruv, options.force)
+ if tree_found == 'ca':
+ thisrepl = replication.get_cs_replication_manager(realm, options.host,
+ options.dirman_passwd)
+ else:
+ thisrepl = replication.ReplicationManager(realm, options.host,
+ options.dirman_passwd)
+ thisrepl.abortcleanallruv(ruv, force=True)
print("Cleanup task stopped")
+
def list_clean_ruv(realm, host, dirman_passwd, verbose, nolookup=False):
"""
List all clean RUV tasks.
@@ -699,7 +746,7 @@ def clean_dangling_ruvs(realm, host, options):
for csruv in master_info['clean_csruv']:
if csruv[1] not in cleaned:
cleaned.add(csruv[1])
- clean_ruv(realm, csruv[1], options, ca=True)
+ clean_ruv(realm, csruv[1], options)
def check_last_link(delrepl, realm, dirman_passwd, force):
@@ -1572,10 +1619,12 @@ def main(options, args):
if options.dirman_passwd:
dirman_passwd = options.dirman_passwd
else:
- if not test_connection(realm, host, options.nolookup):
+ if (not test_connection(realm, host, options.nolookup) or
+ args[0] in dirman_passwd_req_commands):
dirman_passwd = installutils.read_password("Directory Manager",
confirm=False, validate=False, retry=False)
- if dirman_passwd is None:
+ if dirman_passwd is None or (
+ not dirman_passwd and args[0] in dirman_passwd_req_commands):
sys.exit("Directory Manager password required")
options.dirman_passwd = dirman_passwd
@@ -1593,8 +1642,6 @@ def main(options, args):
replica = args[1]
list_replicas(realm, host, replica, dirman_passwd, options.verbose,
options.nolookup)
- elif args[0] == "list-ruv":
- list_ruv(realm, host, dirman_passwd, options.verbose, options.nolookup)
elif args[0] == "del":
del_master(realm, args[1], options)
elif args[0] == "re-initialize":
@@ -1625,10 +1672,14 @@ def main(options, args):
replica1 = host
replica2 = args[1]
del_link(realm, replica1, replica2, dirman_passwd)
- elif args[0] == "clean-ruv":
- clean_ruv(realm, args[1], options)
- elif args[0] == "abort-clean-ruv":
- abort_clean_ruv(realm, args[1], options)
+ elif args[0] in dirman_passwd_req_commands:
+ if args[0] == "list-ruv":
+ list_ruv(realm, host, dirman_passwd, options.verbose,
+ options.nolookup)
+ elif args[0] == "clean-ruv":
+ clean_ruv(realm, args[1], options)
+ elif args[0] == "abort-clean-ruv":
+ abort_clean_ruv(realm, args[1], options)
elif args[0] == "list-clean-ruv":
list_clean_ruv(realm, host, dirman_passwd, options.verbose,
options.nolookup)
diff --git a/install/tools/man/ipa-replica-manage.1 b/install/tools/man/ipa-replica-manage.1
index ae109c4c5ff4720eb70b06c6f14a791088696d47..07b48636ef86c913740bde55304e0e29516d89cf 100644
--- a/install/tools/man/ipa-replica-manage.1
+++ b/install/tools/man/ipa-replica-manage.1
@@ -57,7 +57,7 @@ The available commands are:
\- Cleans all RUVs and CS\-RUVs that are left in the system from uninstalled replicas.
.TP
\fBabort\-clean\-ruv\fR [REPLICATION_ID]
-\- Abort a running CLEANALLRUV task. With \-\-force option the task does not wait for all the replica servers to have been sent the abort task, or be online, before completing.
+\- Abort a running CLEANALLRUV task.
.TP
\fBlist\-clean\-ruv\fR
\- List all running CLEANALLRUV and abort CLEANALLRUV tasks.
@@ -135,6 +135,7 @@ Password for the IPA system user used by the Windows PassSync plugin to synchron
.TP
\fB\-\-from\fR=\fISERVER\fR
The server to pull the data from, used by the re\-initialize and force\-sync commands.
+.TP
.SH "RANGES"
IPA uses the 389\-ds Distributed Numeric Assignment (DNA) Plugin to allocate POSIX ids for users and groups. A range is created when IPA is installed and half the range is assigned to the first IPA master for the purposes of allocation.
.TP
@@ -190,8 +191,11 @@ Using connect/disconnect you can manage the replication topology.
.TP
List the replication IDs in use:
# ipa\-replica\-manage list\-ruv
- srv1.example.com:389: 7
- srv2.example.com:389: 4
+ Replica Update Vectors:
+ srv1.example.com:389: 7
+ srv2.example.com:389: 4
+ Certificate Server Replica Update Vectors:
+ srv1.example.com:389: 9
.TP
Remove references to an orphaned and deleted master:
# ipa\-replica\-manage del \-\-force \-\-cleanup master.example.com
--
2.5.0
From 1407b496887ca8c2f934c8aaff0a45c4fe60eb21 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Wed, 16 Mar 2016 12:28:36 +0100
Subject: [PATCH 3/3] Moved password check from clean_dangling_ruv
The proper password check is now done elsewhere
https://fedorahosted.org/freeipa/ticket/4987
---
install/tools/ipa-replica-manage | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index b02c6924a58155f0a97b66c23431a2146395e48e..01e2cbb37e0631bf8d66bb56bcf8c75f5caaed2c 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -69,7 +69,8 @@ commands = {
}
# tuple of commands that work with ca tree and need Directory Manager password
-dirman_passwd_req_commands = ("list-ruv", "clean-ruv", "abort-clean-ruv")
+dirman_passwd_req_commands = ("list-ruv", "clean-ruv", "abort-clean-ruv",
+ "clean-dangling-ruv")
def parse_options():
parser = IPAOptionParser(version=version.VERSION)
@@ -598,15 +599,6 @@ def clean_dangling_ruvs(realm, host, options):
Cleans all RUVs and CS-RUVs that are left in the system from
uninstalled replicas
"""
- # get the Directory Manager password
- if not options.dirman_passwd:
- options.dirman_passwd = installutils.read_password('Directory Manager',
- confirm=False,
- validate=False,
- retry=False)
- if options.dirman_passwd is None:
- sys.exit('Directory Manager password is required')
-
conn = ipaldap.IPAdmin(host, 636, cacert=CACERT)
try:
conn.do_simple_bind(bindpw=options.dirman_passwd)
@@ -1680,11 +1672,11 @@ def main(options, args):
clean_ruv(realm, args[1], options)
elif args[0] == "abort-clean-ruv":
abort_clean_ruv(realm, args[1], options)
+ elif args[0] == "clean-dangling-ruv":
+ clean_dangling_ruvs(realm, host, options)
elif args[0] == "list-clean-ruv":
list_clean_ruv(realm, host, dirman_passwd, options.verbose,
options.nolookup)
- elif args[0] == "clean-dangling-ruv":
- clean_dangling_ruvs(realm, host, options)
elif args[0] == "dnarange-show":
if len(args) == 2:
master = args[1]
--
2.5.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
