[Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer & password migration

Wed Mar 13 11:48:08 UTC 2013

On 03/12/2013 03:34 PM, Petr Viktorin wrote:
> On 03/12/2013 01:37 PM, Martin Kosek wrote:
>> 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
>>
> 
> Fixed, thanks.
> 

This looks OK, thanks.

ACK, pushed to master.

Martin