Merge lp:~mbruzek/charm-helpers/add_uid_gid into lp:~jamesbeedy/charm-helpers/add_uid_gid

[Matt Bruzek]

Hey James,

Instead of pointing out problems I fixed the problems that I found.

Please consider this merge proposal or if you already have the code fixed, please use/extend the tests that I wrote here.

[james beedy (jamesbeedy)]

Nice work! Thanks for helping with the needed tests! My +1 doesn't really matter for the merge, but you've got it anyway!

review: Approve

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2016-05-31 18:30:01 +0000
+++ charmhelpers/core/host.py	2016-05-31 18:55:12 +0000
@@ -175,19 +175,24 @@
     return os.path.isdir(SYSTEMD_SYSTEM)
 
 
+<<<<<<< TREE
 def adduser(username, uid=None, password=None, shell='/bin/bash',
             system_user=False, primary_group=None, secondary_groups=None):
+=======
+def adduser(username, password=None, shell='/bin/bash', system_user=False,
+            primary_group=None, secondary_groups=None, uid=None):
+>>>>>>> MERGE-SOURCE
     """Add a user to the system.
 
     Will log but otherwise succeed if the user already exists.
 
     :param str username: Username to create
-    :param int uid: UID for user being created
     :param str password: Password for user; if ``None``, create a system user
     :param str shell: The default shell for the user
     :param bool system_user: Whether to create a login or system user
     :param str primary_group: Primary group for user; defaults to username
     :param list secondary_groups: Optional list of additional groups
+    :param int uid: UID for user being created
 
     :returns: The password database entry struct, as returned by `pwd.getpwnam`
     """

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2016-05-31 18:30:01 +0000
+++ tests/core/test_host.py	2016-05-31 18:55:12 +0000
@@ -642,6 +642,52 @@
         getpwnam.assert_called_with(username)
 
     @patch('pwd.getpwnam')
+    @patch('pwd.getpwuid')
+    @patch('grp.getgrnam')
+    @patch('subprocess.check_call')
+    @patch.object(host, 'log')
+    def test_add_user_uid(self, log, check_call, getgrnam, getpwuid, getpwnam):
+        user_name = 'james'
+        user_id = 1111
+        uid_key_error = KeyError('user not found')
+        getpwuid.side_effect = uid_key_error
+        host.adduser(user_name, uid=user_id)
+
+        check_call.assert_called_with([
+            'useradd',
+            '--uid',
+            str(user_id),
+            '--system',
+            '-g',
+            user_name,
+            user_name
+        ])
+        getpwnam.assert_called_with(user_name)
+        getpwuid.assert_called_with(user_id)
+
+    @patch('grp.getgrnam')
+    @patch('grp.getgrgid')
+    @patch('subprocess.check_call')
+    @patch.object(host, 'log')
+    def test_add_group_gid(self, log, check_call, getgrgid, getgrnam):
+        group_name = 'darkhorse'
+        group_id = 1005
+        existing_group_gid = KeyError('group not found')
+        new_group_gid = 1006
+        getgrgid.side_effect = [existing_group_gid, new_group_gid]
+
+        host.add_group(group_name, gid=group_id)
+        check_call.assert_called_with([
+            'addgroup',
+            '--gid',
+            str(group_id),
+            '--group',
+            group_name
+        ])
+        getgrgid.assert_called_with(group_id)
+        getgrnam.assert_called_with(group_name)
+
+    @patch('pwd.getpwnam')
     def test_user_exists_true(self, getpwnam):
         getpwnam.side_effect = 'pw info'
         self.assertTrue(host.user_exists('bob'))