cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py
> index 3e5d0d0..d7aaee9 100644
> --- a/cloudinit/cmd/status.py
> +++ b/cloudinit/cmd/status.py
> @@ -93,6 +93,8 @@ def _is_cloudinit_disabled(disable_file, paths):
>      elif not os.path.exists(os.path.join(paths.run_dir, 'enabled')):
>          is_disabled = True
>          reason = 'Cloud-init disabled by cloud-init-generator'
> +    else:
> +        reason = 'Cloud-init enabled by systemd cloud-init-generator'

i dont like 'systemd'.  I think just "Cloud-init is enabled." why did you want to put 'systemd'?

>      return (is_disabled, reason)
>  
>  
> diff --git a/cloudinit/cmd/tests/test_status.py b/cloudinit/cmd/tests/test_status.py
> index 6d4a11e..bc28c09 100644
> --- a/cloudinit/cmd/tests/test_status.py
> +++ b/cloudinit/cmd/tests/test_status.py
> @@ -137,8 +150,9 @@ class TestStatus(CiTestCase):
>          self.assertEqual(expected, m_stdout.getvalue())
>  
>      def test_status_returns_running(self):
> -        '''Report running when status file exists but isn't finished.'''
> -        write_json(self.status_file, {'v1': {'init': {'finished': None}}})
> +        '''Report running when status  exists with an unfinished stage.'''

when status<two spaces>exists.

> +        write_json(self.status_file,
> +                   {'v1': {'init': {'start': 1, 'finished': None}}})
>          cmdargs = myargs(long=False, wait=False)
>          with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
>              retcode = wrap_and_call(
> diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py
> index eba58b0..229f105 100644
> --- a/cloudinit/config/cc_power_state_change.py
> +++ b/cloudinit/config/cc_power_state_change.py
> @@ -194,6 +194,7 @@ def doexit(sysexit):
>  
>  
>  def execmd(exe_args, output=None, data_in=None):
> +    ret = 0
>      try:
>          proc = subprocess.Popen(exe_args, stdin=subprocess.PIPE,

I think you should initialize this to a non-0 value
so that if anyway it *did* get to the doexit(ret) without getting set to an actual value, it would be an error, not a success.

>                                  stdout=output, stderr=subprocess.STDOUT)
> diff --git a/cloudinit/config/cc_rh_subscription.py b/cloudinit/config/cc_rh_subscription.py
> index a9d21e7..b4ae5e1 100644
> --- a/cloudinit/config/cc_rh_subscription.py
> +++ b/cloudinit/config/cc_rh_subscription.py
> @@ -276,9 +276,8 @@ class SubscriptionManager(object):
>          cmd = ['attach', '--auto']
>          try:
>              return_out, return_err = self._sub_man_cli(cmd)
> -        except util.ProcessExecutionError:
> -            self.log_warn("Auto-attach failed with: "
> -                          "{0}]".format(return_err.strip()))
> +        except util.ProcessExecutionError as e:
> +            self.log_warn("Auto-attach failed with: {0}]".format(e))

Can you remove the confusing ']' .

>              return False
>          for line in return_out.split("\n"):
>              if line is not "":
> diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py
> index bad112f..823b52a 100644
> --- a/cloudinit/distros/freebsd.py
> +++ b/cloudinit/distros/freebsd.py
> @@ -347,15 +348,10 @@ class Distro(distros.Distro):
>              bymac[Distro.get_interface_mac(n)] = {
>                  'name': n, 'up': self.is_up(n), 'downable': None}
>  
> +        nics_with_addresses = set()
>          if check_downable:
> -            nics_with_addresses = set()
> -            ipv6 = self.get_ipv6()
> -            ipv4 = self.get_ipv4()
> -            for bytes_out in (ipv6, ipv4):
> -                for i in ipv6:
> -                    nics_with_addresses.update(i)
> -                for i in ipv4:
> -                    nics_with_addresses.update(i)
> +            for i in self.get_ipv4() + self.get_ipv6():
> +                nics_with_addresses.update(i)

Can't this even *further* be simplified with:

        nics_with_addresses = set()
        if check_downable:
            nics_with_addresses = set(self.get_ipv4() + self.get_ipv6())

>  
>          for d in bymac.values():
>              d['downable'] = (d['up'] is False or


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/336453
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:uninitialized-variables into cloud-init:master.