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
>From 942a83926d3af0a314c5ad8a2f78ef02d5be553e 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..56bba18f0df48134e42714e89f691d7e4ee98da5 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'')
+
+ if options.get('no_wait'):
+ self.msg_summary = _('Automember rebuild membership task started')
+ result = {'dn': task_dn}
+ else:
+ self.msg_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':
+ self.msg_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,
+ 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 c7e7bfea9848fb58709462b0e633de8372862bc6 Mon Sep 17 00:00:00 2001
From: Adam Misnyovszki <[email protected]>
Date: Tue, 25 Mar 2014 15:15:10 +0100
Subject: [PATCH] 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 56bba18f0df48134e42714e89f691d7e4ee98da5..a46dae5bea367da0772f5f27674ea1839d7102b6 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):
return dict(
result=result,
value=u'')
-
-api.register(automember_rebuild)
--
1.9.0
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
