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

[Martin Kosek]

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