summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChad Smith <chad.smith@canonical.com>2018-01-26 13:36:30 -0700
committerChad Smith <chad.smith@canonical.com>2018-01-26 13:36:30 -0700
commit1d8c327139a8c291eeb244ee1a6a8badd83e9e72 (patch)
tree737b92f0e9d476a64e5a945fdaa0c6d7f4f2bf6f
parentbc84f5023f795c261e32cf0690b2d29e12cfaedd (diff)
downloadvyos-cloud-init-1d8c327139a8c291eeb244ee1a6a8badd83e9e72.tar.gz
vyos-cloud-init-1d8c327139a8c291eeb244ee1a6a8badd83e9e72.zip
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 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
-rw-r--r--cloudinit/cmd/status.py7
-rw-r--r--cloudinit/cmd/tests/test_status.py21
-rw-r--r--cloudinit/config/cc_power_state_change.py1
-rw-r--r--cloudinit/config/cc_rh_subscription.py5
-rw-r--r--cloudinit/distros/freebsd.py11
-rw-r--r--cloudinit/util.py4
6 files changed, 33 insertions, 16 deletions
diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py
index 3e5d0d07..d7aaee9d 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 6d4a11e8..a7c0a91a 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 eba58b02..4da3a588 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 = 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
index a9d21e78..530808ce 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))
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 bad112fe..aa468bca 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,9 @@ 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)
+ 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
index 9976400f..338fb971 100644
--- 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()