On 03/01/2013 11:57 PM, Rob Crittenden wrote:
Implement the design at http://freeipa.org/page/V3/Recover_DNA_Ranges
Could you add the link to the commit message?
Note that this required some new ACIs in cn=config which is not
replicated so the range-set commands won't work against older instances.
It should be gracefully handled though.
I think noting this in the man page would be helpful.
On new installs, the ACI on cn=Posix IDs,cn=Distributed Numeric
Assignment Plugin,cn=plugins,cn=config is added before the entry itself.
I didn't test everything as I didn't get the access.
It also doesn't work so well if you try it using a delegated
administrator, see ticket https://fedorahosted.org/freeipa/ticket/3480
rob
It should be possible to shrink existing ranges, i.e. ones that overlap
with themselves:
$ ipa-replica-manage dnarange-show
vm-075.idm.lab.eng.brq.redhat.com: 1401000005-1401100499
$ ipa-replica-manage dnarange-set vm-075.idm.lab.eng.brq.redhat.com
1401000005-1401100498
New range overlaps the DNA range on vm-075.idm.lab.eng.brq.redhat.com
freeipa-rcrit-1088-dnarange.patch
From 9a654b0b3730f8d9058dfbf25a93a58e1f4939e7 Mon Sep 17 00:00:00 2001
From: Rob Crittenden<rcrit...@redhat.com>
Date: Fri, 1 Mar 2013 15:02:14 -0500
Subject: [PATCH] Extend ipa-replica-manage to be able to manage DNA ranges.
Attempt to automatically save DNA ranges when a master is removed.
This is done by trying to find a master that does not yet define
a DNA on-deck range. If one can be found then the range on the deleted
master is added.
If one cannot be found then it is reported as an error.
Some validation of the ranges are done to ensure that they do overlap
an IPA local range and do not overlap existing DNA ranges configured
on other masters.
The patch adds some trailing whitespace, please trim it.
I also found some nitpicks, see below.
https://fedorahosted.org/freeipa/ticket/3321
---
[...]
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index
859809bf1c301913c3eb7fc92d1ed58675609b8c..6c0b45620dd2deabfc11ef2249b18205fb23b1fd
100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
[...]
+def showDNARanges(hostname, master, realm, dirman_passwd, nextrange=False):
Style issue: please don't use camel case for functions.
[...]
+ try:
+ repl = replication.ReplicationManager(realm, hostname, dirman_passwd)
+ except Exception, e:
+ sys.exit("Connection failed: %s" % ipautil.convert_ldap_error(e))
ipaldap should convert LDAP errors to IPA ones, there's no need to call
convert_ldap_error. Same in other places.
+ dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), repl.suffix)
+ try:
+ entries = repl.conn.get_entries(dn, repl.conn.SCOPE_ONELEVEL)
+ except:
Don't use a bare except. Same in other places.
[...]
+def setDNARange(hostname, range, realm, dirman_passwd, next_range=False):
+ """
+ Given a DNA range try to change it on the designated master.
+
+ The range must not overlap with any other ranges and must be within
+ one of the IPA local ranges as defined in cn=ranges.
+
+ Setting an on-deck range of 0-0 removes the range.
+
+ Return True if range was saved, False if not
+
+ hostname: hostname of the master to set the range on
+ range: The DNA range to set
+ realm: our realm, needed to create a connection
+ dirman_passwd: the DM password, needed to create a connection
Please also mention next_range.
[...]
+ def range_intersection(s1, s2, r1, r2):
+ overlap = xrange(max(s1, r1), min(s2, r2) + 1)
+ return len(overlap) > 0
That looks complicated. How about:
def ranges_intersect(s1, s2, r1, r2):
return max(s1, r1) <= min(s2, r2)
[...]
+ # Normalize the range
+ (dna_next, dna_max) = range.split('-', 1)
Can this be done before validate_range, so id doesn't have to be
duplicated there?
[...]
+ dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'),
ipautil.realm_to_suffix(realm))
I think you should use repl.suffix instead of generating it again.
[...]
+ # Verify that this is within one of the IPA domain ranges.
+ dn = DN(('cn','ranges'),('cn','etc'),repl.suffix)
Style issue: no spaces after commas
+ try:
+ entries = repl.conn.get_entries(dn, repl.conn.SCOPE_ONELEVEL,
+ "(objectclass=ipaDomainIDRange)")
+ except errors.NotFound, e:
+ sys.exit('Unable to load IPA ranges: %s' % e.message)
+
+ failed = 0
+ for ent in entries:
This loops more than necessary and is somewhat hard to follow. Consider
using for-else here:
for ...:
...
if okay:
break
else:
raise error
+ entry_start = int(ent.single_value('ipabaseid'))
+ entry_max = entry_start + int(ent.single_value('ipaidrangesize'))
+ if not range_intersection(dna_next, dna_max, entry_start,
entry_max):
I think we want the DNA range to be fully contained in the idrange,
rather than just overlap a part of it.
Please also adjust the man page when you change this.
+ failed += 1
+
+ if failed == len(entries):
+ sys.exit("New range does not fit within existing IPA ranges. See ipa
help idrange command")
+ # If this falls within any of the AD ranges then it fails.
+ try:
+ entries = repl.conn.get_entries(dn, repl.conn.SCOPE_BASE,
+
"(objectclass=ipatrustedaddomainrange)")
If we add more types of ranges in the future, should they also be
checked? Would (!(objectclass=ipaDomainIDRange)) be more appropriate here?
[...]
diff --git a/install/tools/man/ipa-replica-manage.1
b/install/tools/man/ipa-replica-manage.1
index
836743902278ec2273f3ce7a7fbf3992370c4828..d23e2566eb9a22c70991cbdca0140eb1d268533c
100644
--- a/install/tools/man/ipa-replica-manage.1
+++ b/install/tools/man/ipa-replica-manage.1
@@ -16,13 +16,13 @@
.\"
.\" Author: Rob Crittenden<rcrit...@redhat.com>
.\"
-.TH "ipa-replica-manage" "1" "Mar 14 2008" "FreeIPA" "FreeIPA Manual Pages"
+.TH "ipa-replica-manage" "1" "Mar 1 2013" "FreeIPA" "FreeIPA Manual Pages"
.SH "NAME"
ipa\-replica\-manage \- Manage an IPA replica
.SH "SYNOPSIS"
-ipa\-replica\-manage [\fIOPTION\fR]...
[connect|disconnect|del|list|re\-initialize|force\-sync]
+ipa\-replica\-manage [\fIOPTION\fR]... [COMMAND}
Please straighten the curly brace at the end
.SH "DESCRIPTION"
-Manages the replication agreements of an IPA server.
+Manages the replication agreements of an IPA server. The available commands
are:
.TP
\fBconnect\fR [SERVER_A] <SERVER_B>
\- Adds a new replication agreement between SERVER_A/localhost and SERVER_B
@@ -54,6 +54,18 @@ Manages the replication agreements of an IPA server.
\fBlist\-clean\-ruv\fR
\- List all running CLEANALLRUV and abort CLEANALLRUV tasks.
.TP
+\fBipadnarange\-show [SERVER]\fR
The subcommand is dnarange-show, no ipa at the start. Same for the others.
+\- List the DNA ranges
+.TP
+\fBipadnarange\-set SERVER x\-y\fR
I'd use START-END instead of x-y
+\- Set the DNA range on a master
+.TP
+\fBipadnanextrange\-show [SERVER]\fR
+\- List the next DNA ranges
+.TP
+\fBipadnanextrange\-set SERVER x\-y\fR
here too
[...]
+The DNA range and on\-deck (next) values can be managed using the
dnarange\-set and dnanextrange\-set commands. The rules for managing these
ranges are:
+\- The range must overlap a local range as defined by the ipa idrange command.
+
+\- The range cannot overlap the DNA range or on\-deck range on another IPA
master.
+
+\- The primary DNA range cannot be removed.
+
+\- An on\-deck range range can be removed by setting it to 0\-0. The
assumption is that the range will be manually moved or merged elsewhere.
Also, a range can't overlap ranges of trusted AD domains.
[...]
index
804d046bf2553daa4aded5c23436a98636e20da0..9076c8396041a95c7ea01ef15aa77991516d30e6
100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -1308,3 +1308,123 @@ class ReplicationManager(object):
print "This may be safely interrupted with Ctrl+C"
wait_for_task(self.conn, dn)
+
+ def getDNARange(self, hostname):
Style issue: please don't use camel-case for methods.
+ """
+ Return the DNA range on this server as a tuple, (next, max), or
+ (None, None) if no range has been assigned yet.
+ """
+ dn = DN(('cn', 'Posix IDs'), ('cn', 'Distributed Numeric Assignment
Plugin'), ('cn', 'plugins'), ('cn', 'config'))
I'd put this in a global constant.
+ try:
+ entry = self.conn.get_entry(dn)
+ except Exception, e:
+ print "Unable to read DNA configuration: %s" % e.message
I think it's better to communicate the error by raising an exception,
rather than pretending the range hasn't been set yet.
With prints, the error won't appear in logs, and can't be checked by the
caller.
Same elsewhere.
[...]
+ if (nextvalue > maxvalue and maxvalue == 1100 and
+ nextvalue == 1101 and remaining == 0):
What are the magic values? Also this redundantly checks if 1101 > 1100.
I'd expect the DNS plugin to ensure that dnaRemainingValues == 0 if
nextvalue > maxvalue, do we need to check explicitly?
[...]
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index
4a46532642013204720ba467966c59de31a92301..cb9a7e98fd0c486abe5b8b92aff711fa69f23fa9
100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1775,6 +1775,8 @@ class IPAdmin(LDAPClient):
if removes:
if not force_replace:
modlist.append((ldap.MOD_DELETE, key, removes))
+ elif new_values == []: # delete an empty value
+ modlist.append((ldap.MOD_DELETE, key, removes))
I don't understand this change. AFAIK updateEntry/generateModList is
only used in ldapupdater now, and it's going away as soon as I can find
time to remove it. If you need to change it I'd like to know why.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
