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

[Martin Kosek]

On 07/13/2012 03:47 PM, Jan Cholasta wrote:
> 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.

Yeah, made sure it did not get to the commit/push.

> 
> 2) There's extra comma in ipa-server-install:
> 
> -               persistent_search=options.persistent_search)
> +               persistent_search=options.persistent_search,
> +               serial_autoincrement=options.serial_autoincrement,)  <----

Fixed.

> 
> 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.

I did not like the last option very much, it is not so clear from the line what
we are actually doing. So I at least reordered the if-then-else clause so that
it can be be read better.

Pushed to master.

I did not close the ticket though, we can close it when the bind-dyndb-ldap
version in the spec file is bumped.

Martin