[Freeipa-devel] [TEST][Patch 0021] Fixed recent replica installation issues in the lab
Petr Spacek
pspacek at redhat.com
Fri Jan 29 10:58:03 UTC 2016
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,
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.
>
> client.run_command(['ipa-client-install', '-U',
> '--domain', client.domain.name,
> -- 2.4.3
--
Petr^2 Spacek
More information about the Freeipa-devel
mailing list