cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:uninitialized-variables into cloud-init:master.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/336453

Fix potential cases of uninitialized variables.

While addressing undeclared variable in 'cloud-init status', I also fixed
the errors raised by automated code reviews against cloud-init master at
https://lgtm.com/projects/g/cloud-init/cloud-init/alerts


-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:uninitialized-variables into cloud-init:master.

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'
     return (is_disabled, reason)
 
 
@@ -127,10 +129,11 @@ def _get_status_details(paths):
             status_detail = value
         elif isinstance(value, dict):
             errors.extend(value.get('errors', []))
+            start = value.get('start') or 0
             finished = value.get('finished') or 0
-            if finished == 0:
+            if finished == 0 and start != 0:
                 status = STATUS_RUNNING
-            event_time = max(value.get('start', 0), finished)
+            event_time = max(start, finished)
             if event_time > latest_event:
                 latest_event = event_time
     if errors:
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
@@ -93,6 +93,19 @@ class TestStatus(CiTestCase):
         self.assertTrue(is_disabled, 'expected disabled cloud-init')
         self.assertEqual('Cloud-init disabled by cloud-init-generator', reason)
 
+    def test__is_cloudinit_disabled_false_when_enabled_in_systemd(self):
+        '''Report enabled when systemd generator creates the enabled file.'''
+        enabled_file = os.path.join(self.paths.run_dir, 'enabled')
+        write_file(enabled_file, '')
+        (is_disabled, reason) = wrap_and_call(
+            'cloudinit.cmd.status',
+            {'uses_systemd': True,
+             'get_cmdline': 'something ignored'},
+            status._is_cloudinit_disabled, self.disable_file, self.paths)
+        self.assertFalse(is_disabled, 'expected enabled cloud-init')
+        self.assertEqual(
+            'Cloud-init enabled by systemd cloud-init-generator', reason)
+
     def test_status_returns_not_run(self):
         '''When status.json does not exist yet, return 'not run'.'''
         self.assertFalse(
@@ -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.'''
+        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(
@@ -338,7 +352,8 @@ class TestStatus(CiTestCase):
 
     def test_status_main(self):
         '''status.main can be run as a standalone script.'''
-        write_json(self.status_file, {'v1': {'init': {'finished': None}}})
+        write_json(self.status_file,
+                   {'v1': {'init': {'start': 1, 'finished': None}}})
         with self.assertRaises(SystemExit) as context_manager:
             with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
                 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,
                                 stdout=output, stderr=subprocess.STDOUT)
diff --git a/cloudinit/config/cc_rh_subscription.py b/cloudinit/config/cc_rh_subscription.py
index a9d21e7..539aa00 100644
--- a/cloudinit/config/cc_rh_subscription.py
+++ b/cloudinit/config/cc_rh_subscription.py
@@ -277,8 +277,7 @@ class SubscriptionManager(object):
         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()))
+            self.log_warn("Auto-attach failed with: {0}]".format(e))
             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
@@ -116,6 +116,7 @@ class Distro(distros.Distro):
         (out, err) = util.subp(['ifconfig', '-a'])
         ifconfigoutput = [x for x in (out.strip()).splitlines()
                           if len(x.split()) > 0]
+        bsddev = 'NOT_FOUND'
         for line in ifconfigoutput:
             m = re.match('^\w+', line)
             if m:
@@ -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)
 
         for d in bymac.values():
             d['downable'] = (d['up'] is False or
diff --git a/cloudinit/util.py b/cloudinit/util.py
index e42498d..804070a 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1581,6 +1581,10 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True):
         mtypes = list(mtype)
     elif mtype is None:
         mtypes = None
+    else:
+        raise TypeError(
+            'Unsupported type provided for mtype parameter: {_type}'.format(
+                _type=type(mytype)))
 
     # clean up 'mtype' input a bit based on platform.
     platsys = platform.system().lower()