On Thu, 20 Mar 2014 14:19:51 +0100
Misnyovszki Adam <amisn...@redhat.com> wrote:

> On Fri, 14 Mar 2014 13:26:15 -0400
> Rob Crittenden <rcrit...@redhat.com> wrote:
> 
> > Misnyovszki Adam wrote:
> > > Hi,
> > >
> > > 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, according to
> > > http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation.
> > > Old usage can be enabled using --nowait.
> > >
> > > https://fedorahosted.org/freeipa/ticket/4239
> > >
> > > Request for comments:
> > > - Should I add a parameter to specify the polling time? (now 1ms)
> > > - Should I add a parameter to specify the maximum polling number?
> > > Now if something fails about creating the task, it polls forever.
> > > - Obviously, if these parameters should be added, there should be
> > > a reasonable default for them (~ Required=False, Default=X).
> > 
> > I don't think you need a polling time, esp since this is hidden from
> > the user, but I think that is probably too short and you may end up 
> > hammering the LDAP server.
> > 
> > I also wonder if there should be some maximum wait time. I don't
> > like loops that can never exit. I'm at a loss for what that time
> > should be though. And we'd need to spell out that we gave up
> > waiting, not that the task necessarily failed. So rather than
> > having a polling time option, rename nowait into wait_for=20, so
> > wait for 20 seconds. Or something like that.
> > 
> > I'd suggest using get_entry since you already know the full DN and
> > there is only ever one. It would make this much more readable.
> > 
> > I wonder if some top-level documentation should be added to flesh
> > this out some more. This does, for example, return False in one
> > case. The meaning for that should be spelled out.
> > 
> > rob
> 
> Hi,
> personally I would keep --no-wait, with a delay of 1 seconds, and a
> maximum waiting time for 60 seconds. Questions is, do we need to
> parameterize with other parameters(wait-for to the maximum time,
> and/or poll-delay for the delay time, both not required), or it is
> not a case which worth to develop?
> Greets
> Adam

Hi,
here are the corrections Petr asked, also the other patch conatins the
plugin registration refactor.
Thanks
Adam

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

>From 32c4fdb505b02a582afbff65366382216982359d Mon Sep 17 00:00:00 2001
From: Adam Misnyovszki <amisn...@redhat.com>
Date: Thu, 20 Mar 2014 15:50:29 +0100
Subject: [PATCH] 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.

https://fedorahosted.org/freeipa/ticket/4239
---
 API.txt                      |  3 ++-
 ipalib/plugins/automember.py | 52 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..52959922241d0df11556c2890bf56d7b8107ed62 100644
--- a/API.txt
+++ b/API.txt
@@ -201,10 +201,11 @@ 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,5,3
 option: Str('hosts*')
 option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup'))
 option: Str('users*')
+option: Flag('no_wait?', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Output('result', <type 'bool'>, None)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index a12bfb52522e38bc083d0750dc66c894a4aeba53..feb2cd7637fcdbf31bf5dea86be1667ffd51876b 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -17,7 +17,10 @@
 # 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.plugins.baseldap import *
 from ipalib.request import context
@@ -623,9 +626,14 @@ 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_value
-    msg_summary = _('Automember rebuild membership task completed')
 
     def validate(self, **kw):
         """
@@ -693,20 +701,44 @@ 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'])
         ldap.add_entry(entry)
-        return dict(result=True, value=u'')
+
+        no_wait = options.get('no_wait')
+        if not no_wait:
+            start_time = time.time()
+            while True:
+                try:
+                    task = ldap.get_entry(task_dn)
+                except errors.NotFound:
+                    break
+
+                if 'nstaskexitcode' in task:
+                    if str(task['nstaskexitcode'][0]) == '0':
+                        break
+                    else:
+                        raise errors.DatabaseError(
+                            desc=_("Automember rebuild membership task failed"), 
+                            info=_("nstaskexitcode = '%s'" % str(task['nstaskexitcode'][0])))
+                time.sleep(1)
+                if time.time() > (start_time + 60):
+                   raise errors.DatabaseTimeout() 
+
+        return dict(
+            result=True, 
+            value=u'', 
+            summary=u'Automember rebuild membership task completed')
 
 api.register(automember_rebuild)
-- 
1.8.5.3

>From e6da821cdd9196a427a3db7fb35a3d0252f32171 Mon Sep 17 00:00:00 2001
From: Adam Misnyovszki <amisn...@redhat.com>
Date: Thu, 20 Mar 2014 16:10:50 +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 | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index feb2cd7637fcdbf31bf5dea86be1667ffd51876b..bb14d3f3f54ca7041622db24ba1316a471db4b0f 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, _, 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,7 @@ automember_rule = (
     ),
 )
 
+@register()
 class automember(LDAPObject):
 
     """
@@ -234,8 +238,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 +246,8 @@ def automember_container_exists(ldap):
         return False
     return True
 
+
+@register()
 class automember_add(LDAPCreate):
     __doc__ = _("""
     Add an automember rule.
@@ -266,9 +270,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 +362,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 +449,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 +464,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 +479,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 +498,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 +513,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 +541,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 +575,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 +599,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:
@@ -740,5 +734,3 @@ class automember_rebuild(Command):
             result=True, 
             value=u'', 
             summary=u'Automember rebuild membership task completed')
-
-api.register(automember_rebuild)
-- 
1.8.5.3

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

Reply via email to