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 ad...@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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code