On Tue, 08 Apr 2014 17:31:25 +0200 Petr Viktorin <[email protected]> wrote:
> On 04/08/2014 04:17 PM, Misnyovszki Adam wrote: > > On Mon, 07 Apr 2014 09:43:10 +0200 > > Petr Viktorin <[email protected]> wrote: > > > >> On 03/27/2014 03:37 PM, Misnyovszki Adam wrote: > >>> On Wed, 26 Mar 2014 13:15:55 +0100 > >>> Petr Viktorin <[email protected]> wrote: > >> [...] > >>>> > >>>> Looks great! I'm just concerned about the error returned when the > >>>> task takes too long: > >>>> $ ipa automember-rebuild --type group > >>>> ipa: ERROR: LDAP timeout > >>>> I don't think it's sufficiently clear from this that waiting for > >>>> the task timed out, but the task was actually started > >>>> successfully. A custom error with a more descriptive message > >>>> would be useful. > >>>> > >>>> > >>>> Also I've noticed that the "nstaskstatus" of a successful task > >>>> is: Automember rebuild task finished. Processed (1) entries. > >>>> This looks helpful; we could return it as the summary. > >>>> > >>> > >>> Hi, > >>> both fixed. > >>> Greets > >>> Adam > >>> > >> > >> Sorry for the delay! > >> 'Automember' is a translatable string, so please wrap it in _() > >> when raising TaskTimeout. Also please update the tests. > >> Otherwise with a little rebase it's good to go. > >> > >> > > > > Hi, > > see the attached modifications, tests corrected, and added for > > no-wait, also rebased for current master. > > Greets > > Adam > > > > Looks good overall, but why do you now set `self.msg_summary`? Keep > in mind that currently the same Command object is reused for every > automember_rebuild command, including commands that run in parallel > in different threads. It should never be modified. > Hi, corrected. Greets Adam
>From e437d45d5d3b2aaba486e6359b7334bffb657723 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki <[email protected]> Date: Tue, 25 Mar 2014 14:47:03 +0100 Subject: [PATCH 1/2] automember rebuild nowait feature added automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. this patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished. Old usage can be enabled using --nowait, and returns the DN of the task for further polling. New tests added also. https://fedorahosted.org/freeipa/ticket/4239 --- API.txt | 7 ++- VERSION | 4 +- ipalib/errors.py | 16 ++++++ ipalib/plugins/automember.py | 70 +++++++++++++++++++++----- ipatests/test_xmlrpc/test_automember_plugin.py | 67 ++++++++++++++++++++---- ipatests/test_xmlrpc/xmlrpc_test.py | 10 ++++ 6 files changed, 149 insertions(+), 25 deletions(-) diff --git a/API.txt b/API.txt index 14dde56832793f8dd9fa6795a5ba79d0a2431d51..a0285d49466b887bc9aeb4b4190cc0d99687cf6d 100644 --- a/API.txt +++ b/API.txt @@ -201,12 +201,15 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) output: Output('value', <type 'unicode'>, None) command: automember_rebuild -args: 0,4,3 +args: 0,7,3 +option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Str('hosts*') +option: Flag('no_wait?', autofill=True, default=False) +option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup')) option: Str('users*') option: Str('version?', exclude='webui') -output: Output('result', <type 'bool'>, None) +output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) output: Output('value', <type 'unicode'>, None) command: automember_remove_condition diff --git a/VERSION b/VERSION index 7c6722965bc3b37b71e036ce7f2b2472fd662877..e787e371318b2a817a7d18c1bb1750db9130192e 100644 --- a/VERSION +++ b/VERSION @@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=81 -# Last change: amisnyov - user plugin extend +IPA_API_VERSION_MINOR=82 +# Last change: amisnyov - automember nowait add diff --git a/ipalib/errors.py b/ipalib/errors.py index 311127f62e54017c85541d27276020a9f950ab0f..8ef35f590390eda1e847589d669cd4d28644a6a5 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -1530,6 +1530,22 @@ class DNSDataMismatch(ExecutionError): format = _('DNS check failed: Expected {%(expected)s} got {%(got)s}') +class TaskTimeout(DatabaseError): + """ + **4213** Raised when an LDAP task times out + + For example: + + >>> raise TaskTimeout() + Traceback (most recent call last): + ... + TaskTimeout: Automember LDAP task timeout, Task DN: '' + """ + + errno = 4213 + format = _("%(task)s LDAP task timeout, Task DN: '%(task_dn)s'") + + class CertificateError(ExecutionError): """ **4300** Base class for Certificate execution errors (*4300 - 4399*). diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py index a12bfb52522e38bc083d0750dc66c894a4aeba53..ac6f66a7f58be982d3c3a0dd570ce2684aa87686 100644 --- a/ipalib/plugins/automember.py +++ b/ipalib/plugins/automember.py @@ -17,8 +17,11 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. import uuid +import time + import ldap as _ldap -from ipalib import api, errors, Str, StrEnum, _, ngettext + +from ipalib import api, errors, Str, StrEnum, DNParam, _, ngettext from ipalib.plugins.baseldap import * from ipalib.request import context from ipapython.dn import DN @@ -623,9 +626,21 @@ class automember_rebuild(Command): label=_('Hosts'), doc=_('Rebuild membership for specified hosts'), ), + Flag( + 'no_wait?', + default=False, + label=_('No wait'), + doc=_("Don't wait for rebuilding membership"), + ), + ) + has_output = output.standard_entry + has_output_params = ( + DNParam( + 'dn', + label=_('Task DN'), + doc=_('DN of the started task'), + ), ) - has_output = output.standard_value - msg_summary = _('Automember rebuild membership task completed') def validate(self, **kw): """ @@ -693,20 +708,51 @@ class automember_rebuild(Command): else: search_filter = '(%s=*)' % obj.primary_key.name + task_dn = DN( + ('cn', cn), + ('cn', 'automember rebuild membership'), + ('cn', 'tasks'), + ('cn', 'config')) + entry = ldap.make_entry( - DN( - ('cn', cn), - ('cn', 'automember rebuild membership'), - ('cn', 'tasks'), - ('cn', 'config'), - ), + task_dn, objectclass=['top', 'extensibleObject'], cn=[cn], basedn=[basedn], filter=[search_filter], - scope=['sub'] - ) + scope=['sub'], + ttl=[3600]) ldap.add_entry(entry) - return dict(result=True, value=u'') + + summary = _('Automember rebuild membership task started') + result = {'dn': task_dn} + + if not options.get('no_wait'): + summary = _('Automember rebuild membership task completed') + result = {} + start_time = time.time() + + while True: + try: + task = ldap.get_entry(task_dn) + except errors.NotFound: + break + + if 'nstaskexitcode' in task: + if str(task.single_value['nstaskexitcode']) == '0': + summary=task.single_value['nstaskstatus'] + break + else: + raise errors.DatabaseError( + desc=task.single_value['nstaskstatus'], + info=_("Task DN = '%s'" % task_dn)) + time.sleep(1) + if time.time() > (start_time + 60): + raise errors.TaskTimeout(task=_('Automember'), task_dn=task_dn) + + return dict( + result=result, + summary=unicode(summary), + value=u'') api.register(automember_rebuild) diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py index 9453ebb5a6a06fa8e1fa86c0dee58619a86f915d..600d54890d580903d580fd9a2ea7e38299c683da 100644 --- a/ipatests/test_xmlrpc/test_automember_plugin.py +++ b/ipatests/test_xmlrpc/test_automember_plugin.py @@ -24,7 +24,8 @@ Test the `ipalib/plugins/automember.py` module. from ipalib import api, errors from ipapython.dn import DN from ipatests.test_xmlrpc import objectclasses -from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid +from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid, \ + fuzzy_automember_dn, fuzzy_automember_message from ipatests.test_xmlrpc.test_user_plugin import get_user_result @@ -242,8 +243,20 @@ class test_automember(Declarative): command=('automember_rebuild', [], dict(type=u'hostgroup')), expected=dict( value=u'', - summary=u'Automember rebuild membership task completed', - result=True + summary=fuzzy_automember_message, + result=dict() + ), + ), + + dict( + desc='Rebuild membership for hostgroups asynchronously', + command=('automember_rebuild', [], dict(type=u'hostgroup',no_wait=True)), + expected=dict( + value=u'', + summary=u'Automember rebuild membership task started', + result=dict( + dn=fuzzy_automember_dn + ), ), ), @@ -349,8 +362,20 @@ class test_automember(Declarative): command=('automember_rebuild', [], dict(hosts=fqdn1)), expected=dict( value=u'', - summary=u'Automember rebuild membership task completed', - result=True + summary=fuzzy_automember_message, + result=dict() + ), + ), + + dict( + desc='Rebuild membership for host: %s asynchronously' % fqdn1, + command=('automember_rebuild', [], dict(hosts=fqdn1, no_wait=True)), + expected=dict( + value=u'', + summary=u'Automember rebuild membership task started', + result=dict( + dn=fuzzy_automember_dn + ), ), ), @@ -519,8 +544,20 @@ class test_automember(Declarative): command=('automember_rebuild', [], dict(type=u'group')), expected=dict( value=u'', - summary=u'Automember rebuild membership task completed', - result=True + summary=fuzzy_automember_message, + result=dict() + ), + ), + + dict( + desc='Rebuild membership for groups asynchronously', + command=('automember_rebuild', [], dict(type=u'group', no_wait=True)), + expected=dict( + value=u'', + summary=u'Automember rebuild membership task started', + result=dict( + dn=fuzzy_automember_dn + ), ), ), @@ -584,8 +621,20 @@ class test_automember(Declarative): command=('automember_rebuild', [], dict(users=user1)), expected=dict( value=u'', - summary=u'Automember rebuild membership task completed', - result=True + summary=fuzzy_automember_message, + result=dict() + ), + ), + + dict( + desc='Rebuild membership for user: %s asynchronously' % user1, + command=('automember_rebuild', [], dict(users=user1, no_wait=True)), + expected=dict( + value=u'', + summary=u'Automember rebuild membership task started', + result=dict( + dn=fuzzy_automember_dn + ), ), ), diff --git a/ipatests/test_xmlrpc/xmlrpc_test.py b/ipatests/test_xmlrpc/xmlrpc_test.py index 8f31a1ec02a38ac2abd981b62de406167ccd6147..a596cd69c9a4ab4d9f35361d6db4ed5b3ffa5763 100644 --- a/ipatests/test_xmlrpc/xmlrpc_test.py +++ b/ipatests/test_xmlrpc/xmlrpc_test.py @@ -38,6 +38,16 @@ uuid_re = '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}' # Matches an ipauniqueid like u'784d85fd-eae7-11de-9d01-54520012478b' fuzzy_uuid = Fuzzy('^%s$' % uuid_re) +# Matches an automember task DN +fuzzy_automember_dn = Fuzzy( + '^cn=%s,cn=automember rebuild membership,cn=tasks,cn=config$' % uuid_re +) + +# Matches an automember task finish message +fuzzy_automember_message = Fuzzy( + '^Automember rebuild task finished\. Processed \(\d+\) entries\.$' +) + # Matches trusted domain GUID, like u'463bf2be-3456-4a57-979e-120304f2a0eb' fuzzy_guid = fuzzy_uuid -- 1.9.0
>From c18c4ffba83bfd8b5c1cda1abfff9102d618e326 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki <[email protected]> Date: Tue, 25 Mar 2014 15:15:10 +0100 Subject: [PATCH 2/2] plugin registration refactoring for automembership decorators used for plugin registration in automembership according to: http://www.freeipa.org/page/Coding_Best_Practices#Decorator-based_plugin_registration --- ipalib/plugins/automember.py | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py index ac6f66a7f58be982d3c3a0dd570ce2684aa87686..4b3f6f06f80ca8d20245a784ac2ba9a07c17a3e9 100644 --- a/ipalib/plugins/automember.py +++ b/ipalib/plugins/automember.py @@ -22,6 +22,7 @@ import time import ldap as _ldap from ipalib import api, errors, Str, StrEnum, DNParam, _, ngettext +from ipalib.plugable import Registry from ipalib.plugins.baseldap import * from ipalib.request import context from ipapython.dn import DN @@ -124,6 +125,8 @@ EXAMPLES: ipa automember-rebuild --hosts=web1.example.com --hosts=web2.example.com """) +register = Registry() + # Options used by Condition Add and Remove. INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' @@ -167,6 +170,8 @@ automember_rule = ( ), ) + +@register() class automember(LDAPObject): """ @@ -234,8 +239,6 @@ class automember(LDAPObject): else: raise errors.NotFound(reason=_('%s is not a valid attribute.') % attr) -api.register(automember) - def automember_container_exists(ldap): try: @@ -244,6 +247,8 @@ def automember_container_exists(ldap): return False return True + +@register() class automember_add(LDAPCreate): __doc__ = _(""" Add an automember rule. @@ -266,9 +271,8 @@ class automember_add(LDAPCreate): result['value'] = keys[-1] return result -api.register(automember_add) - +@register() class automember_add_condition(LDAPUpdate): __doc__ = _(""" Add conditions to an automember rule. @@ -359,9 +363,8 @@ class automember_add_condition(LDAPUpdate): result['value'] = keys[-1] return result -api.register(automember_add_condition) - +@register() class automember_remove_condition(LDAPUpdate): __doc__ = _(""" Remove conditions from an automember rule. @@ -447,9 +450,8 @@ class automember_remove_condition(LDAPUpdate): result['value'] = keys[-1] return result -api.register(automember_remove_condition) - +@register() class automember_mod(LDAPUpdate): __doc__ = _(""" Modify an automember rule. @@ -463,9 +465,8 @@ class automember_mod(LDAPUpdate): result['value'] = keys[-1] return result -api.register(automember_mod) - +@register() class automember_del(LDAPDelete): __doc__ = _(""" Delete an automember rule. @@ -479,9 +480,8 @@ class automember_del(LDAPDelete): result['value'] = keys[-1] return result -api.register(automember_del) - +@register() class automember_find(LDAPSearch): __doc__ = _(""" Search for automember rules. @@ -499,9 +499,8 @@ class automember_find(LDAPSearch): ndn = DN(('cn', options['type']), base_dn) return (filters, ndn, scope) -api.register(automember_find) - +@register() class automember_show(LDAPRetrieve): __doc__ = _(""" Display information about an automember rule. @@ -515,9 +514,8 @@ class automember_show(LDAPRetrieve): result['value'] = keys[-1] return result -api.register(automember_show) - +@register() class automember_default_group_set(LDAPUpdate): __doc__ = _(""" Set default (fallback) group for all unmatched entries. @@ -544,9 +542,8 @@ class automember_default_group_set(LDAPUpdate): result['value'] = options['type'] return result -api.register(automember_default_group_set) - +@register() class automember_default_group_remove(LDAPUpdate): __doc__ = _(""" Remove default (fallback) group for all unmatched entries. @@ -579,9 +576,8 @@ class automember_default_group_remove(LDAPUpdate): result['value'] = options['type'] return result -api.register(automember_default_group_remove) - +@register() class automember_default_group_show(LDAPRetrieve): __doc__ = _(""" Display information about the default (fallback) automember groups. @@ -604,9 +600,8 @@ class automember_default_group_show(LDAPRetrieve): result['value'] = options['type'] return result -api.register(automember_default_group_show) - +@register() class automember_rebuild(Command): __doc__ = _('Rebuild auto membership.') # TODO: Add a --dry-run option: @@ -754,5 +749,3 @@ class automember_rebuild(Command): result=result, summary=unicode(summary), value=u'') - -api.register(automember_rebuild) -- 1.9.0
_______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
