Merge ~chad.smith/cloud-init:uninitialized-variables into cloud-init:master

Proposed by Chad Smith
Status: Merged
Approved by: Scott Moser
Approved revision: efab669ff3d9c0089b0686c03234093b629bb1c8
Merge reported by: Chad Smith
Merged at revision: 1d8c327139a8c291eeb244ee1a6a8badd83e9e72
Proposed branch: ~chad.smith/cloud-init:uninitialized-variables
Merge into: cloud-init:master
Diff against target: 146 lines (+33/-16)
6 files modified
cloudinit/cmd/status.py (+5/-2)
cloudinit/cmd/tests/test_status.py (+18/-3)
cloudinit/config/cc_power_state_change.py (+1/-0)
cloudinit/config/cc_rh_subscription.py (+2/-3)
cloudinit/distros/freebsd.py (+3/-8)
cloudinit/util.py (+4/-0)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+336453@code.launchpad.net

Commit message

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

Description of the change

See commit message

To post a comment you must log in.
Revision history for this message
Chad Smith (chad.smith) wrote :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:1669768f15889a2224735cae8ddc951087b96a2e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/713/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/713/rebuild

review: Needs Fixing (continuous-integration)
e7bcfc5... by Chad Smith

fix uninitialized local variable alerts as raised by lgtm

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:e7bcfc5e193ad84c7263b1ac6ff351291d35aab1
https://jenkins.ubuntu.com/server/job/cloud-init-ci/734/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/734/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) :
efab669... by Chad Smith

Address scott's review comments:

- Use set declaration to coalesce get_ipv4|ipv6 devices in freebsd
- whitespace docstring unit test fix
- drop trailing ] from log message
- non-zero value for execmd in cc_power_state_change

Revision history for this message
Chad Smith (chad.smith) :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:efab669ff3d9c0089b0686c03234093b629bb1c8
https://jenkins.ubuntu.com/server/job/cloud-init-ci/739/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/739/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

thanks.
lgtm.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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):
93 elif not os.path.exists(os.path.join(paths.run_dir, 'enabled')):93 elif not os.path.exists(os.path.join(paths.run_dir, 'enabled')):
94 is_disabled = True94 is_disabled = True
95 reason = 'Cloud-init disabled by cloud-init-generator'95 reason = 'Cloud-init disabled by cloud-init-generator'
96 else:
97 reason = 'Cloud-init enabled by systemd cloud-init-generator'
96 return (is_disabled, reason)98 return (is_disabled, reason)
9799
98100
@@ -127,10 +129,11 @@ def _get_status_details(paths):
127 status_detail = value129 status_detail = value
128 elif isinstance(value, dict):130 elif isinstance(value, dict):
129 errors.extend(value.get('errors', []))131 errors.extend(value.get('errors', []))
132 start = value.get('start') or 0
130 finished = value.get('finished') or 0133 finished = value.get('finished') or 0
131 if finished == 0:134 if finished == 0 and start != 0:
132 status = STATUS_RUNNING135 status = STATUS_RUNNING
133 event_time = max(value.get('start', 0), finished)136 event_time = max(start, finished)
134 if event_time > latest_event:137 if event_time > latest_event:
135 latest_event = event_time138 latest_event = event_time
136 if errors:139 if errors:
diff --git a/cloudinit/cmd/tests/test_status.py b/cloudinit/cmd/tests/test_status.py
index 6d4a11e..a7c0a91 100644
--- a/cloudinit/cmd/tests/test_status.py
+++ b/cloudinit/cmd/tests/test_status.py
@@ -93,6 +93,19 @@ class TestStatus(CiTestCase):
93 self.assertTrue(is_disabled, 'expected disabled cloud-init')93 self.assertTrue(is_disabled, 'expected disabled cloud-init')
94 self.assertEqual('Cloud-init disabled by cloud-init-generator', reason)94 self.assertEqual('Cloud-init disabled by cloud-init-generator', reason)
9595
96 def test__is_cloudinit_disabled_false_when_enabled_in_systemd(self):
97 '''Report enabled when systemd generator creates the enabled file.'''
98 enabled_file = os.path.join(self.paths.run_dir, 'enabled')
99 write_file(enabled_file, '')
100 (is_disabled, reason) = wrap_and_call(
101 'cloudinit.cmd.status',
102 {'uses_systemd': True,
103 'get_cmdline': 'something ignored'},
104 status._is_cloudinit_disabled, self.disable_file, self.paths)
105 self.assertFalse(is_disabled, 'expected enabled cloud-init')
106 self.assertEqual(
107 'Cloud-init enabled by systemd cloud-init-generator', reason)
108
96 def test_status_returns_not_run(self):109 def test_status_returns_not_run(self):
97 '''When status.json does not exist yet, return 'not run'.'''110 '''When status.json does not exist yet, return 'not run'.'''
98 self.assertFalse(111 self.assertFalse(
@@ -137,8 +150,9 @@ class TestStatus(CiTestCase):
137 self.assertEqual(expected, m_stdout.getvalue())150 self.assertEqual(expected, m_stdout.getvalue())
138151
139 def test_status_returns_running(self):152 def test_status_returns_running(self):
140 '''Report running when status file exists but isn't finished.'''153 '''Report running when status exists with an unfinished stage.'''
141 write_json(self.status_file, {'v1': {'init': {'finished': None}}})154 write_json(self.status_file,
155 {'v1': {'init': {'start': 1, 'finished': None}}})
142 cmdargs = myargs(long=False, wait=False)156 cmdargs = myargs(long=False, wait=False)
143 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:157 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
144 retcode = wrap_and_call(158 retcode = wrap_and_call(
@@ -338,7 +352,8 @@ class TestStatus(CiTestCase):
338352
339 def test_status_main(self):353 def test_status_main(self):
340 '''status.main can be run as a standalone script.'''354 '''status.main can be run as a standalone script.'''
341 write_json(self.status_file, {'v1': {'init': {'finished': None}}})355 write_json(self.status_file,
356 {'v1': {'init': {'start': 1, 'finished': None}}})
342 with self.assertRaises(SystemExit) as context_manager:357 with self.assertRaises(SystemExit) as context_manager:
343 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:358 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
344 wrap_and_call(359 wrap_and_call(
diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py
index eba58b0..4da3a58 100644
--- a/cloudinit/config/cc_power_state_change.py
+++ b/cloudinit/config/cc_power_state_change.py
@@ -194,6 +194,7 @@ def doexit(sysexit):
194194
195195
196def execmd(exe_args, output=None, data_in=None):196def execmd(exe_args, output=None, data_in=None):
197 ret = 1
197 try:198 try:
198 proc = subprocess.Popen(exe_args, stdin=subprocess.PIPE,199 proc = subprocess.Popen(exe_args, stdin=subprocess.PIPE,
199 stdout=output, stderr=subprocess.STDOUT)200 stdout=output, stderr=subprocess.STDOUT)
diff --git a/cloudinit/config/cc_rh_subscription.py b/cloudinit/config/cc_rh_subscription.py
index a9d21e7..530808c 100644
--- a/cloudinit/config/cc_rh_subscription.py
+++ b/cloudinit/config/cc_rh_subscription.py
@@ -276,9 +276,8 @@ class SubscriptionManager(object):
276 cmd = ['attach', '--auto']276 cmd = ['attach', '--auto']
277 try:277 try:
278 return_out, return_err = self._sub_man_cli(cmd)278 return_out, return_err = self._sub_man_cli(cmd)
279 except util.ProcessExecutionError:279 except util.ProcessExecutionError as e:
280 self.log_warn("Auto-attach failed with: "280 self.log_warn("Auto-attach failed with: {0}".format(e))
281 "{0}]".format(return_err.strip()))
282 return False281 return False
283 for line in return_out.split("\n"):282 for line in return_out.split("\n"):
284 if line is not "":283 if line is not "":
diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py
index bad112f..aa468bc 100644
--- a/cloudinit/distros/freebsd.py
+++ b/cloudinit/distros/freebsd.py
@@ -116,6 +116,7 @@ class Distro(distros.Distro):
116 (out, err) = util.subp(['ifconfig', '-a'])116 (out, err) = util.subp(['ifconfig', '-a'])
117 ifconfigoutput = [x for x in (out.strip()).splitlines()117 ifconfigoutput = [x for x in (out.strip()).splitlines()
118 if len(x.split()) > 0]118 if len(x.split()) > 0]
119 bsddev = 'NOT_FOUND'
119 for line in ifconfigoutput:120 for line in ifconfigoutput:
120 m = re.match('^\w+', line)121 m = re.match('^\w+', line)
121 if m:122 if m:
@@ -347,15 +348,9 @@ class Distro(distros.Distro):
347 bymac[Distro.get_interface_mac(n)] = {348 bymac[Distro.get_interface_mac(n)] = {
348 'name': n, 'up': self.is_up(n), 'downable': None}349 'name': n, 'up': self.is_up(n), 'downable': None}
349350
351 nics_with_addresses = set()
350 if check_downable:352 if check_downable:
351 nics_with_addresses = set()353 nics_with_addresses = set(self.get_ipv4() + self.get_ipv6())
352 ipv6 = self.get_ipv6()
353 ipv4 = self.get_ipv4()
354 for bytes_out in (ipv6, ipv4):
355 for i in ipv6:
356 nics_with_addresses.update(i)
357 for i in ipv4:
358 nics_with_addresses.update(i)
359354
360 for d in bymac.values():355 for d in bymac.values():
361 d['downable'] = (d['up'] is False or356 d['downable'] = (d['up'] is False or
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 9976400..338fb97 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):
1587 mtypes = list(mtype)1587 mtypes = list(mtype)
1588 elif mtype is None:1588 elif mtype is None:
1589 mtypes = None1589 mtypes = None
1590 else:
1591 raise TypeError(
1592 'Unsupported type provided for mtype parameter: {_type}'.format(
1593 _type=type(mtype)))
15901594
1591 # clean up 'mtype' input a bit based on platform.1595 # clean up 'mtype' input a bit based on platform.
1592 platsys = platform.system().lower()1596 platsys = platform.system().lower()

Subscribers

People subscribed via source and target branches