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'

-- 
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

Reply via email to