Fix potential cases of uninitialized variables.

blackboxsw · blackboxsw · commit 1d8c327139a8 · 2018-01-26T13:36:30.000-07:00
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 The following items are addressed: * Fix 'cloud-init status': * Only report 'running' state when any stage in /run/cloud-init/status.json has a start time but no finished time. Default start time to 0 if null. * undeclared variable 'reason' now reports 'Cloud-init enabled by systemd cloud-init-generator' when systemd enables cloud-init * cc_rh_subscription.py util.subp return values aren't set during if an exception is raised, use ProcessExecution as e instead. * distros/freebsd.py: * Drop repetitive looping over ipv4 and ipv6 nic lists. * Initialize bsddev to 'NOTFOUND' in the event that no devs are discovered * declare nics_with_addresses = set() in broader scope outside check_downable conditional * cloudinit/util.py: Raise TypeError if mtype parameter isn't string, iterable or None. LP: #1744796
diff --git 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
@@ -93,6 +93,19 @@ def test__is_cloudinit_disabled_true_when_generator_disables(self):
         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 @@ def fakeexists(filepath):
         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 @@ def fake_sleep(interval):
 
     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
@@ -194,6 +194,7 @@ def doexit(sysexit):
 
 
 def execmd(exe_args, output=None, data_in=None):
+    ret = 1
     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
@@ -276,9 +276,8 @@ def _set_auto_attach(self):
         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))
             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
@@ -116,6 +116,7 @@ def getnetifname(self, dev):
         (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,9 @@ def _get_current_rename_info(self, check_downable=True):
             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)
+            nics_with_addresses = set(self.get_ipv4() + self.get_ipv6())
 
         for d in bymac.values():
             d['downable'] = (d['up'] is False or
diff --git a/cloudinit/util.py b/cloudinit/util.py
@@ -1587,6 +1587,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(mtype)))
 
     # clean up 'mtype' input a bit based on platform.
     platsys = platform.system().lower()
