x/vmware-nsx
Code Issues Proposed changes

Make linux.utils.execute log error on return codes

Browse Source 
Previously, the execute method in neutron logs everything as debug which hides
a lot of extremely fatal errors like unable to apply security group rules!
This patch changes this code so that we log all non 0 returns as error.

Change-Id: I7328e62269212ccd9c7950ff064a3e337de56918
Closes-bug: 1323832
Related-bug: 1322945
This commit is contained in:
Aaron Rosen 2014-05-27 13:39:39 -07:00
parent 06ee130050
commit 593aa60ee9
2 changed files with 41 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
neutron/agent/linux/utils.py
View File

@ -71,9 +71,12 @@ def execute(cmd, root_helper=None, process_input=None, addl_env=None,
m = _("\nCommand: %(cmd)s\nExit code: %(code)s\nStdout: %(stdout)r\n" m = _("\nCommand: %(cmd)s\nExit code: %(code)s\nStdout: %(stdout)r\n"
"Stderr: %(stderr)r") % {'cmd': cmd, 'code': obj.returncode, "Stderr: %(stderr)r") % {'cmd': cmd, 'code': obj.returncode,
'stdout': _stdout, 'stderr': _stderr} 'stdout': _stdout, 'stderr': _stderr}
LOG.debug(m) if obj.returncode:
if obj.returncode and check_exit_code: LOG.error(m)
if check_exit_code:
raise RuntimeError(m) raise RuntimeError(m)
else:
LOG.debug(m)
finally: finally:
# NOTE(termie): this appears to be necessary to let the subprocess # NOTE(termie): this appears to be necessary to let the subprocess
# call clean something up in between calls, without # call clean something up in between calls, without

35
neutron/tests/unit/test_agent_linux_utils.py
View File

@ -20,6 +20,19 @@ from neutron.agent.linux import utils
from neutron.tests import base from neutron.tests import base
class FakeCreateProcess(object):
class FakeStdin(object):
def close(self):
pass
def __init__(self, returncode):
self.returncode = returncode
self.stdin = self.FakeStdin()
def communicate(self, process_input=None):
return '', ''
class AgentUtilsExecuteTest(base.BaseTestCase): class AgentUtilsExecuteTest(base.BaseTestCase):
def setUp(self): def setUp(self):
super(AgentUtilsExecuteTest, self).setUp() super(AgentUtilsExecuteTest, self).setUp()
@ -75,6 +88,28 @@ class AgentUtilsExecuteTest(base.BaseTestCase):
addl_env={'foo': 'bar'}) addl_env={'foo': 'bar'})
self.assertEqual(result, expected) self.assertEqual(result, expected)
def test_return_code_log_error_raise_runtime(self):
with mock.patch.object(utils, 'create_process') as create_process:
create_process.return_value = FakeCreateProcess(1), 'ls'
with mock.patch.object(utils, 'LOG') as log:
self.assertRaises(RuntimeError, utils.execute,
['ls'])
self.assertTrue(log.error.called)
def test_return_code_log_error_no_raise_runtime(self):
with mock.patch.object(utils, 'create_process') as create_process:
create_process.return_value = FakeCreateProcess(1), 'ls'
with mock.patch.object(utils, 'LOG') as log:
utils.execute(['ls'], check_exit_code=False)
self.assertTrue(log.error.called)
def test_return_code_log_debug(self):
with mock.patch.object(utils, 'create_process') as create_process:
create_process.return_value = FakeCreateProcess(0), 'ls'
with mock.patch.object(utils, 'LOG') as log:
utils.execute(['ls'])
self.assertTrue(log.debug.called)
class AgentUtilsGetInterfaceMAC(base.BaseTestCase): class AgentUtilsGetInterfaceMAC(base.BaseTestCase):
def test_get_interface_mac(self): def test_get_interface_mac(self):