[Freeipa-devel] [TEST][Patch 0021] Fixed recent replica installation issues in the lab
Oleg Fayans
ofayans at redhat.com
Thu Feb 4 12:49:36 UTC 2016
Hi Petr,
An updated patch is attached. Please see my comment inline.
On 02/01/2016 12:47 PM, Petr Spacek wrote:
> On 1.2.2016 11:52, Oleg Fayans wrote:
>> 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)
>
> Sure, but it cannot be done unconditionally (without additional logic).
Fair point. Implemented a check whether ipa master is authoritative for
the client's domain.
>
>>> 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.
>
> Let me rephrase it:
> If IPA DNS manages the zone with replica FQDN or associated reverse zone, we
> need the --ip-address option.
>
> If IPA does not manage any of these, we do not need --ip-address option.
>
>>> Feel free to ask if something is unclear.
>
> Petr^2 Spacek
>
>>> (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.4-Removed-ip-address-option-from-replica-installation.patch
Type: text/x-patch
Size: 4107 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160204/31ac0019/attachment.bin>
More information about the Freeipa-devel
mailing list