Fixes to user module for better error handling

The user module now returns the output, both stdout and stderr, from
useradd, usermod, and userdel.  This should help debug cases why one of
those commands fail.  In addition, the user module will now call
fail_json() when the attempted command failed so as to properly
communicate a failure in a playbook.

user

@@ -74,11 +74,10 @@ def user_del(user, **kwargs):
         elif key == 'remove' and kwargs[key]:
             cmd.append('-r')
     cmd.append(user)
-    rc = subprocess.call(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    p = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if rc == 0:
+    (out, err) = p.communicate()
-        return True
+    rc = p.returncode
-    else:
+    return (rc, out, err)
-        return False
 
 def user_add(user, **kwargs):
     cmd = [USERADD]
@@ -118,11 +117,10 @@ def user_add(user, **kwargs):
         elif key == 'system' and kwargs[key] == 'yes':
             cmd.append('-r')
     cmd.append(user)
-    rc = subprocess.call(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    p = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if rc == 0:
+    (out, err) = p.communicate()
-        return True
+    rc = p.returncode
-    else:
+    return (rc, out, err)
-        return False
 
 """
 Without spwd, we would have to resort to reading /etc/shadow
@@ -183,13 +181,12 @@ def user_mod(user, **kwargs):
                 cmd.append(kwargs[key])
     # skip if no changes to be made
     if len(cmd) == 1:
-        return False
+        return (None, '', '')
     cmd.append(user)
-    rc = subprocess.call(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    p = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if rc == 0:
+    (out, err) = p.communicate()
-        return True
+    rc = p.returncode
-    else:
+    return (rc, out, err)
-        return False
 
 def group_exists(group):
     try:
@@ -311,27 +308,44 @@ if append not in [ 'yes', 'no' ]:
 if name is None:
     fail_json(msg='name is required')
 
-changed = False
+result = {}
 rc = 0
+out = ''
+err = ''
+result['name'] = name
 if state == 'absent':
-    if user_exists(name):
+    if not user_exists(name):
-        changed = user_del(name, force=force, remove=remove)
+        result['changed'] = False
-    exit_json(name=name, changed=changed, force=force, remove=remove)
+    else:
+        (rc, out, err) = user_del(name, force=force, remove=remove)
+        if rc != 0:
+            fail_json(name=name, msg=err)
+        else:
+            result['changed'] = True
+        result['force'] = force
+        result['remove'] = remove
 elif state == 'present':
     if not user_exists(name):
-        changed = user_add(name, uid=uid, group=group, groups=groups,
+        (rc, out, err) = user_add(name, uid=uid, group=group, groups=groups,
                                   comment=comment, home=home, shell=shell,
                                   password=password, createhome=createhome,
                                   system=system)
     else:
-        changed = user_mod(name, uid=uid, group=group, groups=groups,
+        (rc, out, err) = user_mod(name, uid=uid, group=group, groups=groups,
                                   comment=comment, home=home, shell=shell,
                                   password=password, append=append)
-
+    if rc is not None and rc != 0:
-    if password is not None:
+        fail_json(name=name, msg=err)
-        exit_json(name=name, changed=changed, password="XXXXXXXX")
+    if rc is None:
+        result['changed'] = False
     else:
-        exit_json(name=name, changed=changed)
+        result['changed'] = True
+    if password is not None:
+        result['password'] = 'NOTLOGGINGPASSWORD'
 
-fail_json(name=name, msg='Unexpected position reached')
+if out:
+    result['stdout'] = out
+if err:
+    result['stderr'] = err
+exit_json(**result)
 sys.exit(0)