[Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
Petr Spacek
pspacek at redhat.com
Mon Dec 14 15:54:35 UTC 2015
On 14.12.2015 16:31, David Kupka wrote:
> On 14/12/15 15:25, David Kupka wrote:
>> On 14/12/15 14:52, David Kupka wrote:
>>> On 11/12/15 15:00, Petr Spacek wrote:
>>>> 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!
>>>>
>>> Updated patches attached.
>>>
>> As always, FreeIPA's code is really live before release. Rebased patches
>> attached.
>>
> Updated patches attached.
ACK! Push it NOW! :-D
--
Petr^2 Spacek
More information about the Freeipa-devel
mailing list