From ce72e77959a6834620793dc9d4006432781d09d7 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Wed, 29 Aug 2018 19:16:58 +0200 Subject: [PATCH] Improve detection of multipathd running There is a case where `multipathd show status` should fail, yet it doesn't exit with an error code. Instead it returns 0. Error is printed to stdout as "error receiving packet". This means that enforce will not work as expected. The multipathd error got fixed here: https://www.redhat.com/archives/dm-devel/2015-March/msg00136.html But that patch is missing in some packages, so this patch adds a workaround to detect the error base on the stdout of the multipathd show status command. Closes-Bug: #1789699 Change-Id: I1cb29782541e3fe53b0b1744ab36f56464cd2135 (cherry picked from commit 028af871de8a3f45f4cc8342e92d5586a7058e5a) (cherry picked from commit f2fed213f6b1d36513504d0ec4ebbfeeddc52b94) --- os_brick/initiator/linuxscsi.py | 9 +++++++-- os_brick/tests/initiator/test_connector.py | 4 ++-- os_brick/tests/initiator/test_linuxscsi.py | 21 +++++++++++++++------ 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/os_brick/initiator/linuxscsi.py b/os_brick/initiator/linuxscsi.py index 381725f4b..7c14ba5fb 100644 --- a/os_brick/initiator/linuxscsi.py +++ b/os_brick/initiator/linuxscsi.py @@ -168,8 +168,13 @@ class LinuxSCSI(executor.Executor): try: if execute is None: execute = priv_rootwrap.execute - execute('multipathd', 'show', 'status', - run_as_root=True, root_helper=root_helper) + cmd = ('multipathd', 'show', 'status') + out, _err = execute(*cmd, run_as_root=True, + root_helper=root_helper) + # There was a bug in multipathd where it didn't return an error + # code and just printed the error message in stdout. + if out and out.startswith('error receiving packet'): + raise putils.ProcessExecutionError('', out, 1, cmd, None) except putils.ProcessExecutionError as err: LOG.error('multipathd is not running: exit code %(err)s', {'err': err.exit_code}) diff --git a/os_brick/tests/initiator/test_connector.py b/os_brick/tests/initiator/test_connector.py index 61699c88e..36bd44dea 100644 --- a/os_brick/tests/initiator/test_connector.py +++ b/os_brick/tests/initiator/test_connector.py @@ -89,7 +89,7 @@ class ConnectorUtilsTestCase(test_base.TestCase): def test_brick_get_connector_properties(self): self._test_brick_get_connector_properties(False, False, False) - @mock.patch.object(priv_rootwrap, 'execute') + @mock.patch.object(priv_rootwrap, 'execute', return_value=('', '')) def test_brick_get_connector_properties_multipath(self, mock_execute): self._test_brick_get_connector_properties(True, True, True) mock_execute.assert_called_once_with('multipathd', 'show', 'status', @@ -149,7 +149,7 @@ class ConnectorTestCase(test_base.TestCase): def test_get_connector_properties(self): with mock.patch.object(priv_rootwrap, 'execute') as mock_exec: - mock_exec.return_value = True + mock_exec.return_value = ('', '') multipath = True enforce_multipath = True props = base.BaseLinuxConnector.get_connector_properties( diff --git a/os_brick/tests/initiator/test_linuxscsi.py b/os_brick/tests/initiator/test_linuxscsi.py index 6896c8325..21d4beda0 100644 --- a/os_brick/tests/initiator/test_linuxscsi.py +++ b/os_brick/tests/initiator/test_linuxscsi.py @@ -19,6 +19,7 @@ import time import ddt import mock +from oslo_concurrency import processutils as putils from oslo_log import log as logging from os_brick import exception @@ -829,12 +830,20 @@ loop0 0""" expected = 13 self.assertEqual(expected, result) - @mock.patch('os_brick.privileged.rootwrap') - def test_is_multipath_running_default_executor(self, mock_rootwrap): - self.assertTrue( - linuxscsi.LinuxSCSI.is_multipath_running( - False, None, mock_rootwrap.execute)) - mock_rootwrap.execute.assert_called_once_with( + @mock.patch('os_brick.privileged.rootwrap.execute', return_value=('', '')) + def test_is_multipath_running_default_executor(self, mock_exec): + res = linuxscsi.LinuxSCSI.is_multipath_running(False, None, mock_exec) + self.assertTrue(res) + mock_exec.assert_called_once_with( + 'multipathd', 'show', 'status', run_as_root=True, root_helper=None) + + @mock.patch('os_brick.privileged.rootwrap.execute') + def test_is_multipath_running_failure_exit_code_0(self, mock_exec): + mock_exec.return_value = ('error receiving packet', '') + self.assertRaises(putils.ProcessExecutionError, + linuxscsi.LinuxSCSI.is_multipath_running, + True, None, mock_exec) + mock_exec.assert_called_once_with( 'multipathd', 'show', 'status', run_as_root=True, root_helper=None) @mock.patch('glob.glob')