[Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
David Kupka
dkupka at redhat.com
Wed Sep 24 13:44:47 UTC 2014
On 09/23/2014 08:25 PM, Martin Basti wrote:
> On 23/09/14 13:23, David Kupka wrote:
>> On 09/18/2014 06:34 PM, Martin Basti wrote:
>>> ...
>>> 1)
>>> + if options.unattended:
>>> + for ip in ip_addresses:
>>> + if search_reverse_zones and find_reverse_zone(str(ip)):
>>> + # reverse zone is already in LDAP
>>> + continue
>>> + for rz in ret_reverse_zones:
>>> + if verify_reverse_zone(rz, ip):
>>> + # reverse zone was entered by user
>>> + break
>>> + else:
>>> + rz = get_reverse_zone_default(str(ip))
>>> + ret_reverse_zones.append(rz)
>>> + elif options.reverse_zones or create_reverse():
>>> + for ip in ip_addresses:
>>> + if search_reverse_zones and find_reverse_zone(str(ip)):
>>> + # reverse zone is already in LDAP
>>> + continue
>>> + for rz in ret_reverse_zones:
>>> + if verify_reverse_zone(rz, ip):
>>> + # reverse zone was entered by user
>>> + break
>>> + else:
>>> + rz = get_reverse_zone_default(str(ip))
>>> + rz = read_reverse_zone(rz, str(ip))
>>> + ret_reverse_zones.append(rz)
>>> + else:
>>> + options.no_reverse = True
>>> + ret_reverse_zones = []
>>>
>>> You can make it shorter without duplications:
>>>
>>> # this ifs can be in one line
>>> if not options.unatended:
>>> if not options.reverse_zones
>>> if not create_reverse():
>>> options.no_reverse=True
>>> return []
>>>
>>> for ip in ip_addresses:
>>> if search_reverse_zones and find_reverse_zone(str(ip)):
>>> # reverse zone is already in LDAP
>>> continue
>>> for rz in ret_reverse_zones:
>>> if verify_reverse_zone(rz, ip):
>>> # reverse zone was entered by user
>>> break
>>> else:
>>> rz = get_reverse_zone_default(str(ip))
>>> if not options.unattended:
>>> rz = read_reverse_zone(rz, str(ip))
>>> ret_reverse_zones.append(rz)
>>>
>>
>> Thanks, I modified it bit different way to alse address recommendation
>> 3).
>>
>>>
>>> 2)
>>> Typo? There is no IP address matching reverze_zone %s."
>>> ---------------------------------------------^^
>>>
>>
>> Thanks, fixed.
>>
>>>
>>> 3)
>>> Would be nice to ask user to create new zones only if new zones are
>>> required. (If all required zones exist in LDAP, you ask user anyway)
>>>
>>
>> I added one more variable and ask only once.
>>
>>> 4)
>>> Ask framework gurus, if installutils module is better place for function
>>> above
>>>
>>>
>>
>> Petr^3 said that it's ok to have it in bindinstance.py.
>>
>>>
>>
> NACK, most important point is 7
>
> 1)
> I'm not sure if this is bug, but interactively is allowed to add only
> one ip address
>
> Unable to resolve IP address for host name
> Please provide the IP address to be used for this host name: 2001:db8::2
> The kerberos protocol requires a Realm name to be defined.
>
For the sake of infinite usability and UX I rewrote it to ask for
multiple addresses the same way as for DNS forwarders. Also I really
simplified IP address checking code when I was in it. I tested it but
please look at it carefully.
Also I found that ipa-dns-install and ipa-adtrust-install also accept
--ip-address param. So I modified ipa-dns-install in the same way as
ipa-server-install and ipa-replica-install. After discussion with tbabej
I decided to dont touch ipa-adtrust-install now as it do not use
specified value at all. I will remove the processing code and mark the
param as deprecated in separate patch.
> 2)
> I'm getting error message
>
> Invalid reverse zone 10.in-addr.arpa. for IP address 2001:db8::dead:beef
> Invalid reverse zone 10.in-addr.arpa. for IP address
> fed0:babe:baab:0:21a:4aff:fe10:4e18
>
> - or -
>
> Do you want to configure the reverse zone? [yes]:
> Please specify the reverse zone name
> [0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.]:
> Invalid reverse zone 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa. for IP
> address fed0:babe:baab:0:21a:4aff:fe10:4e18
> Please specify the reverse zone name
> [0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.]:
> Using reverse zone(s) 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.,
> 0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.
>
> This shouldn't be there
Moved the message to function used when installation is attended by user.
>
> Could be better to ask user to specific zone for ip address a.b.c.d.
Probably, but lets leave some work for future.
>
> 4) just nitpick
> The IPA Master Server will be configured with:
> ...
> IP address(es): 2001:db8::dead:beef, fed0:babe:baab:0:21a:4aff:fe10:4e18
> ...
> Reverse zone: 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.,
> 0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.
>
> You have label "IP address(es)", so you should use label "Reverse zone(s)"
>
Fixed.
> 5)
> ipa-server-install --ip-address=10.16.78.105
> --reverse-zone=10.in-addr.arpa. --reverse-zone=16.10.in-addr.arpa.
> --setup-dns
>
> Creates both reverse zones, but 10.in-addr.arpa. is empty. I'm not sure
> if this is wrong, but we prevents user to add zone without address in
> it, so we should prevents, to add empty zone.
>
It would be nice but not in this patch.
> 6)
> ipa-replica-prepare --ip-address 10.16.78.105 --ip-address
> 2001:db8::dead:beef --reverse-zone 1.0.0.2.ip6.arpa. vm-105.example.com
> Directory Manager (existing master) password:
>
> Invalid reverse zone 1.0.0.2.ip6.arpa. for IP address 10.16.78.105
> Invalid reverse zone 1.0.0.2.ip6.arpa.
>
> IMO This should work, right?
>
> + sys.exit("There is no IP address matching reverse zone
> %s." % rz)
> I expected at least this error to be shown.
Fixed, thanks.
>
> 7)
> ipa-replica-prepare --ip-address 10.16.78.105 --ip-address
> 2001:db8::dead:beef vm-105.example.com
> Directory Manager (existing master) password:
>
> .......
> Adding DNS records for vm-105.example.com
> Values instance has no attribute 'ip_address'
>
> Command returns the attribute error.
> It fails with one --ip-address too.
>
Sorry, fixed.
>
> *) Not related with your patch
> Problem with installation:
> I'm getting message:
> IPA server is already configured on this system.
> Even if I validation wasn't successful and installation was aborted.
>
> IPA install detects previous installations by checking state file and
> restore files. Function get_server_ip_address() stores some data and
> hosts file and modify the host file, before user agreed installation.
> This error was there before your patch.
> https://fedorahosted.org/freeipa/ticket/4561
>
>
>
--
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-9-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 44224 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140924/fcc81df2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-9-ipa41-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 44204 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140924/fcc81df2/attachment-0001.bin>
More information about the Freeipa-devel
mailing list