Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
Dne 26.9.2014 v 08:28 David Kupka napsal(a): On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/26/2014 09:34 AM, Jan Cholasta wrote: Dne 26.9.2014 v 08:28 David Kupka napsal(a): On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/26/2014 10:30 AM, David Kupka wrote: On 09/26/2014 09:34 AM, Jan Cholasta wrote: Dne 26.9.2014 v 08:28 David Kupka napsal(a): On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/26/2014 02:47 PM, David Kupka wrote: On 09/26/2014 10:30 AM, David Kupka wrote: On 09/26/2014 09:34 AM, Jan Cholasta wrote: Dne 26.9.2014 v 08:28 David Kupka napsal(a): On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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.,
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 26/09/14 14:47, David Kupka wrote: On 09/26/2014 10:30 AM, David Kupka wrote: On 09/26/2014 09:34 AM, Jan Cholasta wrote: Dne 26.9.2014 v 08:28 David Kupka napsal(a): On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/26/2014 05:50 PM, Martin Basti wrote: On 26/09/14 14:47, David Kupka wrote: On 09/26/2014 10:30 AM, David Kupka wrote: On 09/26/2014 09:34 AM, Jan Cholasta wrote: Dne 26.9.2014 v 08:28 David Kupka napsal(a): On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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.,
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/26/2014 05:50 PM, Martin Basti wrote: On 26/09/14 14:47, David Kupka wrote: On 09/26/2014 10:30 AM, David Kupka wrote: On 09/26/2014 09:34 AM, Jan Cholasta wrote: Dne 26.9.2014 v 08:28 David Kupka napsal(a): On 09/25/2014 04:17 PM, David Kupka wrote: On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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.,
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/24/2014 08:54 PM, Martin Basti wrote: On 24/09/14 15:44, David Kupka wrote: 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.
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 24/09/14 15:44, David Kupka wrote: 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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
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. -- David Kupka From cea7c3e8eb41798d7f2bba916a95a78c034ee052 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 27 Aug 2014 13:50:21 +0200 Subject: [PATCH] Detect and configure all usable IP addresses. Find, verify and configure all IP addresses that can be used to reach the server FreeIPA is being installed on. Ignore some IP address only if user specifies subset of detected addresses using --ip-address option. This change simplyfies FreeIPA installation on multihomed and dual-stacked servers. https://fedorahosted.org/freeipa/ticket/3575 --- install/tools/ipa-replica-install| 48 ++--- install/tools/ipa-server-install | 50 ++ ipaserver/install/bindinstance.py| 114 ++- ipaserver/install/installutils.py| 96 +- ipaserver/install/ipa_replica_prepare.py | 65 ++ 5 files changed, 207 insertions(+), 166 deletions(-) diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index e3b65b0963d92e4d102a1166aa001fe8cb164562..bdc1a86fd339718c91a67b6c32daa69905025e90 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -67,8 +67,8 @@ def parse_options(): default=False, help=configure a dogtag CA) basic_group.add_option(--setup-kra, dest=setup_kra, action=store_true, default=False, help=configure a dogtag KRA) -basic_group.add_option(--ip-address, dest=ip_address, - type=ip, ip_local=True, +basic_group.add_option(--ip-address, dest=ip_addresses, + type=ip, ip_local=True, action=append, default=[], help=Replica server IP Address) basic_group.add_option(-p, --password, dest=password, sensitive=True, help=Directory Manager (existing master) password) @@ -112,7 +112,8 @@ def parse_options(): type=ip, help=Add a DNS forwarder) dns_group.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) -dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use) +dns_group.add_option(--reverse-zone, dest=reverse_zones, default=[], +
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
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. 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 Could be better to ask user to specific zone for ip address a.b.c.d. 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) 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. 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. 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
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/17/2014 07:25 AM, David Kupka wrote: On 09/16/2014 06:09 PM, Martin Basti wrote: On 16/09/14 15:59, David Kupka wrote: On 09/12/2014 07:24 PM, Martin Basti wrote: snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' Stupid mistake, sorry. 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function You are right, it's better to say what's wrong. 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip I indented the else branch more that I wanted. 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. The behavior now basically is: Check if IP address matches any specified zone a. if yes we are good b. else search if there is existing zone that can be used ba. if yes we are good bb. else ask user for zone or create default if --unattended 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones The behavoir now should be almost the same. The only difference is that when installing replica we need to search for existing reverse zones as they could be added during on another server. 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle Rewritten. 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#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) Please remove commented section I forget to clean the trash, thanks. You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips =
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
... 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) 2) Typo? There is no IP address matching reverze_zone %s. -^^ 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) 4) Ask framework gurus, if installutils module is better place for function above -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/16/2014 06:09 PM, Martin Basti wrote: On 16/09/14 15:59, David Kupka wrote: ... 2) +# check that there is IP address in every reverse zone +if options.reverse_zones: +for rz in options.reverse_zones: +for ip in config.ip_addresses: +if bindinstance.verify_reverse_zone(rz, ip): + reverse_zones.append(bindinstance.normalize_zone(rz)) +break +else: +sys.exit(There is no IP address matching reverze_zone %s. % rz) +if not options.no_reverse: +# check that there is reverse zone for every IP +if options.unattended: +for ip in config.ip_addresses: +if bindinstance.find_reverse_zone(str(ip)): +# reverse zone is already in LDAP +continue +for rz in reverse_zones: +if bindinstance.verify_reverse_zone(rz, ip): +# reverse zone is entered by user +break +else: +rz = util.get_reverse_zone_default(str(ip)) +reverse_zones.append(rz) +elif options.reverse_zones or (not(options.no_reverse) and bindinstance.create_reverse()): +for ip in config.ip_addresses: +if bindinstance.find_reverse_zone(str(ip)): +# reverse zone is already in LDAP +continue +for rz in reverse_zones: +if bindinstance.verify_reverse_zone(rz, ip): +# reverse zone is entered by user +break +else: +rz = util.get_reverse_zone_default(str(ip)) +rz = bindinstance.read_reverse_zone(rz, str(ip)) +reverse_zones.append(rz) +else: +options.no_reverse = True +reverse_zones = [] Code above is duplicated in replica-install and server-install, with small difference, could you put it inside function, for example into bindinstance module? Also there are duplicated parts inside in if and elif code block, could you add it to one function as well? +1, I wanted to comment the exactly same idea. Keep in mind that we plan to refactor the installers in FreeIPA 4.2 so we will want the installer to be rather calling shared logic and shared functions instead of duplicating code across bare installers. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 17/09/14 07:25, David Kupka wrote: 3) +elif options.reverse_zones or (not(options.no_reverse) and bindinstance.create_reverse()): OR operator, this will create additional zones (non-specified by user) even if user write NO When user specifies some reverse zone (using --reverse-zone) we can assume that he wants to configure reverse zones (otherwise his is kind of indecisive, at least). So there is no reason to ask him again. The question is asked only when the user didn't provided any reverse zone nor specified --no-reverse. 4) IF user specify zone 10.in-addr.arpa, and ip addresses 10.0.0.1, 192.168.1.1, and answer to not create additional zones, how is this case handled? The question is not asked. See the answer to 3). Sorry my bad. -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/12/2014 07:24 PM, Martin Basti wrote: snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' Stupid mistake, sorry. 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function You are right, it's better to say what's wrong. 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip I indented the else branch more that I wanted. 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. The behavior now basically is: Check if IP address matches any specified zone a. if yes we are good b. else search if there is existing zone that can be used ba. if yes we are good bb. else ask user for zone or create default if --unattended 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones The behavoir now should be almost the same. The only difference is that when installing replica we need to search for existing reverse zones as they could be added during on another server. 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle Rewritten. 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#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) Please remove commented section I forget to clean the trash, thanks. You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips = options.ip_addresses +else: +print sys.stderr, Error: the hostname resolves to an IP address that is
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 16/09/14 15:59, David Kupka wrote: On 09/12/2014 07:24 PM, Martin Basti wrote: snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' Stupid mistake, sorry. 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function You are right, it's better to say what's wrong. 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip I indented the else branch more that I wanted. 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. The behavior now basically is: Check if IP address matches any specified zone a. if yes we are good b. else search if there is existing zone that can be used ba. if yes we are good bb. else ask user for zone or create default if --unattended 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones The behavoir now should be almost the same. The only difference is that when installing replica we need to search for existing reverse zones as they could be added during on another server. 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle Rewritten. 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#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) Please remove commented section I forget to clean the trash, thanks. You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips = options.ip_addresses +else: +print sys.stderr,
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/16/2014 06:09 PM, Martin Basti wrote: On 16/09/14 15:59, David Kupka wrote: On 09/12/2014 07:24 PM, Martin Basti wrote: snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' Stupid mistake, sorry. 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function You are right, it's better to say what's wrong. 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip I indented the else branch more that I wanted. 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. The behavior now basically is: Check if IP address matches any specified zone a. if yes we are good b. else search if there is existing zone that can be used ba. if yes we are good bb. else ask user for zone or create default if --unattended 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones The behavoir now should be almost the same. The only difference is that when installing replica we need to search for existing reverse zones as they could be added during on another server. 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle Rewritten. 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#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) Please remove commented section I forget to clean the trash, thanks. You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips = options.ip_addresses +else: +
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/08/2014 05:56 PM, Martin Basti wrote: 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. freeipa-dkupka-0012 is now accepted and merged upstream so there is no need to take this dependency into account. 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. Fixed, thanks. 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 Since now, only localhost addresses (::1, 127.0.0.1) are placed inside this file when DNS server is part of the installation. 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 :-) Changed to plural as there are other variables named this way. -- David Kupka From 7c5bc742b08403a1a6d878b21dc725a2f71f48da Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 27 Aug 2014 13:50:21 +0200 Subject: [PATCH] Detect and configure all usable IP addresses. Find, verify and configure all IP addresses that can be used to reach the server FreeIPA is being installed on. Ignore some IP address only if user specifies subset of detected addresses using --ip-address option. This change simplyfies FreeIPA installation on multihomed and dual-stacked servers. https://fedorahosted.org/freeipa/ticket/3575 --- install/tools/ipa-replica-install| 69 ++- install/tools/ipa-server-install | 63 - ipaserver/install/bindinstance.py| 74 +++-- ipaserver/install/installutils.py| 94 ipaserver/install/ipa_replica_prepare.py | 66 -- 5 files changed, 207 insertions(+), 159 deletions(-) diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index 7c9e27e2b9d248653681c85236d3541ec9ab0ec7..6ca78a9c60d61d811ab09eb898966fb55bb3daf6 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -67,8 +67,8 @@ def parse_options(): default=False, help=configure a dogtag CA) basic_group.add_option(--setup-kra, dest=setup_kra, action=store_true, default=False, help=configure a dogtag KRA) -basic_group.add_option(--ip-address, dest=ip_address, - type=ip, ip_local=True, +basic_group.add_option(--ip-address, dest=ip_addresses, + type=ip, ip_local=True, action=append, default=[], help=Replica server IP Address) basic_group.add_option(-p, --password, dest=password, sensitive=True, help=Directory Manager (existing master) password) @@ -112,7 +112,8 @@ def parse_options(): type=ip, help=Add a DNS forwarder) dns_group.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) -dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use) +dns_group.add_option(--reverse-zone, dest=reverse_zones, default=[], + action=append, help=The reverse DNS zone to use) dns_group.add_option(--no-reverse, dest=no_reverse,
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#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) Please remove commented section You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips = options.ip_addresses +else: +print sys.stderr, Error: the hostname resolves to an IP address that is different +print sys.stderr, from the one provided on the command line. Please fix your DNS +print sys.stderr, or /etc/hosts file and restart the installation. +sys.exit(1) Could you write those extra addresses to output? We need to improve usability of our error messages -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 08/09/14 17:56, Martin Basti wrote: 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 :-) We might add support for more ip addres for replica install too ipa-replica-prepare replica.example.com --ip-address=192.168.1.2 --ip-address=2001:db8::2 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
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. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- David Kupka From 8eaea5ada941ac813e22efa076b6989d2dbf6be6 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 27 Aug 2014 13:50:21 +0200 Subject: [PATCH] Detect and configure all usable IP addresses. Find, verify and configure all IP addresses that can be used to reach the server FreeIPA is being installed on. Ignore some IP address only if user specifies subset of detected addresses using --ip-address option. This change simplyfies FreeIPA installation on multihomed and dual-stacked servers. https://fedorahosted.org/freeipa/ticket/3575 --- install/tools/ipa-server-install | 43 ipaserver/install/bindinstance.py | 46 +++-- ipaserver/install/installutils.py | 86 +++ 3 files changed, 94 insertions(+), 81 deletions(-) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 6e77b434a018faec36a2808626c99a54bd493908..dde7731e5d991f3329efe8232fcd1bce434e280d 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -176,7 +176,7 @@ def parse_options(): on their first login) basic_group.add_option(--hostname, dest=host_name, help=fully qualified name of server) basic_group.add_option(--ip-address, dest=ip_address, - type=ip, ip_local=True, + type=ip, ip_local=True, action=append, default=[], help=Master Server IP Address) basic_group.add_option(-N, --no-ntp, dest=conf_ntp, action=store_false, help=do not configure ntp, default=True) @@ -236,7 +236,8 @@ def parse_options(): type=ip, help=Add a DNS forwarder) dns_group.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) -dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use) +dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use, + action=append, default=[]) dns_group.add_option(--no-reverse, dest=no_reverse, action=store_true, default=False, help=Do not create reverse DNS zone) dns_group.add_option(--zonemgr, action=callback, callback=bindinstance.zonemgr_callback, @@ -832,11 +833,11 @@ def main(): realm_name = host_name = domain_name = -ip_address = +ip_address = [] master_password = dm_password = admin_password = -reverse_zone = None +reverse_zone = [] if not options.setup_dns and not options.unattended: if ipautil.user_input(Do you want to configure integrated DNS (BIND)?, False): @@ -895,11 +896,14 @@ def main(): domain_name = domain_name.lower() -ip = get_server_ip_address(host_name, fstore, options.unattended, options) -ip_address = str(ip) +ip_address = get_server_ip_address(host_name, fstore, options.unattended, options) -if options.reverse_zone and not bindinstance.verify_reverse_zone(options.reverse_zone, ip): -sys.exit(1) +for ip in ip_address: +for rev_zone in reverse_zone: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) if not options.realm_name: realm_name = read_realm_name(domain_name, options.unattended) @@ -972,16 +976,23 @@ def main(): dns_forwarders = read_dns_forwarders() if options.reverse_zone: -reverse_zone = bindinstance.normalize_zone(options.reverse_zone) +for rz in options.reverse_zone: +reverse_zone.append(bindinstance.normalize_zone(rz)) elif not options.no_reverse: if options.unattended: -reverse_zone = util.get_reverse_zone_default(ip) +for ip in ip_address: +rz =
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
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. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- David Kupka From 8eaea5ada941ac813e22efa076b6989d2dbf6be6 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 27 Aug 2014 13:50:21 +0200 Subject: [PATCH] Detect and configure all usable IP addresses. Find, verify and configure all IP addresses that can be used to reach the server FreeIPA is being installed on. Ignore some IP address only if user specifies subset of detected addresses using --ip-address option. This change simplyfies FreeIPA installation on multihomed and dual-stacked servers. https://fedorahosted.org/freeipa/ticket/3575 --- install/tools/ipa-server-install | 43 ipaserver/install/bindinstance.py | 46 +++-- ipaserver/install/installutils.py | 86 +++ 3 files changed, 94 insertions(+), 81 deletions(-) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 6e77b434a018faec36a2808626c99a54bd493908..dde7731e5d991f3329efe8232fcd1bce434e280d 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -176,7 +176,7 @@ def parse_options(): on their first login) basic_group.add_option(--hostname, dest=host_name, help=fully qualified name of server) basic_group.add_option(--ip-address, dest=ip_address, - type=ip, ip_local=True, + type=ip, ip_local=True, action=append, default=[], help=Master Server IP Address) basic_group.add_option(-N, --no-ntp, dest=conf_ntp, action=store_false, help=do not configure ntp, default=True) @@ -236,7 +236,8 @@ def parse_options(): type=ip, help=Add a DNS forwarder) dns_group.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) -dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use) +dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use, + action=append, default=[]) dns_group.add_option(--no-reverse, dest=no_reverse, action=store_true, default=False, help=Do not create reverse DNS zone) dns_group.add_option(--zonemgr, action=callback, callback=bindinstance.zonemgr_callback, @@ -832,11 +833,11 @@ def main(): realm_name = host_name = domain_name = -ip_address = +ip_address = [] master_password = dm_password = admin_password = -reverse_zone = None +reverse_zone = [] if not options.setup_dns and not options.unattended: if ipautil.user_input(Do you want to configure integrated DNS (BIND)?, False): @@ -895,11 +896,14 @@ def main(): domain_name = domain_name.lower() -ip = get_server_ip_address(host_name, fstore, options.unattended, options) -ip_address = str(ip) +ip_address = get_server_ip_address(host_name, fstore, options.unattended, options) -if options.reverse_zone and not bindinstance.verify_reverse_zone(options.reverse_zone, ip): -sys.exit(1) +for ip in ip_address: +for rev_zone in reverse_zone: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) if not options.realm_name: realm_name = read_realm_name(domain_name, options.unattended) @@ -972,16 +976,23 @@ def main(): dns_forwarders = read_dns_forwarders() if options.reverse_zone: -reverse_zone = bindinstance.normalize_zone(options.reverse_zone) +for rz in options.reverse_zone: +
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
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. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- David Kupka From 13ae855492261afd3e7e8545be2cc06d17431b17 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 27 Aug 2014 13:50:21 +0200 Subject: [PATCH] Detect and configure all usable IP addresses. Find, verify and configure all IP addresses that can be used to reach the server FreeIPA is being installed on. Ignore some IP address only if user specifies subset of detected addresses using --ip-address option. This change simplyfies FreeIPA installation on multihomed and dual-stacked servers. https://fedorahosted.org/freeipa/ticket/3575 --- install/tools/ipa-server-install | 43 ipaserver/install/bindinstance.py | 46 +++-- ipaserver/install/installutils.py | 86 +++ 3 files changed, 94 insertions(+), 81 deletions(-) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 6e77b434a018faec36a2808626c99a54bd493908..dde7731e5d991f3329efe8232fcd1bce434e280d 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -176,7 +176,7 @@ def parse_options(): on their first login) basic_group.add_option(--hostname, dest=host_name, help=fully qualified name of server) basic_group.add_option(--ip-address, dest=ip_address, - type=ip, ip_local=True, + type=ip, ip_local=True, action=append, default=[], help=Master Server IP Address) basic_group.add_option(-N, --no-ntp, dest=conf_ntp, action=store_false, help=do not configure ntp, default=True) @@ -236,7 +236,8 @@ def parse_options(): type=ip, help=Add a DNS forwarder) dns_group.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) -dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use) +dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use, + action=append, default=[]) dns_group.add_option(--no-reverse, dest=no_reverse, action=store_true, default=False, help=Do not create reverse DNS zone) dns_group.add_option(--zonemgr, action=callback, callback=bindinstance.zonemgr_callback, @@ -832,11 +833,11 @@ def main(): realm_name = host_name = domain_name = -ip_address = +ip_address = [] master_password = dm_password = admin_password = -reverse_zone = None +reverse_zone = [] if not options.setup_dns and not options.unattended: if ipautil.user_input(Do you want to configure integrated DNS (BIND)?, False): @@ -895,11 +896,14 @@ def main(): domain_name = domain_name.lower() -ip = get_server_ip_address(host_name, fstore, options.unattended, options) -ip_address = str(ip) +ip_address = get_server_ip_address(host_name, fstore, options.unattended, options) -if options.reverse_zone and not bindinstance.verify_reverse_zone(options.reverse_zone, ip): -sys.exit(1) +for ip in ip_address: +for rev_zone in reverse_zone: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) if not options.realm_name: realm_name = read_realm_name(domain_name, options.unattended) @@ -972,16 +976,23 @@ def main(): dns_forwarders = read_dns_forwarders() if options.reverse_zone: -reverse_zone = bindinstance.normalize_zone(options.reverse_zone) +for rz in options.reverse_zone: +reverse_zone.append(bindinstance.normalize_zone(rz)) elif not options.no_reverse: if options.unattended: -reverse_zone = util.get_reverse_zone_default(ip) +for ip in ip_address: +rz = util.get_reverse_zone_default(str(ip)) +if not rz in reverse_zone: +reverse_zone.append(rz) elif bindinstance.create_reverse(): -reverse_zone = util.get_reverse_zone_default(ip) -reverse_zone = bindinstance.read_reverse_zone(reverse_zone,
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 08/27/2014 02:24 PM, David Kupka wrote: ... 2) do not use map(), FreeIPA doesn't like it Well, it is not that FreeIPA does not like it, it is just more Pythonic to use list comprehension and not functional languages remnants in Python :-) ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On Wed, Aug 27, 2014 at 03:17:37PM +0200, Martin Kosek wrote: On 08/27/2014 02:24 PM, David Kupka wrote: ... 2) do not use map(), FreeIPA doesn't like it Well, it is not that FreeIPA does not like it, it is just more Pythonic to use list comprehension and not functional languages remnants in Python :-) Furthermore, if you go the comprehension route, a generator comprehension would be cheaper than a list comprehension. IMBO, this is the main reason to use a [generator] comprehension over ``map`` - in Python 2 at least. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
