[Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

[Martin Basti]

On 02/09/14 16:55, David Kupka wrote:
> The patch now depends on freeipa-dkupka-0012 as both modifies the same 
> part of code.
>
> On 09/02/2014 10:29 AM, David Kupka wrote:
>> Forget to add str() conversion to some places when removing map(). Now
>> it should be working again.
>>
>> On 08/27/2014 02:24 PM, David Kupka wrote:
>>> Patch modified according to jcholast's personally-delivered feedback:
>>>
>>>  > 1) use action='append' instead of that ugly parsing
>>>
>>>  > 2) do not use map(), FreeIPA doesn't like it
>>>
>>> On 08/25/2014 05:04 PM, David Kupka wrote:
>>>> https://fedorahosted.org/freeipa/ticket/3575
>>>>
>>>> Also should fix https://bugzilla.redhat.com/show_bug.cgi?id=1128380 as
>>>> installation is no longer interrupted when multiple IPs are resolved.
>>>> But it does not add the option to change the IP address during second
>>>> run.
>>>>
>>>>
I haven't tested it yet, I only take a look because there may be 
conflict with 'dns root zone support' refactoring

1)
+        for ns_ip_address in nameserver_ip_address:
+            add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup,
+                    ns_hostname=api.env.host, ns_ip_address=ns_ip_address,
+                    force=True)
Are you sure this will work? Domain name is the same, so no new zone 
will be created (DuplicateEntry exception is handled inside add_zone 
function).
IMO you should call add_zone only once.

BTW: I will change the add_zone function in refactoring , ns_hostname 
wil be remove, and ns_ip_address will take an p+ipv6 address

2)
+        resolv_txt = ''
+        for ip_address in self.ip_address:
+            resolv_txt += "search "+self.domain+"\nnameserver 
"+str(ip_address)+"\n"
There is multiple search statements.

search example.com
nameserver 192.168.1.1
search example.com
nameserver 2001:db8::1
...

and also there si a limit of namesevers which can be in resolv.conf, but 
I dont know if we care,  statements over limit should be just ignored.
http://linux.die.net/man/5/resolv.conf

3)
self.ip_address is confusing for me, I'm expecting only one address.
Could it be ip_addresses or ip_address_list? Ask the framework gurus :-)

-- 
Martin Basti