[Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
David Kupka
dkupka at redhat.com
Thu Dec 10 16:31:20 UTC 2015
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.
--
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0072.0-dns-Add-auto-reverse-option.patch
Type: text/x-patch
Size: 5177 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151210/34c0f6ea/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0058.10-dns-do-not-add-forward-zone-if-it-is-already-resolva.patch
Type: text/x-patch
Size: 12748 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151210/34c0f6ea/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0064.7-dns-Check-if-domain-already-exists.patch
Type: text/x-patch
Size: 17048 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151210/34c0f6ea/attachment-0002.bin>
More information about the Freeipa-devel
mailing list