[Freeipa-devel] [TEST][Patch 0021] Fixed recent replica installation issues in the lab
Oleg Fayans
ofayans at redhat.com
Thu Feb 4 14:38:15 UTC 2016
Hi,
On 02/04/2016 02:07 PM, Martin Basti wrote:
>
>
> On 04.02.2016 13:49, Oleg Fayans wrote:
>> 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
>>>>>
>>>
>>
>>
>
>> + log.warn(result.stderr_text)
>
> warn is deprecated, please use log.warning instead
Fixed
>
> Martin^2
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ofayans-0021.5-Removed-ip-address-option-from-replica-installation.patch
Type: text/x-patch
Size: 4110 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160204/fd63ede2/attachment.bin>
More information about the Freeipa-devel
mailing list