[Freeipa-devel] [TEST][Patch 0021] Fixed recent replica installation issues in the lab
Oleg Fayans
ofayans at redhat.com
Mon Feb 1 10:52:00 UTC 2016
Hi Petr,
Please find the new version of the patch attached. Comments are inline
On 01/29/2016 11:58 AM, Petr Spacek wrote:
> On 27.1.2016 11:16, 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 <ofayans at 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.
>>
>>
>> freeipa-ofayans-0021.2-Removed-ip-address-option-from-replica-installation.patch
>>
>>
>> From e70daf4ed9dfbac7e8ea75cb8e9ab0f2af12ad48 Mon Sep 17 00:00:00 2001
>> From: Oleg Fayans <ofayans at redhat.com>
>> Date: Wed, 27 Jan 2016 11:09:03 +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.
>
> NACK
>
> The commit message and logic is incorrect. In fact it has nothing to do with
> bind-dyndb-ldap. Either:
> a] The zone already exists somewhere so you should not create it in IPA, and
> this --ip-address option is invalid.
> b] The zone does not exist yet and then --ip-address option is required.
>
> I do not see any logic around
>> '--ip-address', replica.ip,
The explicit ip-address passing was removed (that's the main point of
this patch)
> thus the NACK.
>
>
> In other words, network setup is job for a provisioning system. IPA needs to
> respect whatever the provisioning system did or did not do.
>
> Feel free to ask if something is unclear.
>
> (other comments inline)
>
>
>>
>> Enabled reverse-zone creation for the clients residing in different subnet from
>> master
>> ---
>> ipatests/test_integration/tasks.py | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
>> index 6eb55501389c72b4c7aaa599fd4852d7e8f1f3c2..b8b7fcfcbbb1dc672349119bbbb4cdbbd68c54ec 100644
>> --- a/ipatests/test_integration/tasks.py
>> +++ b/ipatests/test_integration/tasks.py
>> @@ -66,9 +66,13 @@ def check_arguments_are(slice, instanceof):
>>
>> def prepare_reverse_zone(host, ip):
>> zone = get_reverse_zone_default(ip)
>> - host.run_command(["ipa",
>> + result = host.run_command(["ipa",
>> "dnszone-add",
>> zone], raiseonerr=False)
>> + if result.returncode > 0:
>> + log.warn(result.stderr_text)
>> + return zone, result.returncode
>> +
>>
>> def prepare_host(host):
>> if isinstance(host, Host):
>> @@ -319,11 +323,10 @@ 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])
>> + '--auto-reverse', replica.hostname])
>> replica_bundle = master.get_file_contents(
>> paths.REPLICA_INFO_GPG_TEMPLATE % replica.hostname)
>> replica_filename = get_replica_filename(replica)
>> @@ -339,8 +342,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 +382,14 @@ 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, error = prepare_reverse_zone(master, client.ip)
>> + if not error:
>> + master.run_command(["ipa", "dnszone-mod", zone,
>> + "--dynamic-update=TRUE"], raiseonerr=False)
>
> I believe that this call to dnszone-mod should use raiseonerr=True so you know
> that environment was not configured to accept dynamic updates.
Agreed. Fixed.
>
>>
>> client.run_command(['ipa-client-install', '-U',
>> '--domain', client.domain.name,
>> -- 2.4.3
>
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ofayans-0021.3-Removed-ip-address-option-from-replica-installation.patch
Type: text/x-patch
Size: 2440 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160201/6e1cebe2/attachment.bin>
More information about the Freeipa-devel
mailing list