Merge pull request #568 from sfromm/user

User modules fixes

user

@@ -75,11 +75,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]
@@ -119,11 +118,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
@@ -184,13 +182,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:
@@ -313,27 +310,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,
+                                  comment=comment, home=home, shell=shell,
-                           password=password, createhome=createhome,
+                                  password=password, createhome=createhome,
-                           system=system)
+                                  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,
+                                  comment=comment, home=home, shell=shell,
-                           password=password, append=append)
+                                  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)