Return reason for failure when updating group membership fails.

We used to return a list of dns that failed to be added. We now return
a list of tuples instead. The tuple looks like (dn, reason) where reason
is the exception that was returned.

Also made the label we use for failures to be singular instead of plural
since we now print them out individually instead of as comma-separated.

ticket 270

7 files changed, 78 insertions, 48 deletions

diff --git a/ipalib/cli.py b/ipalib/cli.py
index e8242bfd2..6d057ca93 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -274,7 +274,17 @@ class textui(backend.Backend):
                     self.print_indented(format % (attr, self.encode_binary(v)), indent)
             else:
                 value = map(lambda v: self.encode_binary(v), value)
-                text = ', '.join(value)
+                if len(value) > 0 and type(value[0]) in (list, tuple):
+                    # This is where we print failed add/remove members
+                    for l in value:
+                        text = ': '.join(l)
+                        self.print_indented(format % (attr, self.encode_binary(text)), indent)
+                    return
+                else:
+                    if len(value) > 0:
+                        text = ', '.join(value)
+                    else:
+                        return
                 line_len = self.get_tty_width()
                 if line_len and text:
                     s_indent = '%s%s' % (
@@ -343,6 +353,8 @@ class textui(backend.Backend):
                 label = labels.get(key, key)
                 value = entry[key]
                 if isinstance(value, dict):
+                    if frontend.entry_count(value) == 0:
+                        continue
                     self.print_indented(format % (label, ''), indent)
                     self.print_entry(
                         value, order, labels, print_all, format,
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 577c2fcc9..73680ef2f 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -46,6 +46,22 @@ def is_rule(obj):
     return callable(obj) and getattr(obj, RULE_FLAG, False) is True
 
 
+def entry_count(entry):
+    """
+    Return the number of entries in an entry. This is primarly for the
+    failed output parameter so we don't print empty values.
+
+    We also use this to determine if a non-zero return value is needed.
+    """
+    num_entries = 0
+    for f in entry:
+        if type(entry[f]) is dict:
+            num_entries = num_entries + entry_count(entry[f])
+        else:
+            num_entries = num_entries + len(entry[f])
+
+    return num_entries
+
 
 class HasParam(Plugin):
     """
@@ -844,22 +860,6 @@ class Command(HasParam):
                 continue
             yield param
 
-    def number_failed(self, failed):
-        """
-        Return the number of entries in the failed output parameter.
-
-        This is used to determine whether the failed members should be
-        displayed and what the return value should be.
-        """
-        num_failed = 0
-        for f in failed:
-            if type(failed[f]) is dict:
-                num_failed = num_failed + self.number_failed(failed[f])
-            else:
-                num_failed = num_failed + len(failed[f])
-
-        return num_failed
-
     def output_for_cli(self, textui, output, *args, **options):
         """
         Generic output method. Prints values the output argument according
@@ -899,7 +899,7 @@ class Command(HasParam):
             if o.lower() == 'count' and result == 0:
                 rv = 1
             elif o.lower() == 'failed':
-                if self.number_failed(result) == 0:
+                if entry_count(result) == 0:
                     # Don't display an empty failed list
                     continue
                 else:
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index f764efbb7..f3aa09d3e 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -117,6 +117,12 @@ global_output_params = (
     Str('externalhost?',
         label=_('External host'),
     ),
+    Str('memberhost',
+        label=_('Failed hosts/hostgroups'),
+    ),
+    Str('memberuser',
+        label=_('Failed users/groups'),
+    ),
 )
 
 
@@ -818,7 +824,7 @@ class LDAPModMember(LDAPQuery):
                 name = to_cli(ldap_obj_name)
                 doc = self.member_param_doc % ldap_obj.object_name_plural
                 yield List('%s?' % name, cli_name='%ss' % name, doc=doc,
-                           label=ldap_obj.object_name_plural)
+                           label=ldap_obj.object_name)
 
     def get_member_dns(self, **options):
         dns = {}
@@ -835,8 +841,8 @@ class LDAPModMember(LDAPQuery):
                     ldap_obj = self.api.Object[ldap_obj_name]
                     try:
                         dns[attr][ldap_obj_name].append(ldap_obj.get_dn(name))
-                    except errors.PublicError:
-                        failed[attr][ldap_obj_name].append(name)
+                    except errors.PublicError, e:
+                        failed[attr][ldap_obj_name].append((name, unicode(e)))
         return (dns, failed)
 
 
@@ -884,10 +890,11 @@ class LDAPAddMember(LDAPModMember):
                         continue
                     try:
                         ldap.add_entry_to_group(m_dn, dn, attr)
-                    except errors.PublicError:
+                    except errors.PublicError, e:
                         ldap_obj = self.api.Object[ldap_obj_name]
-                        failed[attr][ldap_obj_name].append(
-                            ldap_obj.get_primary_key_from_dn(m_dn)
+                        failed[attr][ldap_obj_name].append((
+                            ldap_obj.get_primary_key_from_dn(m_dn),
+                            unicode(e),)
                         )
                     else:
                         completed += 1
@@ -985,10 +992,11 @@ class LDAPRemoveMember(LDAPModMember):
                         continue
                     try:
                         ldap.remove_entry_from_group(m_dn, dn, attr)
-                    except errors.PublicError:
+                    except errors.PublicError, e:
                         ldap_obj = self.api.Object[ldap_obj_name]
-                        failed[attr][ldap_obj_name].append(
-                            ldap_obj.get_primary_key_from_dn(m_dn)
+                        failed[attr][ldap_obj_name].append((
+                            ldap_obj.get_primary_key_from_dn(m_dn),
+                            unicode(e),)
                         )
                     else:
                         completed += 1
diff --git a/ipalib/plugins/netgroup.py b/ipalib/plugins/netgroup.py
index 3b714213a..28647b963 100644
--- a/ipalib/plugins/netgroup.py
+++ b/ipalib/plugins/netgroup.py
@@ -156,13 +156,13 @@ class netgroup_add_member(LDAPAddMember):
             external_hosts = entry_attrs_.get('externalhost', [])
             failed_hosts = []
             for host in failed['memberhost']['host']:
-                host = host.lower()
-                host_dn = self.api.Object['host'].get_dn(host)
-                if host not in external_hosts and host_dn not in members:
-                    external_hosts.append(host)
+                hostname = host[0].lower()
+                host_dn = self.api.Object['host'].get_dn(hostname)
+                if hostname not in external_hosts and host_dn not in members:
+                    external_hosts.append(hostname)
                     completed_external += 1
                 else:
-                    failed_hosts.append(host)
+                    failed_hosts.append(hostname)
             if completed_external:
                 try:
                     ldap.update_entry(dn, {'externalhost': external_hosts})
@@ -190,12 +190,12 @@ class netgroup_remove_member(LDAPRemoveMember):
             failed_hosts = []
             completed_external = 0
             for host in failed['memberhost']['host']:
-                host = host.lower()
-                if host in external_hosts:
-                    external_hosts.remove(host)
+                hostname = host[0].lower()
+                if hostname in external_hosts:
+                    external_hosts.remove(hostname)
                     completed_external += 1
                 else:
-                    failed_hosts.append(host)
+                    failed_hosts.append(hostname)
             if completed_external:
                 try:
                     ldap.update_entry(dn, {'externalhost': external_hosts})
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 6218ebe59..10d3f6748 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -403,7 +403,7 @@ class test_group(Declarative):
                 completed=0,
                 failed=dict(
                     member=dict(
-                        group=(u'notfound',),
+                        group=[(u'notfound', u'no such entry')],
                         user=tuple(),
                     ),
                 ),
@@ -449,7 +449,7 @@ class test_group(Declarative):
                 completed=0,
                 failed=dict(
                     member=dict(
-                        group=(u'notfound',),
+                        group=[(u'notfound', u'This entry is not a member of the group')],
                         user=tuple(),
                     ),
                 ),
diff --git a/tests/test_xmlrpc/test_netgroup_plugin.py b/tests/test_xmlrpc/test_netgroup_plugin.py
index 3c1fc34dc..1976791ba 100644
--- a/tests/test_xmlrpc/test_netgroup_plugin.py
+++ b/tests/test_xmlrpc/test_netgroup_plugin.py
@@ -35,6 +35,16 @@ netgroup_dn = None
 # See if our LDAP server is up and we can talk to it over GSSAPI
 ccache = krbV.default_context().default_ccache().name
 
+def entry_in_failed(entry, failed):
+    """
+    entry is what we're looking for
+    failed is a tuple of tuples of the form (failure, exception)
+    """
+    for f in failed:
+        if entry == f[0]:
+            return True
+    return False
+
 class test_netgroup(XMLRPC_test):
     """
     Test the `netgroup` plugin.
@@ -150,7 +160,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberhost' in failed
         assert 'host' in failed['memberhost']
-        assert self.host_fqdn in failed['memberhost']['host']
+        assert entry_in_failed(self.host_fqdn, failed['memberhost']['host'])
 
         kw = {'raw': True}
         kw['hostgroup'] = self.hg_cn
@@ -159,7 +169,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberhost' in failed
         assert 'hostgroup' in failed['memberhost']
-        assert self.hg_cn in failed['memberhost']['hostgroup']
+        assert entry_in_failed(self.hg_cn, failed['memberhost']['hostgroup'])
 
         kw = {'raw': True}
         kw['user'] = self.user_uid
@@ -168,7 +178,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberuser' in failed
         assert 'user' in failed['memberuser']
-        assert self.user_uid in failed['memberuser']['user']
+        assert entry_in_failed(self.user_uid, failed['memberuser']['user'])
 
         kw = {'raw': True}
         kw['group'] = self.group_cn
@@ -177,7 +187,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberuser' in failed
         assert 'group' in failed['memberuser']
-        assert self.group_cn in failed['memberuser']['group']
+        assert entry_in_failed(self.group_cn, failed['memberuser']['group'])
 
     def test_5_netgroup_add_member(self):
         """
@@ -301,7 +311,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberhost' in failed
         assert 'host' in failed['memberhost']
-        assert self.host_fqdn in failed['memberhost']['host']
+        assert entry_in_failed(self.host_fqdn, failed['memberhost']['host'])
 
         kw = {'raw': True}
         kw['hostgroup'] = self.hg_cn
@@ -310,7 +320,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberhost' in failed
         assert 'hostgroup' in failed['memberhost']
-        assert self.hg_cn in failed['memberhost']['hostgroup']
+        assert entry_in_failed(self.hg_cn, failed['memberhost']['hostgroup'])
 
         kw = {'raw': True}
         kw['user'] = self.user_uid
@@ -320,7 +330,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberuser' in failed
         assert 'user' in failed['memberuser']
-        assert self.user_uid in failed['memberuser']['user']
+        assert entry_in_failed(self.user_uid, failed['memberuser']['user'])
 
         kw = {'raw': True}
         kw['group'] = self.group_cn
@@ -329,7 +339,7 @@ class test_netgroup(XMLRPC_test):
         failed = ret['failed']
         assert 'memberuser' in failed
         assert 'group' in failed['memberuser']
-        assert self.group_cn in failed['memberuser']['group']
+        assert entry_in_failed(self.group_cn, failed['memberuser']['group'])
 
     def test_b_netgroup_del(self):
         """
diff --git a/tests/test_xmlrpc/test_sudocmdgroup_plugin.py b/tests/test_xmlrpc/test_sudocmdgroup_plugin.py
index 6374c09e4..ec2164c49 100644
--- a/tests/test_xmlrpc/test_sudocmdgroup_plugin.py
+++ b/tests/test_xmlrpc/test_sudocmdgroup_plugin.py
@@ -384,7 +384,7 @@ class test_sudocmdgroup(Declarative):
                 failed=dict(
                     member=dict(
                         sudocmdgroup=tuple(),
-                        sudocmd=(u'notfound',),
+                        sudocmd=[(u'notfound', u'no such entry')],
                     ),
                 ),
                 result={
@@ -430,7 +430,7 @@ class test_sudocmdgroup(Declarative):
                 failed=dict(
                     member=dict(
                         sudocmdgroup=tuple(),
-                        sudocmd=(u'notfound',),
+                        sudocmd=[(u'notfound', u'This entry is not a member of the group')],
                     ),
                 ),
                 result={