[Freeipa-devel] [PATCH] 281 Enable SOA serial autoincrement

[Jan Cholasta]

Dne 12.7.2012 16:25, Martin Kosek napsal(a):
> On 07/02/2012 08:45 AM, Martin Kosek wrote:
>> On 06/29/2012 09:00 PM, Rob Crittenden wrote:
>>> Martin Kosek wrote:
>>>> This patch enables currently developed SOA serial autoincrement feature in
>>>> bind-dyndb-ldap. The patch may be updated if any assumptions about this feature
>>>> are changed (or somebody finds a bug).
>>>>
>>>> ---
>>>>
>>>> SOA serial autoincrement is a requirement for major DNS features,
>>>> e.g. zone transfers or DNSSEC. Enable it by default in named.conf
>>>> both for new and upgraded installations. Name of the bind-dyndb-ldap
>>>> option is "serial_autoincrement".
>>>>
>>>>>  From now on, idnsSOAserial attribute also has to be put to
>>>> replication agreement exclude list as serial will be incremented
>>>> on each DNS server separately and won't be shared. Exclude list
>>>> has to be updated both for new replication agreements and the
>>>> current ones.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/2554
>>>
>>> What version of bind/bind-dyndb-ldap is needed for serial_autoincrement?
>>>
>>> rob
>>
>> Such version is not ready yet, there is only a semi-working patch from Petr
>> Spacek on freeipa-devel list.
>>
>> When a working version of bind-dyndb-ldap package with working
>> serial_autoincrement feature, it should be enough to simply bump package
>> version in bind-dyndb-ldap (that's why I tagged this patch as [WIP]).
>>
>> But otherwise, this patch is reviewable, it should prepare our install tools
>> for the new feature, turn it on in named.conf on upgrades and also update
>> replication agreements to not replicate SOA serial from now on.
>>
>> Martin
>
> Sending a rebased and updated patch with few more fixes:
> 1) Minimum number of connections has been rised to 4 to cover the most recent
> requirements for bind-dyndb-ldap's serial_automember feature
> 2) ipa-upgradeconfig named.conf has been fixed to not crash when the updated
> options are not in the file
>
> I think that we can choose to push this patch earlier before bind-dyndb-ldap
> with serial_automember released. We just need to make sure this patch sets
> serial_automember option in named.conf correctly + does the right thing with
> replication agreement exclude list update.
>
> Later on, we would just need to bump bind-dyndb-ldap version in our spec file
> when that's released.
>
> Martin
>

ACK.

I have a couple of nitpicks though:

1) There's a stray ">" in the commit message:

">From now on, idnsSOAserial attribute ..."

This is probably caused by the mailing list software. Just make sure you 
don't include it in the actual commit.

2) There's extra comma in ipa-server-install:

-               persistent_search=options.persistent_search)
+               persistent_search=options.persistent_search,
+               serial_autoincrement=options.serial_autoincrement,)  <----

3) In ipa-upgradeconfig:

+    else:
+        psearch = psearch.lower() if psearch is not None else None

IMO it would be nicer to do:

+    elif psearch is not None:
+        psearch = psearch.lower()

or:

+    else:
+        psearch = psearch and psearch.lower()

instead.


Honza

-- 
Jan Cholasta