Guys, could you take a look at this one?

On 01/27/2016 11:16 AM, Oleg Fayans wrote:
> Sorry, trailing whitespace detected. This version passes lint
> 
> On 01/27/2016 09:23 AM, Oleg Fayans wrote:
>> Hi,
>>
>> On 01/21/2016 04:41 PM, Petr Spacek wrote:
>>> Hello,
>>>
>>> On 21.1.2016 13:42, Oleg Fayans wrote:
>>>> freeipa-ofayans-0021-Removed-ip-address-option-from-replica-installation.patch
>>>>
>>>>
>>>> From d7ab06a4dcddb919fda351b983d478f1b6968578 Mon Sep 17 00:00:00 2001
>>>> From: Oleg Fayans <ofay...@redhat.com>
>>>> Date: Thu, 21 Jan 2016 13:30:02 +0100
>>>> Subject: [PATCH] Removed --ip-address option from replica installation
>>>>
>>>> Explicitly specifying ip-address of the replica messes up with the current
>>>> bind-dyndb-ldap logic, causing reverse zone not to be created.
>>>>
>>>> Enabled reverse-zone creation for the clients residing in different subnet 
>>>> from
>>>> master
>>>> ---
>>>>  ipatests/test_integration/tasks.py | 19 ++++++++++++-------
>>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/ipatests/test_integration/tasks.py 
>>>> b/ipatests/test_integration/tasks.py
>>>> index 
>>>> 6eb55501389c72b4c7aaa599fd4852d7e8f1f3c2..43ef78b0c55deed24a0444f0ac6c38ddb2517481
>>>>  100644
>>>> --- a/ipatests/test_integration/tasks.py
>>>> +++ b/ipatests/test_integration/tasks.py
>>>> @@ -69,6 +69,8 @@ def prepare_reverse_zone(host, ip):
>>>>      host.run_command(["ipa",
>>>>                        "dnszone-add",
>>>>                        zone], raiseonerr=False)
>>>> +    return zone
>>>> +
>>>>  
>>>>  def prepare_host(host):
>>>>      if isinstance(host, Host):
>>>> @@ -319,11 +321,8 @@ def domainlevel(host):
>>>>  def replica_prepare(master, replica):
>>>>      apply_common_fixes(replica)
>>>>      fix_apache_semaphores(replica)
>>>> -    prepare_reverse_zone(master, replica.ip)
>>>> -    master.run_command(['ipa-replica-prepare',
>>>> -                        '-p', replica.config.dirman_password,
>>>> -                        '--ip-address', replica.ip,
>>>> -                        replica.hostname])
>>>> +    master.run_command(['ipa-replica-prepare', '-p', 
>>>> replica.config.dirman_password,
>>>> +                        '--auto-reverse', replica.hostname])
>>>
>>> I guess that you will need --ip-address option in cases where master's 
>>> reverse
>>> record does not exist (yet).
>> And yo were right. Fixed
>>
>>>
>>> I would recommend you to test this in libvirt or somewhere without revere
>>> records, I suspect that it might blow up.
>>>
>>>>      replica_bundle = master.get_file_contents(
>>>>          paths.REPLICA_INFO_GPG_TEMPLATE % replica.hostname)
>>>>      replica_filename = get_replica_filename(replica)
>>>> @@ -339,8 +338,7 @@ def install_replica(master, replica, setup_ca=True, 
>>>> setup_dns=False,
>>>>      # and replica installation would fail
>>>>      args = ['ipa-replica-install', '-U',
>>>>              '-p', replica.config.dirman_password,
>>>> -            '-w', replica.config.admin_password,
>>>> -            '--ip-address', replica.ip]
>>>> +            '-w', replica.config.admin_password]
>>>>      if setup_ca:
>>>>          args.append('--setup-ca')
>>>>      if setup_dns:
>>>> @@ -380,6 +378,13 @@ def install_client(master, client, extra_args=()):
>>>>      client.collect_log(paths.IPACLIENT_INSTALL_LOG)
>>>>  
>>>>      apply_common_fixes(client)
>>>> +    # Now, for the situations where a client resides in a different 
>>>> subnet from
>>>> +    # master, we need to explicitly tell master to create a reverse zone 
>>>> for
>>>> +    # the client and enable dynamic updates for this zone.
>>>> +    allow_sync_ptr(master)
>>>> +    zone = prepare_reverse_zone(master, client.ip)
>>>> +    master.run_command(["ipa", "dnszone-mod", zone,
>>>> +                        "--dynamic-update=TRUE"], raiseonerr=False)
>>>
>>> I'm not a big fan of ignoring exceptions here, it might be better to
>>> encapsulate the first command with try: except: and run the zone-mod only if
>>> the add worked as expected.
>>>
>>> Also, logging an message that reverse zone was not added might be a good 
>>> idea.
>> Agreed. Done.
>>
>>>
>>> HTH
>>>
>>> Petr^2 Spacek
>>>
>>>
>>>>  
>>>>      client.run_command(['ipa-client-install', '-U',
>>>>                          '--domain', client.domain.name,
>>>
>>
>>
>>
> 
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to