From e4d7c1f41e476eb55b84221076fb18c13ebb2071 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 2 Jun 2021 11:28:11 +0200 Subject: [PATCH] utils.execute: log stdout and stderr even on failure Add logging when a command is not found. Also remove duplicate logging of exit code and command line: it's already done by processutils. Change-Id: I0349073cf0f78fb021dcd3e830fedddf2b4a0c92 --- ironic_lib/tests/test_utils.py | 16 +++++++++++++--- ironic_lib/utils.py | 23 ++++++++++++++++------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/ironic_lib/tests/test_utils.py b/ironic_lib/tests/test_utils.py index 3775d836..d72875be 100644 --- a/ironic_lib/tests/test_utils.py +++ b/ironic_lib/tests/test_utils.py @@ -110,12 +110,12 @@ class ExecuteTestCase(base.IronicLibTestCase): else: utils.execute('foo') execute_mock.assert_called_once_with('foo') - name, args, kwargs = log_mock.debug.mock_calls[1] + name, args, kwargs = log_mock.debug.mock_calls[0] if log_stdout is False: - self.assertEqual(2, log_mock.debug.call_count) + self.assertEqual(1, log_mock.debug.call_count) self.assertNotIn('stdout', args[0]) else: - self.assertEqual(3, log_mock.debug.call_count) + self.assertEqual(2, log_mock.debug.call_count) self.assertIn('stdout', args[0]) def test_execute_with_log_stdout_default(self): @@ -127,6 +127,16 @@ class ExecuteTestCase(base.IronicLibTestCase): def test_execute_with_log_stdout_false(self): self._test_execute_with_log_stdout(log_stdout=False) + @mock.patch.object(utils, 'LOG', autospec=True) + @mock.patch.object(processutils, 'execute', autospec=True) + def test_execute_command_not_found(self, execute_mock, log_mock): + execute_mock.side_effect = FileNotFoundError + self.assertRaises(FileNotFoundError, utils.execute, 'foo') + execute_mock.assert_called_once_with('foo') + name, args, kwargs = log_mock.debug.mock_calls[0] + self.assertEqual(1, log_mock.debug.call_count) + self.assertIn('not found', args[0]) + class MkfsTestCase(base.IronicLibTestCase): diff --git a/ironic_lib/utils.py b/ironic_lib/utils.py index c86d17a8..38cb0003 100644 --- a/ironic_lib/utils.py +++ b/ironic_lib/utils.py @@ -94,13 +94,22 @@ def execute(*cmd, use_standard_locale=False, log_stdout=True, **kwargs): else: kwargs['root_helper'] = CONF.ironic_lib.root_helper - result = processutils.execute(*cmd, **kwargs) - LOG.debug('Execution completed, command line is "%s"', - ' '.join(map(str, cmd))) - if log_stdout: - LOG.debug('Command stdout is: "%s"', result[0]) - LOG.debug('Command stderr is: "%s"', result[1]) - return result + def _log(stdout, stderr): + if log_stdout: + LOG.debug('Command stdout is: "%s"', stdout) + LOG.debug('Command stderr is: "%s"', stderr) + + try: + result = processutils.execute(*cmd, **kwargs) + except FileNotFoundError: + with excutils.save_and_reraise_exception(): + LOG.debug('Command not found: "%s"', ' '.join(map(str, cmd))) + except processutils.ProcessExecutionError as exc: + with excutils.save_and_reraise_exception(): + _log(exc.stdout, exc.stderr) + else: + _log(result[0], result[1]) + return result def try_execute(*cmd, **kwargs):