cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Small changes requested.

Diff comments:

> diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
> index b8a48e8..1c3f1ce 100644
> --- a/cloudinit/distros/__init__.py
> +++ b/cloudinit/distros/__init__.py
> @@ -580,8 +580,15 @@ class Distro(object):
>              # about long names.
>              util.subp(['passwd', '-l', name])
>          except Exception as e:

this really should catch util.ProcessExecutionError.
can you please fix that?
as it is if you caught an Exception, the exception might not have a exit_code so then that would raise an attributeError.

The right fix is to just catch util.ProcessExecutionError which will always have the exit_code.

that is actually what pylint caught:

cloudinit/distros/__init__.py:583: [E1101(no-member), Distro.lock_passwd] Instance of 'Exception' has no 'exit_code' member

> -            util.logexc(LOG, 'Failed to disable password for user %s', name)
> -            raise e
> +            if e.exit_code != 3:
> +                util.logexc(
> +                    LOG, 'Failed to disable password for user %s', name
> +                )
> +                raise e
> +            else:

don't logexc if code is 3. that is just "working as designed". you can log debug if you wanted.

> +                util.logexc(
> +                    LOG, 'Password access already locked for user %s', name
> +                )
>  
>      def set_passwd(self, user, passwd, hashed=False):
>          pass_string = '%s:%s' % (user, passwd)
> diff --git a/tests/unittests/test_distros/test_create_users.py b/tests/unittests/test_distros/test_create_users.py
> index c3f258d..cf7cff9 100644
> --- a/tests/unittests/test_distros/test_create_users.py
> +++ b/tests/unittests/test_distros/test_create_users.py
> @@ -240,4 +244,13 @@ class TestCreateUser(CiTestCase):
>              [mock.call(set(['auth1']), user),  # not disabled
>               mock.call(set(['key1']), 'foouser', options=disable_prefix)])
>  
> +    def test_lock_passwd_already_locked(self, m_subp, m_is_snappy):
> +        """Do not propagate the exception when user is already locked"""
> +        m_subp.side_effect = UserLockedError()

m_subp.side_effect = util.ProcessExecutionError(exit_code=3)

> +        user = 'foouser'
> +        self.dist.lock_passwd(user)
> +        self.assertIn(
> +            'Password access already locked for user foouser',
> +            self.logs.getvalue())
> +
>  # vi: ts=4 expandtab


-- 
https://code.launchpad.net/~rjschwei/cloud-init/+git/cloud-init/+merge/355254
Your team cloud-init commiters is requested to review the proposed merge of ~rjschwei/cloud-init:handleUsrLocked into cloud-init:master.