Make basegroup-{add, del}-member print failed members with error descriptions.

4 files changed, 116 insertions, 85 deletions

diff --git a/ipalib/plugins/basegroup.py b/ipalib/plugins/basegroup.py
index e4f784c72..257758738 100644
--- a/ipalib/plugins/basegroup.py
+++ b/ipalib/plugins/basegroup.py
@@ -43,20 +43,21 @@ def find_members(ldap, failed, members, attr, object_class, parent_dn=''):
     :param object_class: type of entry we're looking for
     :param parent_dn: base DN for the search
     """
-    found = []
+    found = {}
     for m in members:
-        if not m: continue
+        if not m:
+            continue
         try:
             (member_dn, entry_attrs) = ldap.find_entry_by_attr(
                 attr, m, object_class, [''], parent_dn
             )
-            found.append(member_dn)
-        except errors.NotFound:
-            failed.append(m)
+            found[m] = member_dn
+        except errors.NotFound, e:
+            failed[m] = 'ERROR: %s' % e.message
 
     return (found, failed)
 
-def add_members(ldap, completed, members, add_failed, group_dn, memberattr):
+def add_members(ldap, completed, members, add_failed, group_dn, member_attr):
     """
     Add members to a group.
 
@@ -67,20 +68,20 @@ def add_members(ldap, completed, members, add_failed, group_dn, memberattr):
     :param members: list of member DNs to add
     :param add_failed: members who failed to be added
     :param dn: DN of group to add members to
-    :param membetattr: The attribute where members are stored
+    :param membet_attr: The attribute where members are stored
     """
-    for member_dn in members:
+    for (m, member_dn) in members.iteritems():
         if not member_dn:
             continue
         try:
-            ldap.add_entry_to_group(member_dn, group_dn, memberattr)
+            ldap.add_entry_to_group(member_dn, group_dn, member_attr)
             completed += 1
-        except:
-            add_failed.append(member_dn)
+        except Exception, e:
+            add_failed[m] = 'ERROR: %s' % e.message
 
     return (completed, add_failed)
 
-def del_members(ldap, completed, members, rem_failed, group_dn, memberattr):
+def del_members(ldap, completed, members, rem_failed, group_dn, member_attr):
     """
     Remove members from group.
 
@@ -91,15 +92,16 @@ def del_members(ldap, completed, members, rem_failed, group_dn, memberattr):
     :param members: list of member DNs to remove
     :param remove_failed: members who failed to be removed
     :param dn: DN of group to remove members from
-    :param membetattr: The attribute where members are stored
+    :param membet_attr: The attribute where members are stored
     """
-    for member_dn in members:
-        if not member_dn: continue
+    for (m, member_dn) in members.iteritems():
+        if not member_dn:
+            continue
         try:
-            ldap.remove_entry_from_group(member_dn, group_dn, memberattr)
+            ldap.remove_entry_from_group(member_dn, group_dn, member_attr)
             completed += 1
-        except:
-            rem_failed.append(member_dn)
+        except Exception, e:
+            rem_failed[m] = 'ERROR: %s' % e.message
 
     return (completed, rem_failed)
 
@@ -324,7 +326,7 @@ class basegroup_find(crud.Search):
             textui.print_entry(entry_attrs)
             textui.print_plain('')
         textui.print_count(
-            len(result), '%i group matched.', '%i groups matched.'
+            len(entries), '%i group matched.', '%i groups matched.'
         )
         if truncated:
             textui.print_dashed('These results are truncated.', below=False)
@@ -419,7 +421,7 @@ class basegroup_add_member(Command):
         assert self.container
         ldap = self.api.Backend.ldap2
         to_add = []
-        add_failed = []
+        add_failed = {}
         completed = 0
 
         (dn, entry_attrs) = ldap.find_entry_by_attr(
@@ -444,14 +446,17 @@ class basegroup_add_member(Command):
             ldap, completed, to_add, add_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, self.default_attributes))
+        return (
+            completed, add_failed, ldap.get_entry(dn, self.default_attributes)
+        )
 
     def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        (total, (dn, entry_attrs)) = result
+        (total, failed, (dn, entry_attrs)) = result
 
+        textui.print_entry(failed, indent=0)
         textui.print_name(self.name)
         textui.print_attribute('dn', dn)
         textui.print_entry(entry_attrs)
@@ -498,7 +503,7 @@ class basegroup_del_member(Command):
         assert self.container
         ldap = self.api.Backend.ldap2
         to_remove = []
-        remove_failed = []
+        rem_failed = {}
         completed = 0
 
         (dn, entry_attrs) = ldap.find_entry_by_attr(
@@ -506,33 +511,38 @@ class basegroup_del_member(Command):
         )
 
         members = kw.get('groups', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'cn', 'ipausergroup',
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'cn', 'ipausergroup',
             self.api.env.container_group
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
         members = kw.get('users', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'uid', 'posixaccount',
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'uid', 'posixaccount',
             self.api.env.container_user
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, self.default_attributes))
+        return (
+            completed, rem_failed, ldap.get_entry(dn, self.default_attributes)
+        )
 
     def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        (total, (dn, entry_attrs)) = result
+        (total, failed, (dn, entry_attrs)) = result
 
+        textui.print_entry(failed, indent=0)
         textui.print_name(self.name)
         textui.print_attribute('dn', dn)
+
         textui.print_entry(entry_attrs)
+
         textui.print_count(total, '%i member removed.', '%i members removed.')
 
diff --git a/ipalib/plugins/hostgroup.py b/ipalib/plugins/hostgroup.py
index 272d5720d..1147083cd 100644
--- a/ipalib/plugins/hostgroup.py
+++ b/ipalib/plugins/hostgroup.py
@@ -136,7 +136,7 @@ class hostgroup_add_member(basegroup_add_member):
             'cn', cn, self.filter_class, [''], self.container
         )
         to_add = []
-        add_failed = []
+        add_failed = {}
         completed = 0
 
         members = kw.get('groups', [])
@@ -166,7 +166,9 @@ class hostgroup_add_member(basegroup_add_member):
             ldap, completed, to_add, add_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, self.default_attributes))
+        return (
+            completed, add_failed, ldap.get_entry(dn, self.default_attributes)
+        )
 
 api.register(hostgroup_add_member)
 
@@ -208,37 +210,39 @@ class hostgroup_del_member(basegroup_del_member):
             'cn', cn, self.filter_class, [''], self.container
         )
         to_remove = []
-        remove_failed = []
+        rem_failed = {}
         completed = 0
 
         members = kw.get('groups', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'cn', 'ipausergroup',
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'cn', 'ipausergroup',
             self.api.env.container_group
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
         members = kw.get('hosts', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'cn', 'ipahost',
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'cn', 'ipahost',
             self.api.env.container_host
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
         members = kw.get('hostgroups', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'cn', 'ipahostgroup',
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'cn', 'ipahostgroup',
             self.api.env.container_hostgroup
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, _default_attributes))
+        return (
+            completed, rem_failed, ldap.get_entry(dn, _default_attributes)
+        )
 
 api.register(hostgroup_del_member)
 
diff --git a/ipalib/plugins/netgroup.py b/ipalib/plugins/netgroup.py
index a80913d08..76844edb7 100644
--- a/ipalib/plugins/netgroup.py
+++ b/ipalib/plugins/netgroup.py
@@ -21,7 +21,7 @@
 Netgroups
 """
 
-from ipalib import api
+from ipalib import api, errors
 from ipalib.plugins.basegroup import *
 from ipalib import uuid
 
@@ -159,17 +159,18 @@ class netgroup_add_member(basegroup_add_member):
     )
 
     def _add_external(self, ldap, completed, members, group_dn):
-        add_failed = []
+        add_failed = {}
         (dn, entry_attrs) = ldap.get_entry(group_dn, ['externalhost'])
         external_hosts = entry_attrs.get('externalhost', [])
+        e = errors.AlreadyGroupMember()
 
-        for m in members:
+        for m in members.keys():
             m = m.lower()
             if m not in external_hosts:
                 external_hosts.append(m)
                 completed += 1
             else:
-                add_failed.append(m)
+                add_failed[m] = 'ERROR: %s' % e.message
 
         try:
             ldap.update_entry(group_dn, {'externalhost': external_hosts})
@@ -196,7 +197,7 @@ class netgroup_add_member(basegroup_add_member):
             'cn', cn, self.filter_class, [''], self.container
         )
         to_add = []
-        add_failed = []
+        add_failed = {}
         completed = 0
 
         members = kw.get('groups', [])
@@ -217,17 +218,20 @@ class netgroup_add_member(basegroup_add_member):
             ldap, completed, to_add, add_failed, dn, 'member'
         )
 
-        add_failed = []
+        hosts_failed = {}
         members = kw.get('hosts', [])
-        (to_add, add_failed) = find_members(
-            ldap, add_failed, members, 'cn', 'ipahost',
+        (to_add, hosts_failed) = find_members(
+            ldap, hosts_failed, members, 'cn', 'ipahost',
             self.api.env.container_host
         )
 
         # If a host is not found we'll consider it an externalHost. It will
         # be up to the user to handle typos
-        if add_failed:
-            (completed, add_failed) = self._add_external(ldap, completed, add_failed, dn)
+        if hosts_failed:
+            (completed, hosts_failed) = self._add_external(
+                ldap, completed, hosts_failed, dn
+            )
+            add_failed.update(hosts_failed)
 
         (completed, add_failed) = add_members(
             ldap, completed, to_add, add_failed, dn, 'member'
@@ -251,7 +255,9 @@ class netgroup_add_member(basegroup_add_member):
             ldap, completed, to_add, add_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, _default_attributes))
+        return (
+            completed, add_failed, ldap.get_entry(dn, _default_attributes)
+        )
 
 api.register(netgroup_add_member)
 
@@ -280,17 +286,18 @@ class netgroup_del_member(basegroup_del_member):
     )
 
     def _del_external(self, ldap, completed, members, group_dn):
-        rem_failed = []
+        rem_failed = {}
         (dn, entry_attrs) = ldap.get_entry(group_dn, ['externalhost'])
         external_hosts = entry_attrs.get('externalhost', [])
+        e = errors.NotGroupMember()
 
-        for m in members:
+        for m in members.keys():
             m = m.lower()
             if m in external_hosts:
                 external_hosts.remove(m)
                 completed += 1
             else:
-                rem_failed.append(m)
+                rem_failed[m] = 'ERROR: %s' % e.message
 
         try:
             ldap.update_entry(group_dn, {'externalhost': external_hosts})
@@ -317,7 +324,7 @@ class netgroup_del_member(basegroup_del_member):
             'cn', cn, self.filter_class, [''], self.container
         )
         to_rem = []
-        rem_failed = []
+        rem_failed = {}
         completed = 0
 
         members = kw.get('groups', [])
@@ -338,16 +345,20 @@ class netgroup_del_member(basegroup_del_member):
             ldap, completed, to_rem, rem_failed, dn, 'member'
         )
 
+        hosts_failed = {}
         members = kw.get('hosts', [])
-        (to_rem, rem_failed) = find_members(
-            ldap, rem_failed, members, 'cn', 'ipahost',
+        (to_rem, hosts_failed) = find_members(
+            ldap, hosts_failed, members, 'cn', 'ipahost',
             self.api.env.container_host
         )
 
         # If a host is not found we'll consider it an externalHost. It will
         # be up to the user to handle typos
-        if rem_failed:
-            (completed, rem_failed) = self._del_external(ldap, completed, rem_failed, dn)
+        if hosts_failed:
+            (completed, hosts_failed) = self._del_external(
+                ldap, completed, hosts_failed, dn
+            )
+            rem_failed.update(hosts_failed)
 
         (completed, rem_failed) = del_members(
             ldap, completed, to_rem, rem_failed, dn, 'member'
@@ -371,7 +382,9 @@ class netgroup_del_member(basegroup_del_member):
             ldap, completed, to_rem, rem_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, _default_attributes))
+        return (
+            completed, rem_failed, ldap.get_entry(dn, _default_attributes)
+        )
 
 api.register(netgroup_del_member)
 
diff --git a/ipalib/plugins/taskgroup.py b/ipalib/plugins/taskgroup.py
index 380f18618..f46baffc9 100644
--- a/ipalib/plugins/taskgroup.py
+++ b/ipalib/plugins/taskgroup.py
@@ -111,7 +111,7 @@ class taskgroup_add_member(basegroup_add_member):
             'cn', cn, self.filter_class, [''], self.container
         )
         to_add = []
-        add_failed = []
+        add_failed = {}
         completed = 0
 
         members = kw.get('groups', [])
@@ -141,7 +141,9 @@ class taskgroup_add_member(basegroup_add_member):
             ldap, completed, to_add, add_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, _default_attributes))
+        return (
+            completed, add_failed, ldap.get_entry(dn, _default_attributes)
+        )
 
 api.register(taskgroup_add_member)
 
@@ -175,37 +177,39 @@ class taskgroup_del_member(basegroup_del_member):
             'cn', cn, self.filter_class, [''], self.container
         )
         to_remove = []
-        remove_failed = []
+        rem_failed = {}
         completed = 0
 
         members = kw.get('groups', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'cn', 'ipaUserGroup',
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'cn', 'ipaUserGroup',
             self.api.env.container_group
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
         members = kw.get('hosts', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'cn', 'ipaHost',
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'cn', 'ipaHost',
             self.api.env.container_host
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
         members = kw.get('rolegroups', [])
-        (to_remove, remove_failed) = find_members(
-            ldap, remove_failed, members, 'cn', self.filter_class,
+        (to_remove, rem_failed) = find_members(
+            ldap, rem_failed, members, 'cn', self.filter_class,
             self.api.env.container_rolegroup
         )
-        (completed, remove_failed) = del_members(
-            ldap, completed, to_remove, remove_failed, dn, 'member'
+        (completed, rem_failed) = del_members(
+            ldap, completed, to_remove, rem_failed, dn, 'member'
         )
 
-        return (completed, ldap.get_entry(dn, _default_attributes))
+        return (
+            completed, rem_failed, ldap.get_entry(dn, _default_attributes)
+        )
 
 api.register(taskgroup_del_member)