[Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
Petr Spacek
pspacek at redhat.com
Fri Dec 11 14:00:31 UTC 2015
On 11.12.2015 12:35, David Kupka wrote:
> On 10/12/15 18:10, Petr Spacek wrote:
>> On 10.12.2015 17:31, David Kupka wrote:
>>> On 09/12/15 18:55, Petr Spacek wrote:
>>>> On 9.12.2015 13:37, David Kupka wrote:
>>>>> On 08/12/15 15:24, Petr Spacek wrote:
>>>>>> On 8.12.2015 12:19, David Kupka wrote:
>>>>>>> On 08/12/15 08:56, Petr Spacek wrote:
>>>>>>>> On 7.12.2015 14:41, David Kupka wrote:
>>>>>>>>> +def is_host_resolvable(fqdn):
>>>>>>>>> + if not isinstance(fqdn, DNSName):
>>>>>>>>> + fqdn = DNSName(fqdn)
>>>>>>>>> + for rdtype in (rdatatype.A, rdatatype.AAAA):
>>>>>>>>> + try:
>>>>>>>>> + resolver.query(fqdn.make_absolute(), rdtype)
>>>>>>>>> + except DNSException:
>>>>>>>>> + continue
>>>>>>>>> + else:
>>>>>>>>> + return True
>>>>>>>>> +
>>>>>>>>> + return False
>>>>>>>>>
>>>>>>>>
>>>>>>>> NACK, you are re-introducing duplicate function which was removed in
>>>>>>>> 498471e4aed1367b72cd74d15811d0584a6ee268.
>>>>>>>>
>>>>>>>> Please amend the patch ASAP to use new verify_host_resolvable() function
>>>>>>>> so I
>>>>>>>> can test it and get it into 4.3.
>>>>>>>>
>>>>>>> Updated patches attached.
>>>>>>
>>>>>> Hmm, my review script screams:
>>>>>>
>>>>>> Detected base branch: origin/master
>>>>>> Checks will be made against following base commit: 848912a add missing
>>>>>> /ipaplatform/constants.py to .gitignore
>>>>>> Writing API to API.txt
>>>>>> ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual
>>>>>> indentation
>>>>>> ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for
>>>>>> visual indent
>>>>>> ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual
>>>>>> indentation
>>>>>> ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for
>>>>>> visual indent
>>>>>> ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1
>>>>>> ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for
>>>>>> visual indent
>>>>>> ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1
>>>>>> ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters)
>>>>>> ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters)
>>>>>> ./ipaserver/install/bindinstance.py:49:9: E128 continuation line
>>>>>> under-indented for visual indent
>>>>>> ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79
>>>>>> characters)
>>>>>> ./ipaserver/install/bindinstance.py:438:19: E128 continuation line
>>>>>> under-indented for visual indent
>>>>>> ./ipaserver/install/server/common.py:271:63: E261 at least two spaces
>>>>>> before
>>>>>> inline comment
>>>>>> ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79
>>>>>> characters)
>>>>>> ERROR: PEP8 --diff failed, continuing ...
>>>>>> ERROR: API.txt was changed without a change in VERSION, continuing ...
>>>>>> Summary of detected errors:
>>>>>> ERROR: PEP8 --diff failed
>>>>>> ERROR: API.txt was changed without a change in VERSION
>>>>>>
>>>>>> Please fix it :-)
>>>>>>
>>>>>> Make make this more pleasant for you, you can use (and review) my attached
>>>>>> patch. It adds 'review' target to Makefile, it will do the same checks as
>>>>>> I do.
>>>>>>
>>>>>
>>>>> Unfortunately your tool does not work for me (output w/ -o xtrace attached).
>>>>> Anyway I have incremented API version and fixed most of the pep8 errors
>>>>> except
>>>>> for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the
>>>>> file and E501 twice in ipapython/ipautil.py because IMO breaking string
>>>>> constant into multiple lines does not help readability.
>>>>>
>>>>> Updated patches also attached.
>>>>
>>>> We are almost there, but NACK for now.
>>>>
>>>> 1) Following attempt to install IPA explodes. Please note that
>>>> dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server
>>>> before installation is started, so it gives 'timeout' or possibly SERVFAIL.
>>>>
>>>> # ipa-server-install --ds-password=root4lab --admin-password=root4lab
>>>> --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap
>>>> --domain=dom-245.idm.lab.eng.brq.redhat.com
>>>> --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM
>>>> [...]
>>>>
>>>> Configuring DNS (named)
>>>> [1/11]: generating rndc key file
>>>> [2/11]: adding DNS container
>>>> [3/11]: setting up our zone
>>>> [error] InvocationError: DNS check for domain
>>>> dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
>>>> 30.000453949 seconds. Please make sure that the domain is properly delegated
>>>> to this IPA server.
>>>> ipa.ipapython.install.cli.install_tool(Server): ERROR DNS check for domain
>>>> dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
>>>> 30.000453949 seconds. Please make sure that the domain is properly delegated
>>>> to this IPA server.
>>>> ipa.ipapython.install.cli.install_tool(Server): ERROR The
>>>> ipa-server-install command failed. See /var/log/ipaserver-install.log for
>>>> more
>>>> information
>>>>
>>>> 2015-12-09T17:15:37Z DEBUG File
>>>> "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in
>>>> execute
>>>> return_value = self.run()
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line
>>>> 318,
>>>> in run
>>>> cfgr.run()
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 310,
>>>> in run
>>>> self.execute()
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 332,
>>>> in execute
>>>> for nothing in self._executor():
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 372,
>>>> in __runner
>>>> self._handle_exception(exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 394,
>>>> in _handle_exception
>>>> six.reraise(*exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 362,
>>>> in __runner
>>>> step()
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 359,
>>>> in <lambda>
>>>> step = lambda: next(self.__gen)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
>>>> line 81,
>>>> in run_generator_with_yield_from
>>>> six.reraise(*exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
>>>> line 59,
>>>> in run_generator_with_yield_from
>>>> value = gen.send(prev_value)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 571,
>>>> in _configure
>>>> next(executor)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 372,
>>>> in __runner
>>>> self._handle_exception(exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 449,
>>>> in _handle_exception
>>>> self.__parent._handle_exception(exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 394,
>>>> in _handle_exception
>>>> six.reraise(*exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 446,
>>>> in _handle_exception
>>>> super(ComponentBase, self)._handle_exception(exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 394,
>>>> in _handle_exception
>>>> six.reraise(*exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 362,
>>>> in __runner
>>>> step()
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
>>>> line 359,
>>>> in <lambda>
>>>> step = lambda: next(self.__gen)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
>>>> line 81,
>>>> in run_generator_with_yield_from
>>>> six.reraise(*exc_info)
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
>>>> line 59,
>>>> in run_generator_with_yield_from
>>>> value = gen.send(prev_value)
>>>>
>>>> File "/usr/lib/python2.7/site-packages/ipapython/install/common.py", line
>>>> 63, in _install
>>>> for nothing in self._installer(self.parent):
>>>> File
>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
>>>> line 1471, in main
>>>> install(self)
>>>> File
>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
>>>> line 265, in decorated
>>>> func(installer)
>>>> File
>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
>>>> line 958, in install
>>>> dns.install(False, False, options)
>>>> File "/usr/lib/python2.7/site-packages/ipaserver/install/dns.py", line
>>>> 322,
>>>> in install
>>>> bind.create_instance()
>>>> File
>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
>>>> line 680, in create_instance
>>>> self.start_creation()
>>>> File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
>>>> line
>>>> 447, in start_creation
>>>> run_step(full_msg, method)
>>>> File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
>>>> line
>>>> 437, in run_step
>>>> method()
>>>> File
>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
>>>> line 805, in __setup_zone
>>>> ns_hostname=self.api.env.host, force=True, api=self.api)
>>>> File
>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
>>>> line 331, in add_zone
>>>> force=force)
>>>> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in
>>>> __call__
>>>> ret = self.run(*args, **options)
>>>> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 764,
>>>> in run
>>>> return self.execute(*args, **options)
>>>> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
>>>> 2781, in
>>>> execute
>>>> result = super(dnszone_add, self).execute(*keys, **options)
>>>> File "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line
>>>> 1233, in execute
>>>> *keys, **options)
>>>> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
>>>> 2747, in
>>>> pre_callback
>>>> ldap, dn, entry_attrs, attrs_list, *keys, **options)
>>>> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
>>>> 2153, in
>>>> pre_callback
>>>> raise errors.InvocationError(e.message)
>>>>
>>>>
>>>> 2) Please print 'Checking DNS domain <xyz>, please wait ...' when validating
>>>> domain parameter in installer. The timeout is 30 seconds and users may be
>>>> nervous when the installed blocks for 30 seconds without a visible reason.
>>>>
>>>> Precedent for this is in check_forwarders() in
>>>> ipaserver/install/bindinstance.py . Encapsulating check_zone_overlap() in
>>>> auxiliary method may be an option.
>>>>
>>>>
>>>> 3) Timeout is a fatal error instead of warning. We have been discussing this
>>>> and it should really be just a warning.
>>>>
>>>>
>>>> 4) Nitpicks are attached in .patch file.
>>>>
>>>>
>>>> 5) ipa dnszone-add checks work as expected, good job!
>>>>
>>>>
>>>> Thank you for patience!
>>>>
>>> Updated patches attached. Added patch introducing --auto-reverse option that
>>> should generate needed reverse zones automatically.
>>>
>> NACK:
>>
>> $ ipa-server-install --setup-dns
>>
>> Do you want to configure the reverse zone? [yes]:
>> ipa.ipapython.install.cli.install_tool(Server): ERROR 'CheckedIPAddress'
>> object has no attribute 'split'
>>
> Updated patches attached.
>
NACK
1) It breaks replica installation if replica is second+ DNS server.
# ipa-replica-install --setup-dns
Password for admin at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM:
Checking DNS domain abc.idm.lab.eng.brq.redhat.com., please wait ...
Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.
ipa.ipapython.install.cli.install_tool(Replica): ERROR DNS zone
abc.idm.lab.eng.brq.redhat.com. already exists in DNS and is handled by
server(s): vm-058-155.abc.idm.lab.eng.brq.redhat.com.
Maybe the validator should have condition like
"if not replica or not dns_is_configured()" or so ...
2) Check for automatic empty zones does not work:
# ipa dnsforwardzone-add 10.in-addr.arpa. --forwarder=10.34.78.1
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: DNS zone 10.in-addr.arpa. already exists in DNS and is handled by
server(s): 10.IN-ADDR.ARPA.
Here you have to compare names as actual DNS names and not as strings.
Have a nice weekend!
--
Petr^2 Spacek
More information about the Freeipa-devel
mailing list