On 03/12/2013 10:10 AM, Petr Viktorin wrote:
> On 03/11/2013 02:56 PM, Martin Kosek wrote:
>> On 03/11/2013 01:48 PM, Jan Cholasta wrote:
>>> On 11.3.2013 13:43, Petr Viktorin wrote:
>>>> On 03/11/2013 01:13 PM, Jan Cholasta wrote:
>>>>> On 8.3.2013 14:14, Petr Viktorin wrote:
>>>>>> On 03/07/2013 05:42 PM, Jan Cholasta wrote:
>>>>>>> Patch 191:
>>>>>>>
>>>>>>> The patch is missing the ipapython/ipaldap.py file.
>>>>>
>>>>> On 7.3.2013 18:29, Petr Viktorin wrote:
>>>>>  > It's there, it's just copied from ipaserver/ipaldap.py with a small
>>>>>  > change at the bottom.
>>>>>
>>>>> There is no sign of the file, except in the patch header and the patch
>>>>> cannot be applied with git am nor with git apply. But perhaps I'm doing
>>>>> something wrong.
>>>>
>>>> Attaching a re-formatted version of the patch.
>>>>
>>>> [...]
>>>>> ACK.
>>>>>
>>>>> Honza
>>>>>
>>>>
>>>>
>>>
>>> ACK for real.
>>>
>>> Honza
>>>
>>
>> I would not want to rush this, I still see errors:
>>
>> 1) ipa-ldap-updater is broken:
>>
>> # ipa-ldap-updater --upgrade
>> Upgrading IPA:
>>    [1/8]: stopping directory server
>>    [2/8]: saving configuration
>>    [3/8]: disabling listeners
>>    [4/8]: starting directory server
>>    [5/8]: upgrading server
>> Upgrade failed with 'NameSpace' object has no attribute 'ldap2'
>>    [6/8]: stopping directory server
>>    [7/8]: restoring configuration
>>    [8/8]: starting directory server
>> Done.
>> IPA upgrade failed.
> 
> Thanks for the catch!
> 
> This is a symptom of the fact the plugins attach themselves to the default API
> object as soon as they're imported.
> Before, ipaldap imported ldap2, so the ldap2 server plugin was magically
> available whenever ipaldap was imported before.
> Now, ldap2 needs to be imported explicitly if api.Backend.ldap2 needs to be
> available.
> 
>> 2) What's the purpose of this new error?
>>
>> +class DatabaseTimeout(DatabaseError):
>> +    """
>> +    **4211** Raised when an LDAP call times out
>> +
>> +    For example:
>> +
>> +    >>> raise DatabaseTimeout()
>> +    Traceback (most recent call last):
>> +      ...
>> +    DatabaseTimeout: LDAP timeout
>> +    """
>> +
>> +    errno = 4211
>> +    format = _('LDAP timeout')
> 
> Thanks for this catch too, I mis-squashed the code to raise it.
> 
>> It is not raised anywhere (as far as I can see). BTW I assume it is not
>> related to errors.LimitsExceeded in any way, right?
> 
> No, it's timeout in the client↔server communication rather than the LDAP
> operation. It wraps ldap.TIMEOUT rather than ldap.TIMELIMIT_EXCEEDED.
> 
>> 3) Client installation no longer works if the server has disabled
>> anonymous authentication:
>>
>> # ipa-client-install
>> Error checking LDAP: Inappropriate authentication: Anonymous access is
>> not allowed.
>> DNS discovery failed to determine your DNS domain
>> Provide the domain name of your IPA server (ex: example.com): ^C
> 
> I couldn't reproduce this. But I did find some misleading log messages in this
> case. It work well now.
> 
>> 4) I suddenly cannot run some tests, looks like import loop:
>>
>> # ./make-test tests/test_xmlrpc/test_host_plugin.py
>> /usr/bin/nosetests -v --with-doctest --doctest-tests --exclude=plugins
>> tests/test_xmlrpc/test_host_plugin.py
>> Failure: ImportError (cannot import name ipautil) ... ERROR
>>
>> ======================================================================
>> ERROR: Failure: ImportError (cannot import name ipautil)
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>    File "/usr/lib/python2.7/site-packages/nose/loader.py", line 390, in
>> loadTestsFromName
>>      addr.filename, addr.module)
>>    File "/usr/lib/python2.7/site-packages/nose/importer.py", line 39, in
>> importFromPath
>>      return self.importFromDir(dir_path, fqname)
>>    File "/usr/lib/python2.7/site-packages/nose/importer.py", line 86, in
>> importFromDir
>>      mod = load_module(part_fqname, fh, filename, desc)
>>    File "/root/freeipa-master/tests/test_xmlrpc/test_host_plugin.py",
>> line 27, in <module>
>>      from ipapython import ipautil
>>    File "/root/freeipa-master/ipapython/ipautil.py", line 52, in <module>
>>      from ipalib import errors
>>    File "/root/freeipa-master/ipalib/__init__.py", line 930, in <module>
>>      api.finalize()
>>    File "/root/freeipa-master/ipalib/plugable.py", line 674, in finalize
>>      self.__do_if_not_done('load_plugins')
>>    File "/root/freeipa-master/ipalib/plugable.py", line 454, in
>> __do_if_not_done
>>      getattr(self, name)()
>>    File "/root/freeipa-master/ipalib/plugable.py", line 613, in
>> load_plugins
>>      self.import_plugins('ipalib')
>>    File "/root/freeipa-master/ipalib/plugable.py", line 655, in
>> import_plugins
>>      __import__(fullname)
>>    File "/root/freeipa-master/ipalib/plugins/cert.py", line 30, in <module>
>>      from ipalib import pkcs10
>>    File "/root/freeipa-master/ipalib/pkcs10.py", line 24, in <module>
>>      from ipapython import ipautil
>> ImportError: cannot import name ipautil
> 
> Gasp... I have no idea how we didn't catch this earlier.
> Simplifying a bit, it's partly due to the fact that ipalib does a lot of work
> on import in __init__ -- including loading plugins that assume ipalib's 
> already
> set up.
> 
> I've deferred the import, and added a FIXME.
> 
> 
> Thank you for retesting!
> Updated patches attached.
> 

I tested our basic scenarios and everything seems to work fine, so I think we
can push this soon if no one objects. I just hit two more places in the patch
set which look suspicious:

1) In 193.3, one more unexpected raise:

     except Exception, e:
-        root_logger.debug("get_ca_cert_from_ldap() error: %s",
-                          convert_ldap_error(e))
+        raise
+        root_logger.debug("get_ca_cert_from_ldap() error: %s", e)


2) In 194.3, redundant section:

+                try:
+                    self.__wait_for_connection(timeout)
+                except:
+                    raise

Martin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to