Use try_execute from ironic-lib

Also adapt unit tests

Change-Id: I37d050877daabc9dc0a5821cf20a689652b26f34

3 files changed, 47 insertions, 62 deletions

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 99d2d332..44db81b1 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -1119,7 +1119,7 @@ class GenericHardwareManager(HardwareManager):
         freq = cpu_info.get('cpu max mhz', cpu_info.get('cpu mhz'))
 
         flags = []
-        out = utils.try_execute('grep', '-Em1', '^flags', '/proc/cpuinfo')
+        out = il_utils.try_execute('grep', '-Em1', '^flags', '/proc/cpuinfo')
         if out:
             try:
                 # Example output (much longer for a real system):
@@ -1699,9 +1699,9 @@ class GenericHardwareManager(HardwareManager):
                  configured properly
         """
         # These modules are rarely loaded automatically
-        utils.try_execute('modprobe', 'ipmi_msghandler')
-        utils.try_execute('modprobe', 'ipmi_devintf')
-        utils.try_execute('modprobe', 'ipmi_si')
+        il_utils.try_execute('modprobe', 'ipmi_msghandler')
+        il_utils.try_execute('modprobe', 'ipmi_devintf')
+        il_utils.try_execute('modprobe', 'ipmi_si')
 
         try:
             # From all the channels 0-15, only 1-11 can be assigned to
@@ -1742,9 +1742,9 @@ class GenericHardwareManager(HardwareManager):
                  interract with system tools or critical error occurs.
         """
         # These modules are rarely loaded automatically
-        utils.try_execute('modprobe', 'ipmi_msghandler')
-        utils.try_execute('modprobe', 'ipmi_devintf')
-        utils.try_execute('modprobe', 'ipmi_si')
+        il_utils.try_execute('modprobe', 'ipmi_msghandler')
+        il_utils.try_execute('modprobe', 'ipmi_devintf')
+        il_utils.try_execute('modprobe', 'ipmi_si')
 
         null_address_re = re.compile(r'^::(/\d{1,3})*$')
 
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index dd3d6b4a..a356b683 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -19,6 +19,7 @@ import time
 from unittest import mock
 
 from ironic_lib import disk_utils
+from ironic_lib import utils as il_utils
 import netifaces
 from oslo_concurrency import processutils
 from oslo_config import cfg
@@ -968,12 +969,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                 '/sys/class/block/sdfake/device/vendor', 'r')
             self.assertEqual('fake-vendor', vendor)
 
+    @mock.patch.object(il_utils, 'execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_cpus(self, mocked_execute):
-        mocked_execute.side_effect = [
-            (hws.LSCPU_OUTPUT, ''),
-            (hws.CPUINFO_FLAGS_OUTPUT, '')
-        ]
+    def test_get_cpus(self, mocked_execute, mte):
+        mocked_execute.return_value = (hws.LSCPU_OUTPUT, '')
+        mte.return_value = (hws.CPUINFO_FLAGS_OUTPUT, '')
 
         cpus = self.hardware.get_cpus()
         self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz',
@@ -983,12 +983,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('x86_64', cpus.architecture)
         self.assertEqual(['fpu', 'vme', 'de', 'pse'], cpus.flags)
 
+    @mock.patch.object(il_utils, 'execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_cpus2(self, mocked_execute):
-        mocked_execute.side_effect = [
-            (hws.LSCPU_OUTPUT_NO_MAX_MHZ, ''),
-            (hws.CPUINFO_FLAGS_OUTPUT, '')
-        ]
+    def test_get_cpus2(self, mocked_execute, mte):
+        mocked_execute.return_value = (hws.LSCPU_OUTPUT_NO_MAX_MHZ, '')
+        mte.return_value = (hws.CPUINFO_FLAGS_OUTPUT, '')
 
         cpus = self.hardware.get_cpus()
         self.assertEqual('Intel(R) Xeon(R) CPU E5-1650 v3 @ 3.50GHz',
@@ -998,12 +997,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('x86_64', cpus.architecture)
         self.assertEqual(['fpu', 'vme', 'de', 'pse'], cpus.flags)
 
+    @mock.patch.object(il_utils, 'execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_cpus_no_flags(self, mocked_execute):
-        mocked_execute.side_effect = [
-            (hws.LSCPU_OUTPUT, ''),
-            processutils.ProcessExecutionError()
-        ]
+    def test_get_cpus_no_flags(self, mocked_execute, mte):
+        mocked_execute.return_value = (hws.LSCPU_OUTPUT, '')
+        mte.side_effect = processutils.ProcessExecutionError()
 
         cpus = self.hardware.get_cpus()
         self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz',
@@ -1013,12 +1011,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('x86_64', cpus.architecture)
         self.assertEqual([], cpus.flags)
 
+    @mock.patch.object(il_utils, 'execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_cpus_illegal_flags(self, mocked_execute):
-        mocked_execute.side_effect = [
-            (hws.LSCPU_OUTPUT, ''),
-            ('I am not a flag', '')
-        ]
+    def test_get_cpus_illegal_flags(self, mocked_execute, mte):
+        mocked_execute.return_value = (hws.LSCPU_OUTPUT, '')
+        mte.return_value = ('I am not a flag', '')
 
         cpus = self.hardware.get_cpus()
         self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz',
@@ -2189,35 +2186,41 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             mock_open.assert_called_once_with(
                 '/sys/block/sdfake/ro', 'r')
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_address(self, mocked_execute):
+    def test_get_bmc_address(self, mocked_execute, mte):
         mocked_execute.return_value = '192.1.2.3\n', ''
         self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_address_virt(self, mocked_execute):
+    def test_get_bmc_address_virt(self, mocked_execute, mte):
         mocked_execute.side_effect = processutils.ProcessExecutionError()
         self.assertIsNone(self.hardware.get_bmc_address())
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_address_zeroed(self, mocked_execute):
+    def test_get_bmc_address_zeroed(self, mocked_execute, mte):
         mocked_execute.return_value = '0.0.0.0\n', ''
         self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_address_invalid(self, mocked_execute):
+    def test_get_bmc_address_invalid(self, mocked_execute, mte):
         # In case of invalid lan channel, stdout is empty and the error
         # on stderr is "Invalid channel"
         mocked_execute.return_value = '\n', 'Invalid channel: 55'
         self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_address_random_error(self, mocked_execute):
+    def test_get_bmc_address_random_error(self, mocked_execute, mte):
         mocked_execute.return_value = '192.1.2.3\n', 'Random error message'
         self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_address_iterate_channels(self, mocked_execute):
+    def test_get_bmc_address_iterate_channels(self, mocked_execute, mte):
         # For channel 1 we simulate unconfigured IP
         # and for any other we return a correct IP address
         def side_effect(*args, **kwargs):
@@ -2232,18 +2235,19 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         mocked_execute.side_effect = side_effect
         self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_address_not_available(self, mocked_execute):
+    def test_get_bmc_address_not_available(self, mocked_execute, mte):
         mocked_execute.return_value = '', ''
         self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
 
-    @mock.patch.object(utils, 'try_execute', autospec=True)
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_bmc_v6address_not_enabled(self, mocked_execute, mte):
         mocked_execute.side_effect = [('ipv4\n', '')] * 11
         self.assertEqual('::/0', self.hardware.get_bmc_v6address())
 
-    @mock.patch.object(utils, 'try_execute', autospec=True)
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_bmc_v6address_dynamic_address(self, mocked_execute, mte):
         mocked_execute.side_effect = [
@@ -2253,7 +2257,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('2001:1234:1234:1234:1234:1234:1234:1234',
                          self.hardware.get_bmc_v6address())
 
-    @mock.patch.object(utils, 'try_execute', autospec=True)
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_bmc_v6address_static_address_both(self, mocked_execute, mte):
         dynamic_disabled = \
@@ -2266,12 +2270,13 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('2001:5678:5678:5678:5678:5678:5678:5678',
                          self.hardware.get_bmc_v6address())
 
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_get_bmc_v6address_virt(self, mocked_execute):
+    def test_get_bmc_v6address_virt(self, mocked_execute, mte):
         mocked_execute.side_effect = processutils.ProcessExecutionError()
         self.assertIsNone(self.hardware.get_bmc_v6address())
 
-    @mock.patch.object(utils, 'try_execute', autospec=True)
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_bmc_v6address_invalid_enables(self, mocked_execute, mte):
         def side_effect(*args, **kwargs):
@@ -2281,7 +2286,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         mocked_execute.side_effect = side_effect
         self.assertEqual('::/0', self.hardware.get_bmc_v6address())
 
-    @mock.patch.object(utils, 'try_execute', autospec=True)
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_bmc_v6address_invalid_get_address(self, mocked_execute, mte):
         def side_effect(*args, **kwargs):
@@ -2295,7 +2300,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('::/0', self.hardware.get_bmc_v6address())
 
     @mock.patch.object(hardware, 'LOG', autospec=True)
-    @mock.patch.object(utils, 'try_execute', autospec=True)
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_bmc_v6address_ipmitool_invalid_stdout_format(
             self, mocked_execute, mte, mocked_log):
@@ -2312,7 +2317,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                              'command: %(e)s', mock.ANY)
         mocked_log.warning.assert_has_calls([one_call] * 14)
 
-    @mock.patch.object(utils, 'try_execute', autospec=True)
+    @mock.patch.object(il_utils, 'try_execute', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_bmc_v6address_channel_7(self, mocked_execute, mte):
         def side_effect(*args, **kwargs):
diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py
index a5da83d6..7ae827aa 100644
--- a/ironic_python_agent/utils.py
+++ b/ironic_python_agent/utils.py
@@ -88,26 +88,6 @@ def execute(*cmd, **kwargs):
     return ironic_utils.execute(*cmd, **kwargs)
 
 
-def try_execute(*cmd, **kwargs):
-    """The same as execute but returns None on error.
-
-    Executes and logs results from a system command. See docs for
-    oslo_concurrency.processutils.execute for usage.
-
-    Instead of raising an exception on failure, this method simply
-    returns None in case of failure.
-
-    :param cmd: positional arguments to pass to processutils.execute()
-    :param kwargs: keyword arguments to pass to processutils.execute()
-    :raises: UnknownArgumentError on receiving unknown arguments
-    :returns: tuple of (stdout, stderr) or None in some error cases
-    """
-    try:
-        return execute(*cmd, **kwargs)
-    except (processutils.ProcessExecutionError, OSError) as e:
-        LOG.debug('Command failed: %s', e)
-
-
 def _read_params_from_file(filepath):
     """Extract key=value pairs from a file.